Allow necessary in-repo path expansion

[justin808]

Why

Workers treated an incomplete precomputed file list like a permission boundary,
which could strand otherwise authorized and validated work. This change lets a
lane add evidence-backed paths inside the target repository when they are
necessary for the approved goal or required validation, while preserving every
material stop and preventing parallel editors from racing onto the same newly
discovered path.

What changed

• Treat owned paths and execution envelopes as coordination controls, not user
  authorization boundaries.
• Require an active typed reservation for every expanded path until it is
  reflected in the verified map or cancelled. A sole editor continues without
  user approval only after the coordinator records the reservation and refreshed
  preflight accepts it.
• Make multi-editor expansion a typed blocked request: heartbeat and Lane Card
  signal the coordinator, which records the reservation and reruns preflight
  before resuming work.
• For max-one serialization, require holder release, durable requester unblock,
  a fresh accepted preflight, absence from held lanes, and eligibility when a
  launch or relaunch is required; acceptance alone never authorizes resume.
• Add optional expansion_path_reservations v1 records to batch preflight.
  Collision and risky-capacity decisions now use verified PR paths plus active
  reservations; overlap requires explicit max-concurrency-one serialization.
• Keep all material stops unconditional: contradiction, unrelated work,
  destructive actions, trust/repository boundaries, external effects,
  consequential ambiguity or judgment, policy/security changes, collisions,
  and weakened verification.
• Qualify the standalone model-routing and context summaries so necessary
  in-repository path discovery is not confused with material semantic scope
  growth.
• Align compact GMCC-v4 with the existing canonical completion contract by
  treating failed current-head CI/configured reviewers as NOT COMPLETE. This is
  a fail-closed mirror correction, not a new completion policy.
• Mirror the exact full contract across five workflow/skill/doc surfaces, retain
  the compact contract across three worker prompts, and register deterministic
  regression coverage plus an Unreleased changelog entry.

Fixes #408

How to review and verify

1. Review the canonical worker rule in workflows/pr-processing.md and its exact
   full mirrors in plan-pr-batch, pr-batch, triage, and docs/pr-batch-skills.md.
2. Review batch-plan-preflight's optional reservation parser and effective map:
   exact bindings, canonical path, durable evidence, stale-record rejection,
   collision/capacity union, and max-one serialization.
3. Review file-expansion-contract-test.rb and
   batch-plan-preflight-test.rb, then confirm bin/validate registers the
   focused contract and CHANGELOG.md records the behavior.

Agent details

Commands and results

• TDD RED: initial missing full/compact/restatement surfaces and stop wording
  produced focused failures before each correction.
• TDD RED for the final sole-editor and summary correction:
  • file-expansion contract: 4 runs, 6 assertions, 4 expected failures for
    universal-reservation, worker-restatement, compact, and summary semantics.
• ruby skills/pr-batch/bin/file-expansion-contract-test.rb — 4 runs, 66
  assertions, 0 failures.
• ruby skills/plan-pr-batch/bin/batch-plan-preflight-test.rb — 72 runs, 652
  assertions, 0 failures.
• ruby skills/pr-batch/bin/goal-completion-contract-test.rb — 103 runs, 1,346
  assertions, 0 failures.
• Prompt-size guard — passed with 303/302/302 characters of Codex headroom.
• Manual absent-versus-empty reservation replay — byte-identical accepted
  result.
• git diff --check origin/main...HEAD — passed.
• Maker final-byte bin/validate — exit 0; PASS agent-workflows validation;
  RuboCop 138 files/no offenses.
• Independent exact-head QA focused replay — file-expansion 4/66,
  batch-plan-preflight 72/652, goal-completion 103/1,346, prompt headroom
  303/302/302, diff check, Ruby syntax, and direct RuboCop all passed.
• Exact-head qa-evidence v2 replay — SATISFIED with no release-blocking code
  finding.
• A standalone recursive codex review attempt was invalid, terminated, and
  excluded. Independent QA covered correctness, type design, security,
  compatibility, release process, test quality, and documentation consistency.

Exact-head and replay evidence

• Base and merge-base: 3a42c2fe06c956514faf993eb081f5efa6e4440f
• Tested head: 76cca902fa4f8ecd7da0c7882d502603072aef85
• Diff: 14 files, 1,187 insertions, 54 deletions
• Binary diff SHA-256:
  693591f51d70ec4e2d31efd84e191ba129453376292365080f5c7b54799eba67
• User-visible UI change: no; exact interaction route: none.

QA Evidence

• QA lane: /root/qa_408_76cca; independent read-only checker; lifecycle
  complete at detached exact head 76cca902.
• Scope checked: issue Default to allowing necessary file expansion within an approved worker goal #408; full exact base-to-head diff; scalar and rename
  reservation lifecycle/collision semantics; five full and three compact
  mirrors; worker restatement; current-base Explain no-auto-merge verdicts in plain English #431 overlap; security,
  compatibility, documentation, changelog, and release-process lenses.
• Findings: no release-blocking code finding. The secondary GMCC failed-state
  mirror correction is disclosed in What changed and the Decision log; its
  current-head review thread is dispositioned before merge.
• QA status: satisfied; release-blocking status: clear.

Reservation type design

• One exact tagged v1 record binds batch, dependency plan, lane, and wave, with
  one canonical path, a known single-line reason, and a durable URI reference.
• Presence is the only active state; omission means cancellation. There is no
  independent status boolean or new dependency type.
• Malformed, UNKNOWN, noncanonical, duplicate, identity-mismatched,
  completed-lane, and already-reflected reservations reject.
• Effective collision/capacity maps are derived from verified paths plus active
  reservations. A reserved-path overlap cannot be waived by a typed edit edge;
  it requires an explicit shared max-concurrency-one serialization group.
• Absent and empty reservation fields preserve existing accepted inputs.

Coordination and reviewer telemetry

• Coordination: unavailable — repository seam sets coordination_backend to
  n/a; this batch ran serially with durable coordinator-owned state.
• Maker: /root/issue_408_rebase_67714 (Sol/high preferred; observed
  codex/UNKNOWN/UNKNOWN).
