feat(agent-loop): enforce machine-checkable chunk scope - #203
Merged
Conversation
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe PR introduces schema-v1 machine-checkable chunk scopes, signed-state contract selection, complete Git delta enforcement, Agent Gates integration, reviewer evidence binding, updated governance documentation, and extensive validation tests for WS-ENG-008. ChangesChunk scope enforcement
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant AgentGates
participant ScopeValidator
participant SignedLoopMemory
participant Git
participant ReviewEvidence
PullRequest->>AgentGates: submit base, head, and changed files
AgentGates->>SignedLoopMemory: fetch authenticated state
AgentGates->>ScopeValidator: validate chunk scope
ScopeValidator->>Git: inspect signed contract and complete delta
Git-->>ScopeValidator: contract and changed paths
ScopeValidator-->>AgentGates: scope pass or failure
AgentGates->>ReviewEvidence: validate reviewer tracks and commands
ReviewEvidence-->>AgentGates: evidence pass or failure
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
scripts/test_check_chunk_contract.py (4)
191-277: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSecond
select_contractcall depends on leaving the mock scope — make that explicit.Line 254 relies on the fact that
verify_state_refis no longer patched once thewithblock at Lines 242-245 exits, which is easy to break with an innocent re-indent. A one-line comment (or an explicit# unpatched: real signature verification must fail) would preserve the intent.🤖 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/test_check_chunk_contract.py` around lines 191 - 277, The second select_contract call after the initial mock.patch.object scope relies on real verify_state_ref execution to produce the authentication failure. Add a concise comment immediately before that call explicitly stating that verification is intentionally unpatched and must fail with “authentication failed”; leave the existing call and assertions unchanged.
418-423: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThese two tests assume a case-sensitive filesystem tolerating invalid UTF-8 names.
readme.mdalongsideREADME.md(Line 421) andb"bad-\xff"(Line 507) will not reproduce on macOS/APFS or Windows, so a local run fails or silently passes for the wrong reason. Skipping when the filesystem cannot represent the case keeps the suite runnable off Linux CI.Also applies to: 506-520
🤖 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/test_check_chunk_contract.py` around lines 418 - 423, Update test_untracked_case_collision_with_unchanged_head_is_rejected and the related invalid-UTF-8 filename test to detect whether the filesystem supports the required case collision and byte-name representation before creating fixtures. Skip each test when its filesystem capability is unavailable, while preserving the existing assertions on filesystems that support the scenarios.
278-334: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnused repository fixture and a test name that overstates coverage.
test_rejects_stopped_projection_and_blob_mutationonly exercisessigned_contract_blob; nothing asserts stopped-projection behavior. Intest_grandfather_requires_active_exact_cutover_parent_and_prestart, the whole temp-repo setup on Lines 297-316 is never read —require_grandfatheris driven purely by the in-memoryrecordstuple. Drop the dead repo setup (or assert against it) and rename the first test to match what it verifies.🤖 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/test_check_chunk_contract.py` around lines 278 - 334, Rename test_rejects_stopped_projection_and_blob_mutation to reflect that it only verifies signed_contract_blob path/blob mutation rejection. In test_grandfather_requires_active_exact_cutover_parent_and_prestart, remove the unused temporary repository setup and related git/file operations, keeping the in-memory records and require_grandfather assertions intact.
336-377: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
contextlib.ExitStackover thecommontuple.Pre-constructing patchers in a tuple at Lines 353-359 and entering them as
common[0], common[1]is harder to read than duplicating the two patches or stacking them; it also silently breaks if the tuple is ever entered twice.🤖 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/test_check_chunk_contract.py` around lines 336 - 377, Replace the pre-constructed common patcher tuple in test_select_rejects_stopped_and_post_cutover_no_schema_starts with contextlib.ExitStack, entering the shared added_merge_intent and verify_state_ref patches through the stack for the first assertion. Keep the test’s existing behavior and separate second assertion setup unchanged.scripts/test_agent_gates.py (1)
6499-6533: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider anchoring the script assertions to the actual step, not the whole file.
Lines 6509-6514 assert bare substrings anywhere in the workflow, so an unrelated mention would satisfy them. The neighbouring test at Line 6478 already parses the YAML and matches the exact step; doing the same for the trusted-materialization loop would make this regression harder to defeat.
🤖 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/test_agent_gates.py` around lines 6499 - 6533, Update test_machine_chunk_scope_gate_is_mandatory_and_signed_state_bound to parse the workflow YAML and locate the trusted-materialization loop step, then assert the required script names, git show command, trusted_checker invocation, PYTHONPATH, and copy command within that step’s content instead of searching the entire workflow text. Follow the exact-step matching approach used by the neighboring test.scripts/check_chunk_contract.py (1)
323-327: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout and
--ref separation to the sharedgitrunner.Two hardening gaps in the single choke point for all Git execution:
- No
timeout=, so a hunggit(credential prompt, slow fetch of a missing ref) blocks the gate job indefinitely rather than failing closed.- Ref values arrive from
--base-ref/--head-refand are interpolated directly (rev-parse --verify f"{ref}^{{commit}}",ls-tree state_ref). A value beginning with-is parsed as a Git option. Validating refs against a conservative pattern (or inserting--) keeps the trust boundary explicit even though the workflow supplies these today.The static-analysis “command injection” hits on Lines 323 and 603 are otherwise false positives — the argv is a fixed list, not a shell string.
♻️ Suggested change
def _git(repo: Path, *args: str) -> bytes: - proc = subprocess.run(["git", *args], cwd=repo, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + proc = subprocess.run( + ["git", *args], cwd=repo, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + timeout=GIT_TIMEOUT_SECONDS, + ) if proc.returncode: raise ContractError(f"git {' '.join(args)} failed: {proc.stderr.decode('utf-8', 'replace').strip()}") return proc.stdout
subprocess.TimeoutExpiredshould be mapped toContractError(or added to theexcepttuple inmain) so the gate keeps its stable failure surface.🤖 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/check_chunk_contract.py` around lines 323 - 327, Harden the shared _git runner by supplying the configured subprocess timeout and converting subprocess.TimeoutExpired into ContractError so callers retain the stable failure surface. Ensure ref-derived Git arguments are protected with explicit -- separators or conservative validation, including rev-parse and ls-tree paths, while keeping the fixed argv execution unchanged.Source: Linters/SAST tools
🤖 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
@.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/reviews/WS-ENG-008-01-internal-review-evidence.md:
- Around line 23-27: Restore the truncated provenance SHAs to their exact
40-character values consistently across all review records: update the reviewed
code SHA at
.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/reviews/WS-ENG-008-01-internal-review-evidence.md
lines 23-27, the signed state commit at lines 15-19, and both reviewed
implementation SHA references in
.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/reviews/WS-ENG-008-01-pr-trust-bundle.md
lines 26 and 97. Use the authoritative full values already present in the
corresponding assurance blocks and ensure every reference matches exactly.
- Around line 65-82: Align the “Commands Run” fence and “Results” section in the
internal review evidence: either add the exact Ruff check invocation to the
commands list or remove the Ruff passing claim from Results. Keep the evidence
fence authoritative and update only the affected command/result text.
In
@.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/reviews/WS-ENG-008-01-pr-trust-bundle.md:
- Around line 97-104: Update the trust bundle’s reviewer evidence so all nine
required tracks are mapped to reviewer run IDs, matching the nine rows in the
evidence table. Add the missing run IDs or explicitly document which named
tracks shared a session, ensuring the “All nine required tracks passed” claim is
self-verifying.
In
@.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/STATUS.md:
- Line 12: Correct the ENG start projection commit SHA in the “Signed start run”
entry to the valid 40-character commit object ID, preserving the existing
provenance reference and formatting.
In @.agent-loop/templates/CHUNK_CONTRACT.md:
- Around line 57-62: Update the required_reviewers contract example and guidance
so the JSON array includes every applicable conditional track in addition to the
baseline reviewers, keeping it aligned with the human review requirements. Add
or update tests covering agreement between the machine-readable array and the
human review rules, including applicable architecture, CI, docs, reuse/dedup,
and test-delta reviewers.
- Around line 45-50: Update the mandatory chunk-scope JSON example in
CHUNK_CONTRACT.md so phase and risk_class are clearly placeholders or otherwise
represent the permitted values, rather than hard-coding “implementation” and
“L1”. Preserve the documented allowed phase “specification” and risk classes
“L0”–“L4” so copied metadata remains valid.
In @.github/workflows/agent-gates.yml:
- Around line 32-54: Update the fallback branch in the “Machine-checkable chunk
scope” workflow to positively test for the
.agent-loop/merge-intents/WS-ENG-008-01.json cutover marker and explicitly exit
1 when it exists. Remove the negated git cat-file check, and only copy the local
scripts when the marker is absent.
In `@scripts/check_chunk_contract.py`:
- Around line 431-441: Move the guarded imports in the state-ref authentication
flow before the try/except, or otherwise ensure LoopMemoryError is bound before
handler evaluation. Keep verify_generated_state_signature and
generated_state_failures execution covered by the existing exception mapping so
any import or validation failure consistently raises ContractError with the
“state-ref authentication failed” message.
- Around line 636-641: Wrap every relevant UTF-8 decode in
scripts/check_chunk_contract.py—including machine_block_or_none and the bare
decode calls near machine_block, parse-contract handling, and related validation
paths—so UnicodeDecodeError is converted into ContractError with the existing
stable gate-failure message. Preserve current successful decoding and validation
behavior, and ensure main continues to handle malformed non-UTF-8 contract blobs
through ContractError rather than a raw traceback.
In `@scripts/check_internal_review_evidence.py`:
- Around line 6-19: Make check_internal_review_evidence.py load
check_chunk_contract from a base-ref-sourced trusted path instead of the PR
workspace. Apply this to parse_contract_bytes, ScopeContract, and
VERIFICATION_COMMANDS used by machine_contract_for and
validate_machine_evidence, reusing trusted_scope_dir when available or loading
the fetched module via importlib. Preserve the existing evidence validation
behavior while preventing PR changes to the enforcement module from weakening
it.
---
Nitpick comments:
In `@scripts/check_chunk_contract.py`:
- Around line 323-327: Harden the shared _git runner by supplying the configured
subprocess timeout and converting subprocess.TimeoutExpired into ContractError
so callers retain the stable failure surface. Ensure ref-derived Git arguments
are protected with explicit -- separators or conservative validation, including
rev-parse and ls-tree paths, while keeping the fixed argv execution unchanged.
In `@scripts/test_agent_gates.py`:
- Around line 6499-6533: Update
test_machine_chunk_scope_gate_is_mandatory_and_signed_state_bound to parse the
workflow YAML and locate the trusted-materialization loop step, then assert the
required script names, git show command, trusted_checker invocation, PYTHONPATH,
and copy command within that step’s content instead of searching the entire
workflow text. Follow the exact-step matching approach used by the neighboring
test.
In `@scripts/test_check_chunk_contract.py`:
- Around line 191-277: The second select_contract call after the initial
mock.patch.object scope relies on real verify_state_ref execution to produce the
authentication failure. Add a concise comment immediately before that call
explicitly stating that verification is intentionally unpatched and must fail
with “authentication failed”; leave the existing call and assertions unchanged.
- Around line 418-423: Update
test_untracked_case_collision_with_unchanged_head_is_rejected and the related
invalid-UTF-8 filename test to detect whether the filesystem supports the
required case collision and byte-name representation before creating fixtures.
Skip each test when its filesystem capability is unavailable, while preserving
the existing assertions on filesystems that support the scenarios.
- Around line 278-334: Rename test_rejects_stopped_projection_and_blob_mutation
to reflect that it only verifies signed_contract_blob path/blob mutation
rejection. In
test_grandfather_requires_active_exact_cutover_parent_and_prestart, remove the
unused temporary repository setup and related git/file operations, keeping the
in-memory records and require_grandfather assertions intact.
- Around line 336-377: Replace the pre-constructed common patcher tuple in
test_select_rejects_stopped_and_post_cutover_no_schema_starts with
contextlib.ExitStack, entering the shared added_merge_intent and
verify_state_ref patches through the stack for the first assertion. Keep the
test’s existing behavior and separate second assertion setup unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 723b3213-708a-4fa7-88c3-4a5a80d9f7cd
📒 Files selected for processing (24)
.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/STATUS.md.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/chunks/WS-ENG-008-01-machine-checkable-chunk-scope.md.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/chunks/WS-ENG-008-02-scheduled-signed-state-drift-audit.md.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/chunks/WS-ENG-008-03-risk-routed-adversarial-proof.md.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/chunks/WS-ENG-008-04-loop-memory-property-invariants.md.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/chunks/WS-ENG-008-05-authorization-property-invariants.md.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/chunks/WS-ENG-008-06-changed-module-mutation-pilot.md.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/chunks/WS-ENG-008-07-lossless-review-memory-index.md.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/reviews/WS-ENG-008-01-internal-review-evidence.md.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/reviews/WS-ENG-008-01-pr-trust-bundle.md.agent-loop/merge-intents/WS-ENG-008-01.json.agent-loop/policies/definition-of-done.md.agent-loop/policies/repository-engineering-policy.md.agent-loop/templates/CHUNK_CONTRACT.md.github/workflows/agent-gates.ymlAGENTS.mdCONTRIBUTING.mdREADME.mddocs/architecture_lockdown.mddocs/glossary.mdscripts/check_chunk_contract.pyscripts/check_internal_review_evidence.pyscripts/test_agent_gates.pyscripts/test_check_chunk_contract.py
abiorh-claw
self-requested a review
July 26, 2026 09:06
abiorh-claw
approved these changes
Jul 26, 2026
![workstream-loop-memory[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
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.
PR Trust Bundle: WS-ENG-008-01
Chunk
WS-ENG-008-01— Machine-Checkable Chunk ScopeMerge intent:
.agent-loop/merge-intents/WS-ENG-008-01.jsonGoal
Make every post-cutover implementation/specification PR prove its complete Git
delta against the exact machine-readable contract selected by signed start.
Human-approved intent
The approved contract is
.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/chunks/WS-ENG-008-01-machine-checkable-chunk-scope.md.Signed Start Provenance
30191914510bd2203d5e8a972d8afbf833805b92ed70dedee4aimplementation.agent-loop/initiatives/WS-ENG-008-repository-native-sdlc-assurance/chunks/WS-ENG-008-01-machine-checkable-chunk-scope.md2fb0afba71653e7aefcc074079fe98f44051c0681ef5c3bd0bffedec684ae8b6cec2e6affbcb3b21What changed
Design chosen
The signed ledger selects authority; the immutable start blob supplies machine
scope. Agent Gates authenticate generated state, reduce initiative-local events,
compare all base/head/index/worktree paths, and execute only repository-owned
command mappings. After cutover the interpreter and validators come from the
trusted base rather than candidate code. Work already active at cutover may
finish only under its exact signed legacy Allowed-files fence.
Scope control
All 22 implementation files are explicitly listed by the signed human contract
and the new bootstrap machine block. No product, API, database, migration,
authorization, payment, signing, start/cancel, branch-protection, secret,
dependency, or coverage behavior changed.
Final reconciliation found no overlap with active REV PR #195 or the concurrent
AUTH/ART work. Stale PR #149 overlaps the evidence checker and Agent Gate tests;
it has no ENG-008 authority and must rebase and satisfy the post-cutover scope
gate independently.
Acceptance criteria proof
WS-ENG-008-02with explicit start required.Tests/checks run
python3 scripts/check_chunk_contract.py --base-ref origin/main --head-ref HEAD --state-ref origin/automation/loop-memory python3 scripts/test_check_chunk_contract.py python3 scripts/test_agent_gates.py python3 scripts/update_post_merge_memory.py validate-merge-intent --repository-root . --base-ref origin/main python3 scripts/check_internal_review_evidence.py python3 scripts/check_markdown_links.py python3 scripts/check_stale_workstream_wording.py ruff check scripts/check_chunk_contract.py scripts/check_internal_review_evidence.py scripts/test_check_chunk_contract.py scripts/test_agent_gates.py git diff --check origin/main...HEADResult: all passed on the reviewed implementation plus evidence-only publication.
Test delta
CI integrity
continue-on-error, path exclusion, unpinned action, permission expansion,credential persistence, package-script bypass, or test weakening was added.
Reviewer results
Reviewed code SHA:
1ef5c3bd0bffedec684ae8b6cec2e6affbcb3b21Reviewed at:
2026-07-26T07:48:30ZReviewer run IDs:
ci02b_lane_runner,ci02b_cr_arch,ci02b_cr_ci,ci02b_cr_docs,ci02b_cr_reuse,ci02b_cr_test_deltaAll nine required tracks passed. Architecture and reuse recorded only Low-risk
future consolidation opportunities; there are no blocking findings.
Remaining risks
Some validated-ledger, merge-intent, and reviewer-name parsing remains locally
duplicated. Consolidating authority-sensitive helpers requires its own reviewed
contract and must not delay this closed enforcement cutover.
Human review focus
agent-gates.yml.check_chunk_contract.py.External review
Human ownership
The PR remains stopped for explicit review and merge approval. Its merge will
not start
WS-ENG-008-02.Summary by CodeRabbit
New Features
Documentation
Tests