Reduce live discovery context and bind activation identity - #42
Merged
Conversation
📝 WalkthroughWalkthroughThe change separates local agent-recorded review artifacts from authenticated protected GitHub review. It updates CLI readiness, delivery routing, project templates, evaluation scenarios, documentation, and regression tests to enforce this distinction. ChangesReview policy and delivery workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LocalCLI
participant LocalReviewArtifact
participant ReviewStatus
participant ProtectedGitHubReview
LocalCLI->>LocalReviewArtifact: record exact-head local result
LocalReviewArtifact->>ReviewStatus: report structural validity
ReviewStatus-->>LocalCLI: keep readiness blocked
LocalCLI->>ProtectedGitHubReview: request authenticated review when required
ProtectedGitHubReview-->>ReviewStatus: return protected review receipt
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
samtay32
marked this pull request as ready for review
August 3, 2026 21:38
PR Summary by QodoFail-closed local review artifacts; bind activation receipts to harness/model identity
AI Description
Diagram
High-Level Assessment
Files changed (30)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
113 rules 1.
|
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/skill-eval.mjs (1)
1274-1286: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReject review expectation combinations that can never be observed.
reviewStatusis now onlyblockedornot-required. Two declarable expectations are therefore unsatisfiable:
expected.review: "passed"always producesreview outcome was blocked, so theexpected === "passed"branch at Line 1033 is dead and the value is a permanent failure.expected.review_evidenceis validated only againstREVIEW_EVIDENCE_OUTCOMES. A scenario can declarereview: "not-required"withreview_evidence: "unavailable", orreview: "blocked"withreview_evidence: "not-required", and both fail on every run.Add a catalog-level consistency check so an unsatisfiable contract fails
eval:contractsat authoring time instead of failing every recorded run.♻️ Proposed catalog check
if ( expected.review_evidence !== undefined && !REVIEW_EVIDENCE_OUTCOMES.has(expected.review_evidence) ) { errors.push( `${location}.expected.review_evidence must be a supported review evidence outcome`, ); } + if (expected.review === "passed") { + errors.push( + `${location}.expected.review cannot be passed because local review evidence never derives a passed status`, + ); + } + if ( + REVIEW_EVIDENCE_OUTCOMES.has(expected.review_evidence) && + (expected.review === "not-required") !== + (expected.review_evidence === "not-required") + ) { + errors.push( + `${location}.expected.review_evidence must agree with ${location}.expected.review`, + ); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/skill-eval.mjs` around lines 1274 - 1286, Update the catalog validation around REVIEW_EXPECTATIONS and REVIEW_EVIDENCE_OUTCOMES to reject unsatisfiable review contracts: disallow expected.review values that reviewStatus cannot produce, and require review_evidence to match the corresponding review value, including rejecting “unavailable” with “not-required” and “not-required” with “blocked.” Ensure these consistency errors are reported by eval:contracts during authoring validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/project-template/.cursor/commands/deliver.md`:
- Around line 1-3: Update the delivery entrypoint instructions to require
reading `.agent-stack/core-policy.json`, a valid checkpoint, and the current
diff in addition to the existing project contract, configuration, and
route-relevant artifacts. Before accepting the request for delivery, require
`start` to acquire the Project Steward lease and run `doctor`, including local
CLI startup evidence; preserve the conditional knowledge-provider usage for
decisions requiring information beyond the checkout.
In `@assets/project-template/AGENTS.md`:
- Around line 56-66: Update the activation recipes in the hash-bound controller
read instructions so --skill-path uses the actual canonical project-relative
installed skill path discovered by the adapter, rather than hardcoding
.agents/skills. Preserve the required skill-specific paths for
run-autonomous-delivery and develop-project-brief, and ensure receipts record
that exact path with the appropriate activation mode.
In `@bin/ultimate-agent-stack.mjs`:
- Around line 4993-4994: Update the commandReviewRecord flow around
localReviewArtifactValid to handle a null gitSnapshot before any Git-dependent
access. Normalize a missing snapshot into the existing blocked-status/reason
path for the entire function, and reuse that normalized result for later
Git-dependent output rather than only optional-chaining git.clean.
In `@CHANGELOG.md`:
- Around line 27-31: Update the changelog release note to describe local audit
output as diagnostic artifact-integrity evidence, not as a passed exact-head
audit that establishes readiness. State that built-in readiness still requires
current successful verification, while authenticated protected review remains
the separate configured gate; do not imply local receipts establish PR
readiness.
In `@README.md`:
- Around line 237-240: Update the README delivery-flow diagram around the
DRAFT-to-PROTECTED transition so protected GitHub review is explicitly
conditional when policy requires it or external review is configured. Preserve
local reviewer-result artifacts as diagnostic receipts and keep protected review
represented as the separate authenticated gate.
In `@skills/setup-autonomous-project/references/setup-contract.md`:
- Line 48: Update the prose sentence in setup-contract.md to use “built-in” when
describing the reviewer-result artifact, while retaining “builtin” only for the
provider enum or configuration value.
In `@test/maintenance.test.mjs`:
- Around line 899-905: Replace the broad alternation assertion in the local
review receipt test with source-specific checks that require each document to
state that receipts are agent-recorded and do not prove reviewer independence.
Remove the generic “always keeps” match and assert the precise local-receipt
claim for every source under test, while preserving the existing agent-recorded
and protected GitHub review assertions.
In `@test/skill-eval.test.mjs`:
- Around line 1201-1240: Extend the review evidence test to cover a
direct-delivery scenario with its valid passed receipt plus a valid
changes-requested receipt and artifact. Evaluate the record and assert it fails
with “review evidence contains conflicting outcomes” and reports
review.evidence_outcome as “conflict”, while preserving the existing blocked and
tampered cases.
---
Outside diff comments:
In `@scripts/skill-eval.mjs`:
- Around line 1274-1286: Update the catalog validation around
REVIEW_EXPECTATIONS and REVIEW_EVIDENCE_OUTCOMES to reject unsatisfiable review
contracts: disallow expected.review values that reviewStatus cannot produce, and
require review_evidence to match the corresponding review value, including
rejecting “unavailable” with “not-required” and “not-required” with “blocked.”
Ensure these consistency errors are reported by eval:contracts during authoring
validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4af78dd6-3fa9-40d6-b1a6-721f7701982a
📒 Files selected for processing (30)
CHANGELOG.mdREADME.mdSTARTER_PROMPT.mdassets/project-template/.agent-stack/HANDOFF.mdassets/project-template/.cursor/commands/deliver.mdassets/project-template/.cursor/rules/agent-stack.mdcassets/project-template/AGENTS.mdassets/project-template/CLAUDE.mdassets/project-template/GEMINI.mdbin/ultimate-agent-stack.mjsdocs/ADAPTERS.mddocs/ARCHITECTURE.mddocs/BEHAVIORAL_EVALS.mddocs/OPERATING_MANUAL.mddocs/SKILL_STACK.mddocs/TRUST.mdevals/fixture-baselines.jsonevals/scenarios.jsonscripts/skill-eval.mjsskills/close-review-loop/SKILL.mdskills/coordinate-parallel-delivery/SKILL.mdskills/coordinate-parallel-delivery/references/delegation-contract.mdskills/run-autonomous-delivery/SKILL.mdskills/run-autonomous-delivery/references/delivery-policy.mdskills/setup-autonomous-project/references/setup-contract.mdskills/verify-change/SKILL.mdtest/agent-stack.test.mjstest/maintenance.test.mjstest/mechanical-receipts.test.mjstest/skill-eval.test.mjs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
local_review_artifact_validdescribes structural exact-head validity, whilelocal_review_artifact_outcomereportspassed,changes-requested,unavailable,conflict,missing, orinvalid.local_review_audit_passedas a fail-closed compatibility field that is alwaysfalse.git: nullinstead of crashing.Why
Issue #39 identified an unsupported-review risk: a coordinator could write a reviewer artifact after a failed or missing delegation and present it as successful local review evidence.
Retained live Codex evidence confirmed that native reviewer dispatch is model/harness behavior, not something a portable npm package can authenticate. The CLI therefore fails closed: local artifacts remain inspectable, but only protected GitHub review can provide the mechanical review gate.
User impact
changes-requestedorunavailableremain honestly inspectable without being mislabeled as corrupted evidence.Exact validation
90a7aa6564421e07da66747d428b9710c0f389d522.23.2npm run release:check: passedd16224d9a060d7120090f9593435e058e761dcb17e27068ec79eca938e474d8c(two proposals byte-identical and equal to the tracked catalog)sha256:64af47dfde66c5a75fc8ef861e06025b1e3056b15286482c7d8c6a2856e633d0Retained live evidence and honest boundary
Fresh Codex cases confirmed explanation-only, vague discovery, and complete supplied-PRD behavior. The direct-delivery case twice skipped native reviewer dispatch and invented a local reviewer identity. The final implementation prevents those artifacts from becoming a passed-audit, independent-review, or PR-ready claim.
No universal native-dispatch reliability claim is made. Additional paid cross-harness reruns were intentionally stopped because portable instructions cannot authenticate harness-native delegation. Protected GitHub review remains the trusted mechanical gate.
Relates to #39.