• Checker: /root/qa_408_76cca (Sol/xhigh preferred; observed
  codex/UNKNOWN/UNKNOWN).
• Typed dependency edge issue-408 -> qa-408 is satisfied at the tested head;
  both lane states are complete.
• Hosted Lint, Validate, and Claude review are successful; CodeRabbit is terminal
  under the repository rate-limit waiver; the current-head disclosure thread is
  dispositioned before merge.

Review dispositions

• All actionable findings are fixed, including complete mirrors, worker stops,
  exact UNKNOWN wording, parallel collision handling, unconditional stops,
  typed requested-path reservations, sole-editor reservation visibility,
  blocked/Lane Card signaling, standalone summary qualifiers, max-one eligible
  resume, and granular malformed-lane diagnostics.
• The trust-signoff request is declined as already satisfied by trusted
  maintainer-authored issue Default to allowing necessary file expansion within an approved worker goal #408 and this explicit batch authorization. This
  does not waive trust, security, or external-effect stops.
• Model-routing and CONTEXT.md now distinguish material semantic scope growth
  from evidence-backed necessary in-repository path discovery and link the
  canonical workflow.
• The security hardening note is already covered by the repository’s explicit
  untrusted-content posture. The remaining summary is duplicate synthesis; exact
  mirror duplication is intentional and contract-tested.

Decision log

• Necessary tests, validator registration, changelog, triage, and user docs are
  required in-repository acceptance/release surfaces, not new product scope.
• Main moved repeatedly during implementation; each range was inspected before
  rebasing. A transient trusted-base merge-mode regression reproduced on main
  and was repaired upstream by Change merge mode from queue to direct #415 before the final stable base.
• Advisory record/refresh was insufficient for parallel workers. The final
  design makes requested paths typed collision inputs and signals coordinator
  wait state without inventing a second lifecycle or dependency model.
• The compact GMCC-v4 line now includes failed across the workflow, pr-batch,
  triage, prompt-headroom fixture, and goal-completion contract test. This is an
  adjacent fail-closed mirror correction: the pre-existing canonical expanded
  contract already required current-head failures to remain NOT COMPLETE, so no
  new completion policy or broader merge authority is introduced.

Merge confidence

• Implementation confidence: high — exact acceptance has deterministic mirror
  and schema tests plus independent exact-head full validation and type review.
• Ordinary merge readiness: READY — exact-head hosted CI and configured review
  agents are green, the post-cutoff review scan is empty, and no thread is
  unresolved.
• Autonomous merge verdict: human-approved-for-current-head at
  76cca902fa4f8ecd7da0c7882d502603072aef85; triggered gates are
  architectural-product-judgment, autonomous-merge-policy-change,
  changed-lines-limit, commit-count-limit, and public-compatibility.
  Rollback is code-only and established. The exact sorted gate set is approved
  by justin808 in the durable current-head decision at
  Allow necessary in-repo path expansion #411 (comment).
• Remaining UNKNOWNs: none; no policy or authority blocker remains while the
  exact head, base, ordinary gates, and durable decision stay unchanged.
• Rollback: revert the PR merge commit (or the eleven PR commits before merge);
  there is no data migration, deployment, permission grant, or external side
  effect.

Audit receipts

Completed-batch audit

Status: Clean — no outstanding findings or follow-ups. Durable receipt.

[coderabbitai]

Important

Approval pending

CodeRabbit has no unresolved comments, but it has not reviewed the latest commit.

Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.

• 🔍 Trigger review

Walkthrough

The workflow permits evidence-backed repository path expansion within an approved goal. Typed reservations update preflight maps, collision checks, capacity calculations, and multi-editor coordination. Material scope, trust, security, external-effect, ambiguity, and verification risks still stop execution.

Changes

Repository path expansion

Layer / File(s)	Summary
Reservation validation and collision handling skills/plan-pr-batch/bin/batch-plan-preflight, skills/plan-pr-batch/bin/batch-plan-preflight-test.rb	Preflight validates typed reservations, updates effective file-touch maps, detects collisions, enforces serialization, and includes reservations in risky-capacity checks. Tests cover valid, stale, duplicate, malformed, conflicting, and overlapping reservations.
Worker expansion coordination CONTEXT.md, skills/plan-pr-batch/SKILL.md, skills/triage/SKILL.md, workflows/pr-processing.md, docs/pr-batch-skills.md, skills/pr-batch/SKILL.md	Worker rules allow evidence-backed necessary paths, require durable reservations and refreshed preflight, and pause multi-editor lanes until coordinator processing completes.
Workflow contract and regression coverage docs/agent-workflows-model-routing.md, skills/pr-batch/bin/file-expansion-contract-test.rb, bin/validate, CHANGELOG.md	Mirrored guidance distinguishes necessary path discovery from material scope growth. Contract tests validate the shared rules and run through the validation script.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• shakacode/agent-workflows#95: Extends the shared Lane Card workflow for path-expansion requests.
• shakacode/agent-workflows#143: Modifies worker envelope and coordinator stop conditions.
• shakacode/agent-workflows#279: Introduces related preflight collision and capacity validation.

Suggested reviewers: ihabadham

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address issue #408 through documented path expansion, reservations, refreshed preflight checks, stop conditions, mirrored guidance, and regression tests.
Out of Scope Changes check	✅ Passed	The changelog, documentation, workflow, preflight logic, tests, and validation updates directly support the linked issue objectives.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: allowing necessary path expansion within the target repository.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jg-codex/issue-408-file-expansion

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Review Summary

This PR relaxes the worker path-expansion policy (owned paths / envelope are coordination controls, not a permission boundary) and adds a contract test (skills/pr-batch/bin/file-expansion-contract-test.rb) that keeps three surfaces — workflows/pr-processing.md, skills/pr-batch/SKILL.md, skills/plan-pr-batch/SKILL.md — in exact sync. The core wording (default-allow + explicit stop list) is clear and the test approach (exact-text + compact-line parity) matches the repo's existing pattern (goal-completion-contract-test.rb).

Main finding: incomplete mirror set leaves contradictory old-policy text in place

goal-completion-contract-test.rb (the sibling contract for the same kind of "worker stop conditions" policy) treats skills/triage/SKILL.md and docs/pr-batch-skills.md as required mirror surfaces. This PR's new file-expansion-contract-test.rb does not include them, and both still contain the old, stricter contract that this PR is meant to replace:

• skills/triage/SKILL.md:205 — "...returns control before further edits on contradictory evidence, ambiguous criteria, scope or risk growth, weakened verification, or consequential judgment." (skills/triage/SKILL.md is an operative skill that generates $pr-batch prompts, so this text feeds directly into worker instructions.)
• docs/pr-batch-skills.md:212 — "...contradictory evidence, ambiguity, scope/risk growth, weakened verification, or consequential judgment."
• docs/agent-workflows-model-routing.md:98 and :161 — "...evidence contradicts the diagnosis, scope or blast radius grows, a high-risk boundary appears, verification weakens, or consequential judgment is required." (same unrevised phrasing, twice)

None of these mention the new path-expansion allowance, and "scope or risk growth" / "scope or blast radius grows" read as blanket stop triggers — directly contradicting the new rule that a necessary added path is not scope growth. Since skills/triage/SKILL.md actually generates the prompts handed to workers, a triage-originated batch would still instruct workers under the old, stricter contract while a plan-pr-batch/pr-batch-originated batch would use the new, looser one — an inconsistency between entry points for the same underlying policy.

Given the existing precedent of goal-completion-contract-test.rb covering these same files for an analogous contract, this looks like an incomplete rollout rather than an intentional scope limitation. Suggest either updating these three locations to match the new policy (and adding them to the contract test's SURFACES), or explicitly documenting why they're excluded.

Secondary/minor notes (see inline comments)

• workflows/pr-processing.md also restates the same policy a third time, in the "When worker subagents are explicitly authorized" section (~line 1709-1719), as a paraphrase rather than the exact canonical text. It isn't covered by the new contract test, so it can drift independently, and it currently omits two stop conditions that are explicit in the canonical list ("adds unrelated work", "destructive or difficult-to-reverse action").
• The compact Workers: line change quietly swaps "unverifiable facts are UNKNOWN" for "unverified facts are UNKNOWN" — a semantic change (can't-be-checked vs. not-yet-checked) not called out in the PR description/changelog. Worth confirming intentional.

Other notes (not blocking)

• bin/validate wiring and CHANGELOG.md entry look correct and consistent with the described change.
• The new contract test's third assertion block (assert_includes COMPACT_WORKER_CONTRACT, "...") checks the string constant against itself rather than production text — it doesn't add real coverage beyond the assert_equal above it, but it's harmless.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b54036249

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review

The core contract change (default-allow necessary in-repo path expansion, with an explicit stop list) is textually identical across all five full-contract surfaces (workflows/pr-processing.md, skills/plan-pr-batch/SKILL.md, skills/pr-batch/SKILL.md, skills/triage/SKILL.md, docs/pr-batch-skills.md) and the three compact Workers: lines — verified by diffing against skills/pr-batch/bin/file-expansion-contract-test.rb's expected constants, which match verbatim. The previously-flagged gaps (missing triage/docs mirrors, incomplete worker-subagent stop-list restatement in workflows/pr-processing.md ~L1714-1734, and the unverifiable→unverified wording risk) all look correctly resolved in the current head.

One remaining inconsistency, not addressed by this PR: docs/agent-workflows-model-routing.md:98 and :161, and CONTEXT.md:131, still state the pre-#408 blanket rule — "Every worker stops ... when evidence contradicts the diagnosis, scope or blast radius grows, ..." / "Contradictory evidence, ambiguity, scope growth, ... returns control to the coordinator" — with no carve-out for the new "a missing/necessary path alone is not scope growth" exception. The PR description's decision log says these edits were considered and declined ("they do not make file enumeration a permission boundary and remain compatible with the canonical rule"), but the text itself doesn't make that distinction — an agent reading only the model-routing guide or CONTEXT.md's "Worker execution envelope" glossary entry would still see unqualified "scope growth ⇒ stop," which is exactly the behavior this PR is trying to relax elsewhere. Since docs/agent-workflows-model-routing.md is explicitly meant to be read alongside the canonical workflow, worth either mirroring the exception there too or adding an explicit forward-reference ("see the path-expansion exception in pr-processing.md") so the two documents can't be read as contradictory in isolation.

Design/security note (not a blocker, but worth surfacing given the review focus): this policy lets a worker self-determine "repository evidence" as sufficient grounds to expand its own file-editing authority without user approval. The stop list (destructive/irreversible actions, secrets/permissions/deployment/billing effects, security/privacy/compliance changes, trust-boundary crossings) covers the obvious high-risk categories well, but there's no explicit guidance to treat repository content itself (comments, issue text, embedded instructions) as untrusted input when it's the "evidence" driving the expansion decision — a plausible prompt-injection vector in a batch/triage context where the target repo's content isn't fully trusted. Given the enumerated stops are fairly comprehensive, this is more a suggestion for a future hardening pass than a defect in this PR.

Everything else — bin/validate wiring, the CHANGELOG.md entry, and the new test file's structure/style — is consistent with the existing repo conventions.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b78651313a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[justin808]

Addressed the Claude review summary on final head e0136ff. The canonical allowance now has five exact full surfaces and three compact mirrors, including triage and public documentation; the focused contract passes 3/43. The worker restatement and exact UNKNOWN wording are fixed as noted in their threads. The suggested model-routing and CONTEXT edits were reviewed and declined: those files describe genuine semantic scope/risk growth at a higher level and do not treat a file enumeration as a permission boundary, so they remain compatible with the canonical rule. The optional self-assertion concern is also closed by production-bound full/compact/worker assertions. Independent full bin/validate passed.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e0136ff9cb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review summary

This PR loosens the "owned paths" contract so a worker can add evidence-backed, in-repository paths to its edit scope without user approval, replacing a previous hard stop for any out-of-envelope edit. The same paragraph is mirrored verbatim across workflows/pr-processing.md, all three SKILL.md files, and docs/pr-batch-skills.md, and pinned by a new literal-string regression test (skills/pr-batch/bin/file-expansion-contract-test.rb) plus a bin/validate wire-up — consistent with this repo's existing "exact mirrored contract" pattern, and the five surfaces do match byte-for-byte.

Two substantive points left inline:

1. Self-assessed scope expansion (workflows/pr-processing.md:768-782 and mirrors) — the worker itself judges whether an added path is "reasonably necessary," and that judgment alone now unlocks editing outside the originally authorized scope with no user checkpoint. The only backstop is a semantic list of stop conditions the same model has to recognize and apply against its own actions, which widens the prompt-injection surface (repo content or issue/PR text could argue a sensitive path is "necessary"). This is presumably the intended fix for Default to allowing necessary file expansion within an approved worker goal #408, but it's a real trust-boundary change worth explicit maintainer sign-off rather than treating it as a wording tweak.

2. New "expansion request" pause state isn't wired into existing signaling (workflows/pr-processing.md:1727-1731) — every other worker-waits-on-coordinator case in this doc goes through heartbeat --status blocked and/or a Lane Card emission trigger. The expansion-request pause isn't added to the Lane Card trigger list (workflows/pr-processing.md:1745) or the blocked-heartbeat guidance, so there's no defined channel for a separate coordinator process to discover a paused worker and act on the request — as written it could stall indefinitely.

Minor: the contract paragraph is now duplicated six times (5 doc surfaces + 1 Ruby string constant in the test). That's self-enforcing via the exact-match test, so not blocking, but it does mean every future wording change must touch six places in lockstep.

No issues found with bin/validate/CHANGELOG.md wiring — both are correctly and minimally updated.

[justin808]

Current-head dispositions for the later review summaries: the substantive pause-signaling and requested-path reservation concerns are fixed on ff99b3a with blocked lifecycle/heartbeat/Lane Card signaling and typed reservation union in preflight. The model-routing and CONTEXT recommendation remains declined because those summaries describe genuine material semantic scope/risk growth and defer to the canonical workflow; the security note is already covered by the repository explicit untrusted-content posture and unconditional trust/security stops. The trust-signoff request is already satisfied by trusted maintainer-authored issue #408 and this explicit batch authorization. Exact contract duplication is intentional and regression-tested. Independent type review, security preflight, focused tests, and full bin/validate are clean.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ff99b3a431

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review

I independently re-verified the current head (checked-out merge of c4b87520 + ff99b3a4) rather than relying on the PR description's self-reported evidence. Summary: the batch-plan-preflight reservation/collision logic is correct and the previously-raised issues from earlier automated review rounds (missing triage/docs mirrors, incomplete worker-subagent stop-list restatement, expansion-pause not wired into Lane Card/heartbeat signaling) are genuinely fixed in this diff, not just claimed fixed — I traced each one through the code.

Verified correct: skills/plan-pr-batch/bin/batch-plan-preflight

• valid_expansion_path_reservation? / validate_expansion_path_reservations fail closed on malformed shape, UNKNOWN fields, foreign batch/plan/lane/wave identity, non-canonical paths, and multiline reasons.
• Staleness handling (completed_lane_ids.include? or already-reflected in the lane's own verified paths) is correct and only checks the reservation's own lane, which is the right scope.
• effective_file_touch_maps only adds reservation paths into the collision/capacity computation — it never removes or masks real verified paths, so reservations can only make collision detection stricter, never weaker.
• The reservation_collision branch in validate_collisions correctly forces max-1 serialization (no edit-edge override) for reserved-path overlaps, matching the stated design intent, and is exercised by test_typed_edit_edge_does_not_replace_max_one_serialization_for_reserved_paths.
• Backward compatibility: omitting expansion_path_reservations (nil) short-circuits to [[], []] before touching plan.fetch("lanes"), so existing callers that never send the field are unaffected; an explicit empty array produces the same result.
• The top-level rescue KeyError, NoMethodError, TypeError, ArgumentError fail-closed wrapper (pre-existing) covers the new code path too, so malformed input here can't crash into an unhandled exception — it rejects instead.

No correctness bugs found in this file or its test additions.

Still open: docs/agent-workflows-model-routing.md and CONTEXT.md were not updated and now contradict the new policy

This was raised in an earlier review round on this PR and the response was to decline the edit ("those files describe genuine material semantic scope/risk growth ... and remain compatible with the canonical rule"). Having read the actual text, I don't think that holds up:

• docs/agent-workflows-model-routing.md:98 — "Every worker stops without editing further and returns to the coordinator when evidence contradicts the diagnosis, scope or blast radius grows, a high-risk boundary appears, verification weakens, or consequential judgment is required."
• docs/agent-workflows-model-routing.md:160 — same sentence, repeated verbatim in the "Conservative Claude Profile" section.
• CONTEXT.md:131 (glossary entry for Worker execution envelope) — "Contradictory evidence, ambiguity, scope growth, high-risk judgment, or weakened verification returns control to the coordinator instead of authorizing worker re-planning."

All three are unqualified — they don't distinguish "file enumeration" from "semantic scope growth," they just say "scope growth ⇒ stop." That's exactly the blanket rule this PR is relaxing everywhere else (workflows/pr-processing.md, all three SKILL.md files, docs/pr-batch-skills.md now all say "a missing path alone is not material scope growth"). An agent or reviewer consulting only the model-routing guide or the CONTEXT.md glossary — both of which are explicitly meant to be read alongside the canonical workflow — would still see an unconditional stop-on-scope-growth rule with no path-expansion carve-out. Given how much of this PR's design is about keeping worker-facing contract text in exact, tested sync, leaving these two out (and not test-covered like triage/docs/pr-batch-skills.md are for this same contract) looks like a real gap rather than a deliberate distinction. Suggest either adding the same one-clause exception to these three locations, or adding an explicit forward-reference so they can't be read standalone as contradicting the new default-allow rule.

Design note (already discussed at length in this PR's review history, flagging for visibility rather than as a new blocker)

The core mechanism change is that a worker can now expand its own edit scope into new in-repository paths based on its own judgment of "reasonably necessary," with no user checkpoint for the sole-active-editor case. The retained stop list (trust-boundary crossing, destructive/irreversible actions, secrets/permissions/deployment/billing, security/compliance policy changes) is thorough on paper, but enforcement is entirely prompt-based — nothing in batch-plan-preflight or elsewhere validates that a given reason actually reflects genuine necessity, only that the record is well-formed and collision-safe. That's an inherent limitation of a natural-language contract rather than a defect in this diff, but given the PR's own decision log already treats this as requiring explicit authorization from issue #408 rather than a routine merge, it's worth the maintainer explicitly confirming they're comfortable with a prompt-enforced-only boundary before this lands, rather than treating the extensive self-reported QA evidence in the PR description as a substitute for that sign-off.

[justin808]

Current-head disposition for the Claude review summary at #411 (comment):

• Fixed the standalone documentation contradiction in a196e3c: both model-routing summaries and the CONTEXT glossary now qualify material semantic scope growth and link the canonical necessary-path exception. Separate contract assertions cover all three summary sites.
• The reservation implementation praise is confirmed and non-actionable.
• The prompt-enforced trust-boundary design note is acknowledged but does not require another code change: issue Default to allowing necessary file expansion within an approved worker goal #408 is explicit maintainer-authored authorization from a repository-trusted MEMBER, while the contract continues to stop for trust/security boundaries, external effects, destructive actions, policy changes, ambiguity, collisions, and weakened verification.

Independent exact-head QA passed file-expansion 4/54, preflight 60/501, goal mirrors 99/1277, prompt headroom >=300, and full bin/validate.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@skills/plan-pr-batch/bin/batch-plan-preflight`:
- Around line 321-342: Update validate_expansion_path_reservations to build
lanes_by_id using a non-raising lane ID lookup, preserving the structured
lane-id-invalid-or-duplicate rejection when a lane omits id. In
skills/plan-pr-batch/bin/batch-plan-preflight-test.rb lines 545-567, add a
regression case combining a planned lane without id and
expansion_path_reservations, asserting rejection without invalid-envelope.

🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 87b4053a-74d5-4eb1-93f2-920643959d76

📥 Commits

Reviewing files that changed from the base of the PR and between c4b8752 and a196e3c.

📒 Files selected for processing (12)

• CHANGELOG.md
• CONTEXT.md
• bin/validate
• docs/agent-workflows-model-routing.md
• docs/pr-batch-skills.md
• skills/plan-pr-batch/SKILL.md
• skills/plan-pr-batch/bin/batch-plan-preflight
• skills/plan-pr-batch/bin/batch-plan-preflight-test.rb
• skills/pr-batch/SKILL.md
• skills/pr-batch/bin/file-expansion-contract-test.rb
• skills/triage/SKILL.md
• workflows/pr-processing.md

[claude]

Review summary

Reviewed the diff (12 files, +721/-40): the batch-plan-preflight Ruby validator/tests, and the mirrored prose contract added across docs/pr-batch-skills.md, skills/plan-pr-batch/SKILL.md, skills/pr-batch/SKILL.md, skills/triage/SKILL.md, and workflows/pr-processing.md. Left two inline comments; summary below (note: I could not execute Ruby in this sandbox, so the code-level point is a static-read finding, not empirically confirmed — please verify).

Main points:

1. Trust-boundary/design concern (most important): The new "necessary in-repository path expansion" mechanism replaces a previous hard stop (unlisted path → ask the user) with worker self-attestation. The expansion-path-reservation record's reason only needs to be a nonempty single line, and evidence_ref is only checked for URI shape (scheme+host), not that it points to real, verifiable evidence. batch-plan-preflight validates shape, staleness, and collision/serialization safety, but nothing independently verifies the claimed necessity itself. Since these workers routinely process untrusted PR/issue content, this seems like a real (if perhaps intentional/accepted) widening of what a worker can touch without a human ever approving it — see inline comment on docs/pr-batch-skills.md:211.

2. Possible crash on malformed input: validate_expansion_path_reservations in skills/plan-pr-batch/bin/batch-plan-preflight dereferences stage_dependency_plan["id"] without checking it's a Hash first, unlike validate_stage_dependencies elsewhere in the same file which guards defensively before failing closed. If expansion_path_reservations is supplied while stage_dependency_plan is missing/malformed, this looks like it would raise NoMethodError and crash rather than return a structured rejected result — see inline comment at line 340. (This mirrors a pre-existing similar pattern in validate_lane_lifecycle_states, so it's not unique to this PR, but the new code repeats rather than fixes it.)

3. Maintainability/duplication: The same ~250-word contract paragraph (PATH_EXPANSION_DEFAULT/_EXAMPLES/_STOPS in the new test) is mirrored byte-for-byte across five prose surfaces plus hardcoded as literal strings in file-expansion-contract-test.rb for exact-match assertions. This is clearly intentional (the PR explicitly aims for exact mirrors, and the test enforces it), but it means every future wording tweak to this policy requires six synchronized edits, and the test only proves textual parity, not that the underlying behavior it describes actually holds at runtime.

What looked solid: The collision/reservation logic in batch-plan-preflight itself (staleness rejection, duplicate detection, reservation-vs-verified-path collision requiring explicit max-1 serialization rather than a typed edit edge) traced correctly against its test cases on a careful static read, and the doc anchor links (docs/pr-batch-skills.md#implementation-batch-planning-flow) resolve correctly.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a196e3c4de

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bad1733caa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review: Allow necessary in-repo path expansion

Overview

This PR replaces the "workers may only touch owned/pre-declared paths" rule with a coordinated-expansion model: a worker may touch an evidence-backed, necessary-but-undeclared in-repo path without stopping for user approval, provided it's tracked through a new typed expansion-path-reservation v1 record that batch-plan-preflight validates and folds into collision/capacity checks. The prose contract is mirrored verbatim across five docs/skills surfaces and a compact one-liner across three worker-prompt surfaces, with a new file-expansion-contract-test.rb enforcing that mirroring, plus substantial new coverage in batch-plan-preflight-test.rb.

Code quality (skills/plan-pr-batch/bin/batch-plan-preflight)

• The Ruby changes are careful and consistent with existing conventions: they reuse canonical_repository_relative_path? (blocks .., absolute paths, \, empty segments) and durable_state_ref? (requires scheme+host) rather than reinventing validation, and add a sensible known_single_line_text? guard against embedded \r\n\0 in the reason field.
• effective_file_touch_maps correctly dups the paths array per lane before mutating it, so it doesn't corrupt the original file_touch_map structure.
• Staleness handling (reservation whose lane completed, or whose path is already reflected in the verified map) and duplicate-reservation detection look correct and are exercised by tests.
• Left two inline notes on edge cases worth a second look: an unconditional call that bypasses the same lane_ids_valid gating used by a sibling validator (fails closed either way, just loses a more specific violation code in a narrow combination), and an all-or-nothing collision decision when a lane pair shares both a reservation-derived path and an unrelated already-verified path.

Test coverage

• batch-plan-preflight-test.rb additions are thorough: optional/disjoint acceptance, same-wave overlap requiring explicit max-1 serialization (edit edges alone are correctly rejected for reserved paths), collision against a pre-existing verified map, a sole-editor reservation protecting a path when a second lane later joins, the full blocked→eligible max-one lifecycle transition, ~10 malformed/invalid-shape cases, and risky-capacity union with active reservations.
• file-expansion-contract-test.rb verifies exact byte-for-byte mirroring of the new contract paragraph across the 5 full surfaces and the compact worker line across the 3 compact surfaces, and that CONTEXT.md/docs/agent-workflows-model-routing.md each qualify "scope growth" exactly twice. bin/validate correctly registers the new test.

Design/maintainability observations (not blocking)

• The ~400-word contract paragraph is now duplicated verbatim in workflows/pr-processing.md, skills/plan-pr-batch/SKILL.md, skills/pr-batch/SKILL.md, skills/triage/SKILL.md, and docs/pr-batch-skills.md. This matches the repo's existing pattern of inlining full prompt text per surface (presumably so each agent context is self-contained), and the new contract test guards drift, but every future wording tweak is now a 5-file edit. Worth confirming this is the accepted tradeoff going forward.
• The compact mirror line (Workers:paths=coord!=permission;path+resv;multi=>coord;...) is very dense shorthand. Given there's an explicit prompt-size budget driving this compression, that's presumably intentional, but it's worth double-checking a worker model can reliably unpack path+resv;multi=>coord into the full reservation/heartbeat/Lane Card workflow described in the full contract.
• Policy-level note for the maintainer rather than a code issue: this changes the default from "stop and ask the user before touching an undeclared path" to "proceed if the worker judges the path reasonably necessary, coordinate via reservation." The material-stop list (contradiction, destructive action, trust boundary, etc.) is unconditional and unchanged, but none of those stops specifically catches gradual, individually-justifiable scope creep across several "necessary" path additions in one lane. That may be perfectly acceptable given the coordination/reservation trail this PR adds for auditability — just flagging it as a deliberate trust-boundary shift worth an explicit sign-off given the size of this change.

Security

No new external effects, secrets, or network/shell calls. Input handling for the new expansion_path_reservations array is fail-closed and reuses existing path/URI validators, so no path-traversal or injection concerns.

Overall: solid, well-tested implementation of a clearly-scoped feature; the two inline notes are minor robustness/policy points rather than blocking bugs.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

skills/plan-pr-batch/bin/batch-plan-preflight (1)

352-354: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not clear a reservation from planned path evidence.

The stale check treats any matching paths entry as reflected. validate_file_touch_maps accepts planned-path-evidence without source == "verified", but the workflow requires the reservation until the verified file-touch map reflects the path. A planned evidence record can therefore remove the reservation before verification. Require a verified lane map for this check.

Proposed fix

-      reflected_paths = maps.is_a?(Hash) && maps[lane_id].is_a?(Hash) ? maps[lane_id]["paths"] : nil
+      lane_map = maps.is_a?(Hash) ? maps[lane_id] : nil
+      reflected_paths =
+        lane_map.is_a?(Hash) && lane_map["source"] == "verified" ? lane_map["paths"] : nil

🤖 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 `@skills/plan-pr-batch/bin/batch-plan-preflight` around lines 352 - 354, Update
the stale reservation check in the preflight flow around lane_id and
reflected_paths so a matching path only counts when maps[lane_id] is a verified
lane map with source equal to "verified". Do not treat planned-path-evidence
entries as reflected; preserve the completed_lane_ids check and existing
path-matching behavior for verified maps.

🤖 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.

Outside diff comments:
In `@skills/plan-pr-batch/bin/batch-plan-preflight`:
- Around line 352-354: Update the stale reservation check in the preflight flow
around lane_id and reflected_paths so a matching path only counts when
maps[lane_id] is a verified lane map with source equal to "verified". Do not
treat planned-path-evidence entries as reflected; preserve the
completed_lane_ids check and existing path-matching behavior for verified maps.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 368a138a-a829-4d78-9225-fbc7001e4a91

📥 Commits

Reviewing files that changed from the base of the PR and between a196e3c and bad1733.

📒 Files selected for processing (9)

• CHANGELOG.md
• docs/pr-batch-skills.md
• skills/plan-pr-batch/SKILL.md
• skills/plan-pr-batch/bin/batch-plan-preflight
• skills/plan-pr-batch/bin/batch-plan-preflight-test.rb
• skills/pr-batch/SKILL.md
• skills/pr-batch/bin/file-expansion-contract-test.rb
• skills/triage/SKILL.md
• workflows/pr-processing.md

🚧 Files skipped from review as they are similar to previous changes (5)

• CHANGELOG.md
• docs/pr-batch-skills.md
• skills/plan-pr-batch/bin/batch-plan-preflight-test.rb
• skills/triage/SKILL.md
• workflows/pr-processing.md

[justin808]

Current-head address-review closeout for 18a7a35222e3c24e418f30f09533855db0444dfb, scanning since the previous summary at 2026-08-11T17:16:33Z.

Mattered and fixed:

• Typed rename reservations: directory renames now use expansion-rename-reservation with canonical old/new endpoints, and only that form receives ancestor/descendant collision handling. Regression coverage includes shape, reflection, and collision behavior.
• Malformed stage-plan diagnostics: the stage dependency plan id is extracted safely and specific accumulated violations survive malformed-stage-plus-reservation input. Regression coverage added.
• GMCC-v4 failure handling: both compact mirrors now include failed among current-head CI/configured-reviewer blockers, with exact-mirror contract coverage.

Optional and fixed:

• Replaced the Unicode down-arrow shorthand with ASCII verify-down across compact mirrors for portability.

Discussed and dispositioned without code change:

• sole active editor is evaluated through coordinator-owned lifecycle, active-claim, and collision state followed by preflight replay, not worker self-assessment. Defining it as only one state: active record would miss claimed or blocked editors that still require coordination.
• Omitted reservations cannot be inferred mechanically by preflight. This is the acknowledged trust-based workflow boundary: preflight validates submitted reservations, while coordinator record/refresh/replay and unconditional collision, UNKNOWN, material-risk, trust, and external-effect stops remain.

Resolved threads:

• Allow necessary in-repo path expansion #411 (comment)
• Allow necessary in-repo path expansion #411 (comment)
• Allow necessary in-repo path expansion #411 (comment)
• Allow necessary in-repo path expansion #411 (comment)
• Allow necessary in-repo path expansion #411 (comment)

Skipped as no-action artifacts:

• The post-cutoff Codex review summary contained no additional finding.
• The current-head CodeRabbit result was a terminal review-rate-limit waiver; its earlier GMCC finding was fixed and separately acknowledged.
• Claude general review summaries only synthesized the inline findings above and added no separate blocker.

Evidence: preflight 70/644; expansion contract 4/66; goal completion 103/1346; prompt headroom 303/302/302; relevant RuboCop clean; local bin/validate PASS with 134 files/no offenses; hosted Lint, Validate, and Claude review SUCCESS; CodeRabbit terminal rate-limit waiver; exact head/base stable; no unresolved review thread.

Future address-review scans should begin after this summary.

[claude]

Review Summary

This PR reworks the "worker owned-paths" contract across agent-workflows' policy/prompt surfaces (5 full-contract mirrors + 3 compact mirrors + batch-plan-preflight + docs/changelog), so that a lane may edit an in-repository path not on its precomputed file list, provided the path is evidence-backed as necessary, and is protected by a new typed expansion-path-reservation / expansion-rename-reservation v1 record that feeds into collision/serialization checks. Core logic lives in skills/plan-pr-batch/bin/batch-plan-preflight; everything else is a mirrored prose restatement of the same contract plus a new contract test (file-expansion-contract-test.rb) that asserts the mirrors stay byte-identical.

Code quality / correctness (batch-plan-preflight)

• The new reservation validation (validate_expansion_path_reservations, valid_expansion_path_reservation?, effective_file_touch_maps, reservation_collides_with_path?) is careful and fail-closed: malformed/UNKNOWN/noncanonical/duplicate/stale reservations are all rejected, and rejection paths were traced against the callers that gate them (lane_ids_valid, file_touch_violations.empty?) — I couldn't construct an input that slips an invalid reservation through to an "accepted" result.
• The stage_dependency_plan["id"] → precomputed stage_dependency_plan_id refactor (lines ~129-149) is a small hardening improvement: it replaces an implicit dependency on stage_dependency_plan always being a Hash with an explicit .is_a?(Hash) guard, avoiding a potential NoMethodError on a missing/malformed dependency plan.
• Left one inline note on a minor defense-in-depth gap in the reservation identity check (nil-coincidence on batch_plan_id/stage_dependency_plan_id) — currently masked by independent unconditional validators elsewhere, so not exploitable today, but worth tightening given the "every field must be a known identity" design intent.
• Collision handling: when any path in a same-wave collision set is reservation-derived, the whole collision now requires explicit max-1 serialization (a typed edit edge is no longer sufficient), even if other paths in that same set are ordinary verified overlaps unrelated to any reservation. This is a conservative/stricter behavior by design (confirmed by test_mixed_verified_and_reserved_overlap_requires_pair_level_max_one_serialization), not a bug, but worth being aware it's slightly coarser-grained than per-path.
• Test coverage for the Ruby logic is extensive (new reservation shapes, stale/duplicate detection, sole-editor vs multi-editor lifecycle, rename ancestor/descendant collisions, risky-capacity union). I wasn't able to execute the Ruby test suite in this sandbox (bash execution required interactive approval that wasn't available), so I reviewed it statically rather than confirming green.

Design/policy consideration (the actual substance of this change)

The meaningful part of this PR isn't the JSON-schema validation — it's the policy shift: an autonomous worker can now write to a repository path outside its originally-approved list without user approval, gated only by a self-declared reason (a "known single-line" string) and evidence_ref (a URI-shaped string). The tooling verifies these are syntactically well-formed and durable-looking, but has no way to verify the evidence is true — a worker under wrong-diagnosis pressure can produce a plausible-sounding reason and a real-looking coordination-state URI for a path expansion that isn't actually justified. The material stop-list is preserved and reservations are still gated by preflight/collision checks, so this is a reasoned tradeoff rather than an oversight — the author's own PR body flags architectural-product-judgment / autonomous-merge-policy-change gates and marks it human-approval-required. Flagging this explicitly since it's the part most worth a maintainer's deliberate sign-off, separate from the code review.

Maintainability

The full contract paragraph (~40 lines) is duplicated verbatim across 5 files (docs/pr-batch-skills.md, skills/plan-pr-batch/SKILL.md, skills/pr-batch/SKILL.md, skills/triage/SKILL.md, workflows/pr-processing.md), plus 3 compact one-line mirrors. This is enforced only by a string-equality contract test, so it will work, but every future wording tweak to this contract now requires 8 synchronized edits. That's evidently a deliberate pattern already used elsewhere in this repo (not introduced by this PR), so just noting the ongoing cost rather than asking for a rewrite here.

Nit

The PR description is ~250 lines of automated QA/evidence/disposition metadata (rollback SHAs, "priority-finding-dispositions", audit receipts, etc.), which makes it hard to see the actual "why" at a glance. Not a blocker, but a shorter human-facing summary up top would help reviewers.

No functional bugs found in the reviewed diff; findings above are one low-severity hardening suggestion (inline) plus design/process observations.

[justin808]

Fixed on exact current head eec86f0a9258e735ddbed2def9758fbe54d4390d: the stale-reservation check now requires a valid verified pr-file-touch-map, so matching pre-PR planned-path-evidence does not clear either scalar or rename reservations. Added RED-to-GREEN regressions for both record types while retaining completed-lane and verified-reflection stale rejection. Focused preflight is 72 runs / 652 assertions; local and hosted full validation pass. Historical finding: #411 (review)

[claude]

Review Summary

This PR reworks the pr-batch/plan-pr-batch policy so workers may expand into repository paths that weren't in the original owned-paths list, as long as the expansion is evidence-backed and coordinated via a new typed expansion_path_reservations mechanism in batch-plan-preflight. The change is large but almost entirely prompt/policy text mirrored across 5 full surfaces and 3 compact surfaces, with one real code change in skills/plan-pr-batch/bin/batch-plan-preflight (Ruby) plus its test suite and a new file-expansion-contract-test.rb that enforces the mirrors stay byte-identical (after whitespace normalization).

Code quality / correctness (batch-plan-preflight)

Traced the new reservation logic (validate_expansion_path_reservations, valid_expansion_path_reservation?, reservation_reflected?, effective_file_touch_maps, updated validate_collisions) end to end:

• Path canonicalization (canonical_repository_relative_path?) correctly rejects ../. components, leading/trailing slashes, //, backslashes, NUL bytes, and drive letters — good traversal hardening for a field that ends up asserted into a file-touch map.
• evidence_ref/state_ref validation (durable_state_ref?) requires both a URI scheme and host, consistent with the existing lane-lifecycle-state convention.
• Reservation staleness (lane completed, or already reflected in a verified-source map) and duplicate-identity detection look correct; reservation_reflected? correctly excludes planned-path-evidence maps (only source == verified counts), matching the new tests.
• The refactor of stage_dependency_plan into a precomputed stage_dependency_plan_id (guarded by .is_a?(Hash)) is a nice defensive fix — previously stage_dependency_plan["id"] would have raised on a non-hash/missing stage_dependency_plan when lane_ids were valid but the dependency plan wasn't.
• effective_file_touch_maps/colliding_paths interplay is safe: duplicate path entries introduced by merging reservations into a lane's paths array are harmless because collision detection uses set intersection/uniq.
• Gating reservation validation behind lane_ids_valid (skipping reservation violations entirely when lane ids are broken) is intentional and covered by a dedicated test — avoids noisy/confusing violations when lane identity itself is broken.

Did not find a correctness bug in this script; it's well covered by the new tests.

Possible scope creep (worth a maintainer look)

The GMCC-v4 compact completion contract is changed in 5 files (workflows/pr-processing.md x2 occurrences, skills/pr-batch/SKILL.md, skills/triage/SKILL.md, skills/plan-pr-batch/scripts/check_goal_prompt_size.rb, skills/pr-batch/bin/goal-completion-contract-test.rb) from pending|missing|untriaged or threads unresolved|UNKNOWN=>... to pending|missing|untriaged|failed or threads unresolved|UNKNOWN=>....

This adds a failed CI-check state to the NOT COMPLETE gate — a real behavior change to when a batch is considered ready — but it isn't mentioned anywhere in the PR's What changed bullets or the CHANGELOG entry, which are scoped entirely to path expansion/reservations. Given the PR title is Allow necessary in-repo path expansion, it would help future auditors (and the CHANGELOG) if this fix were called out explicitly, or split into its own PR, since it's logically unrelated to the reservation feature.

Maintainability note (already acknowledged in prior review threads)

The full contract paragraph (~35 lines) is duplicated verbatim across 5 markdown surfaces, and a compact one-liner across 3 more. This is guarded by the new file-expansion-contract-test.rb, which will catch drift, so it's not a correctness risk today — just flagging that this doc-as-prompt duplication pattern will keep growing maintenance cost with every future policy tweak.

Security

No issues found. Path/URI validation is appropriately strict; no shell/eval/network calls were introduced in the Ruby script.

Overall: solid, well-tested implementation of the stated feature. The one thing worth calling out explicitly is the undocumented failed-state addition to the GMCC-v4 contract.

[justin808]

Address-review summary

Exact head: 76cca902fa4f8ecd7da0c7882d502603072aef85
Review cutoff replayed from: 2026-08-12T13:45:35Z

• Must-fix: 0.
• Discuss: 1 deduplicated current-head concern — the compact GMCC-v4 failed state was not disclosed. Handled without a code change: What changed and the Decision log now identify the five-surface fail-closed mirror correction; independent exact-head QA confirmed the expanded canonical contract already treated failures as NOT COMPLETE; the inline thread was answered and resolved.
• Optional: 1 earlier defense-in-depth identity check remains explicitly not applicable to this bounded change because unconditional plan validation already fails closed; its thread was previously answered and resolved.
• Skipped: repeated review-summary restatements were deduplicated against the inline items above.

All 25 review threads are resolved. No current-head review item remains untriaged, pending author action, or release-blocking.

[justin808]

---

head_sha: 76cca90
triggered_gates:

• architectural-product-judgment
• autonomous-merge-policy-change
• changed-lines-limit
• commit-count-limit
• public-compatibility
  rollback_disposition: "Code-only rollback is established: revert the merge commit; no migration, deployment, permission grant, or external effect."
  decision: approve
  approved_by: justin808
  source: direct-user-task
  evidence: Allow necessary in-repo path expansion #411 (comment)
  ...

[justin808]

Completed-batch audit: replay evidence follows.