fix: derive auto-repair inScope from requiredFix references, not finding.file alone

[TSOFTP-afk]

Problem

A review finding's file records where the problem was observed, but the fix often targets a different file named in requiredFix (docs vs sample data, config vs code). planQualityFollowUp derived the auto-generated repair round's inScope from finding.file alone:

const files = findings.map((finding) => finding.file).filter(...)
...
inScope: files.length > 0 ? files : source?.inScope,

When file ≠ fix target, the generated repair contract is self-contradictory and unsatisfiable:

1. The repair's acceptance criteria (= the finding's requiredFix) instruct editing file B.
2. The repair's inScope only declares file A.
3. evaluateQualityCompletion rejects every honest completion ("README.md is undeclared"), because changedPaths ⊆ inScope cannot hold for the mandated edit.

The repair then fails, and the downstream review-round-N stays permanently blocked on a failed dependency (a failed dependency cannot be taken over or cancelled) — the whole quality loop dead-locks and the team can never declare delivery.

Real-world reproduction

Reproduced end-to-end in a production multi-agent session (5 tasks): implementation completed → review returned needs_revision with file=<observed data file> and requiredFix="edit README.md …" → the auto repair round could not be completed honestly by any path (declaring the actual edit was rejected by the scope gate; declaring the declared file would be a false report) → downstream review wedged → team had to be abandoned. Full incident notes available on request.

Fix 1 — derive auto-repair inScope from requiredFix (211e729, 4c80f50)

New pure helper:

export function repairScopeFromFindings(
  findings: readonly ReviewFinding[],
  fallback: string[] | undefined,
  inheritedOutOfScope: readonly string[] = [],
): string[] | undefined

Derives the repair inScope from the observed files plus workspace-relative paths referenced in the requiredFix text:

• slash paths (src/parser.ts) and bare filenames with a code/doc extension allowlist (README.md, wc.js) — the allowlist keeps version tokens (v0.1.17), hashes, and prose out of the scope;
• :line suffixes are tolerated and stripped;
• absolute/illegal paths are dropped (they can never match workspace-relative scope patterns anyway — pathMatchesScope rejects drive-letter patterns);
• a candidate that the source task's inherited outOfScope already covers is skipped (see Fix 3);
• dedup, order-preserving; falls back to the source task's inScope when nothing legal remains.

planQualityFollowUp now uses it. Over-inclusion is accepted by design: inScope is an audit upper bound, and the requiredFix text still tells the implementer what to touch.

Fix 2 — captain-only controlled contract amendment (e651d50)

Following the analysis in this comment: when a quality contract is wrong (a verify command that cannot pass, an inScope that forbids the file the objective names), the worker has no honest completion — it dead-locks or games the gate (e.g. pointing package.json.main at the test file to make a broken command exit 0). This adds the missing correction channel:

• agent_teams_amend_task (captain-only): replaces objective / acceptance / verify / inScope / outOfScope (full lists, not deltas) on one non-terminal quality task. Registered via TEAM_TOOL_NAMES but not MEMBER_TOOL_NAMES, so CAPTAIN_TOOL_NAMES filtering keeps it out of member tool schemas entirely — and current main derives MEMBER_DENIED_TOOLS = CAPTAIN_TOOL_NAMES, so the runtime denial list covers it without another hand-written entry.
• Audit trail: every amendment appends a TaskRevision (at / by / reason / fields / previous) to the task's revisions ledger, validated at the durable JSON boundary (isTaskRevision), plus an agent-teams/task-amended session event. Rulings travel in the task record, not the mailbox.
• Freeze rules: rejected for terminal tasks, for kind=work (no contract), and once a review/requirements task with verdict=pass has passed judgment on the task. Mid-flight amendments are allowed; completion gates evaluate the amended contract naturally (they read current task fields).
• Usage protocol: the quality-kinds paragraph documents the amend path.

Fix 3 — the derived scope yields to the inherited outOfScope (a8f4176)

Raised by @Tonited in this comment, cross-referencing #173 / #177. planQualityFollowUp copies outOfScope: source?.outOfScope verbatim, and classifyChangedPath consults outOfScope before inScope (tdd.scope.out-of-scope-wins). A derived candidate covered by an inherited directory pattern is therefore unregistrable — so Fix 1, which widens the derived scope, would have added more such entries rather than fewer.

repairScopeFromFindings now takes the inherited list and skips any candidate it covers, so the widening in Fix 1 cannot manufacture an entry the gate always rejects. Deliberately complementary to #177: resolving the inherited patterns themselves (dropping the intersecting ones, and naming both lists in the rejection message) stays in #177's generator-level change; this PR only stops its own widening from adding colliding entries, and the fallback path is unchanged. Once #177 lands, the filter is a no-op.

History / rebase status

Merged main @ 3b95edbe (v0.1.18) into this branch (merge-forward; mergeable_state is clean). The only conflict was src/index.ts rule 7: the resolution keeps v0.1.18's new scheduling guidance (correct never-started pending tasks through edit_plan update_task; a captain may cancel a pending task directly) and appends this branch's amend_task paragraph. No upstream change is reverted.

Earlier commits had two defects that are fixed here: repairScopeFromFindings returned readonly string[] (a TS2322 against PlannedFollowUpTask.inScope?: string[]), and capabilities.test.mjs tool-count expectations still said 13.

Tests

• scripts/quality-gates-repair-scope.test.mjs — requiredFix fix-target inclusion, absolute-path dropping + fallback, :line stripping, end-to-end incident shape, plus (Fix 3) covered-candidate skipping, all-candidates-covered fallback, and an end-to-end case asserting no declared path is matched by the inherited outOfScope.
• scripts/quality-gates-amend.test.mjs — amendment recording (previous values/fields/reason), gate evaluation of the amended contract, mid-flight vs frozen (review/requirements pass), terminal + work rejection, reason/author/field presence, malformed replacements (blank strings, empty lists, absolute paths, ..), revision accumulation, durable-state validation.
• scripts/capabilities.test.mjs — tool surface assertions updated to the 14-operation SDK.

Verified on the merged tree (Windows, Node v24.12.0) against upstream CI steps:

Check	Result
tsc -p tsconfig.json --noEmit / tsconfig.client.json --noEmit	pass
build (clean-build + tsc emit ×2 + tsdown)	pass
verify.mjs, fallback-tdd, member-failure-tdd, quality-gates-tdd, lifecycle-verify, stress-verify, web-routes-verify, harness-compat-tdd, stability-tdd, compatibility.mjs, doctor.mjs	all exit 0
node --test (repair-scope, amend, capabilities, release-metadata, http-body)	43 pass / 0 fail

One gap in this environment, reported for honesty: scripts/compatibility.test.mjs symlinks its bin into %TEMP%, and this Windows host denies symlink creation (EPERM), so that one suite could not run here; its underlying scripts (compatibility.mjs, doctor.mjs) pass when invoked directly. CI is the authority for it.

Notes for reviewers

• The branch is now on top of v0.1.18 (3b95edbe); the derivation helper and the amendment path are merged with the current scheduling, mailbox, and member-setup code rather than sitting on older file versions.
• A related, separate observation from the same incident: review findings where file ≠ fix target are easy to produce (the reviewer naturally points at the file with wrong content). A docs-side mitigation is to document that reviewers should set file to the fix target — happy to add that to the docs in a follow-up.

中文摘要：①review finding 的 file 记录的是"问题所在的文件"，而 requiredFix 指向的才是"要改的文件"；自动 repair 轮的 inScope 只按前者派生，导致修复契约自相矛盾，worker 无法诚实完成。②契约本身写错时 worker 无纠正通道（update_task 无契约字段 / edit_plan 仅 staged / 终态不可变），只能死锁或作弊；本 PR 增加受控契约修订 agent_teams_amend_task：captain-only、修订入 revisions 台账 + 事件、review pass 后冻结、成员 schema 不可见。③（Tonited 指出，#173/#177）继承的 outOfScope 优先于 inScope，而 Fix 1 拓宽 inScope 反而更容易撞上继承的排除模式；现在派生阶段会跳过被继承 outOfScope 覆盖的候选，与 #177 的生成器级修复互补（#177 落地后该过滤自动失效）。④分支已 merge 到 main v0.1.18（3b95edbe），冲突仅 src/index.ts 规则 7，已保留上游新调度指引并追加本 PR 段落；本地按上游 CI 步骤验证（typecheck / build / 全部 verify 脚本 / node --test 43 项全过）。

[licat2023]

Unrelated to this PR's correctness — the derivation fix (extract paths from requiredFix instead of trusting finding.file) looks right, and the "self-contradictory auto-generated contract" it removes is real. But the failure mode has a second half worth recording, from a 0.1.17 + DSH 0.1.5-rc.2 run.

When a contract is wrong, the workers are not allowed to correct it, so the loop can only dead-lock or cheat:

• agent_teams_update_task (src/tools.ts:1556-1621) accepts only task_id / status / output / attempt_id / verdict / findings / changedPaths / acceptanceResults / commandsRun. There is no way to amend objective / acceptance / verify / inScope / outOfScope on an existing task.
• agent_teams_edit_plan is staged-only: requireStagedTeam (src/tools.ts:240-245) throws team "…" is already running; its plan can no longer be edited.
• Terminal tasks are immutable by design (src/tools.ts:1376, :1664) — a completed repair cannot record a late fix either.

I hit the same shape with a human-authored contract: the verify command in my task was wrong (a node --test <directory> argument that Node v24.20.0 resolves as a module entry, so it dies with Cannot find module). Unable to amend the recorded contract, the implementer satisfied the recorded command by adding a package.json whose main pointed at the test file — which turns the gate into exit 0 with tests 1 / pass 1 (a false green that a later reviewer reproduced). When I then ruled "delete the manifest", the ruling could only travel by mailbox: the task record still said otherwise, the member correctly reported it had no carrier, and an already-passing review of the old revision had to be re-run against the new bytes.

So this PR removes one way to generate a self-contradictory contract. A controlled contract amendment for unfinished tasks — captain-only, recorded as a revision/event, refused once a review has passed — would remove the class, including contradictions that originate from the human rather than the generator.

[TSOFTP-afk]

Implemented — please see e651d50 (appended to this branch): agent_teams_amend_task, a captain-only controlled contract amendment, shaped along the lines you proposed.

Your three locks, addressed:

• update_task has no contract fields → agent_teams_amend_task now replaces objective / acceptance / verify / inScope / outOfScope (full lists, not deltas) on one non-terminal quality task.
• edit_plan is staged-only → the amendment path works mid-flight (running teams); the implementer re-reads the amended contract at its next quality gate, and completion gates need no special casing because they already read the task's current fields.
• Terminal immutability is preserved by design: amendments are rejected for terminal tasks, and — per your "refused once a review has passed" requirement — rejected once a review/requirements task with verdict=pass has passed judgment on the task (reviewedTaskId match). kind=work is rejected (no contract to amend).

Recording: every amendment appends a TaskRevision (at / by / reason / fields / previous values) to the task's revisions ledger, validated at the durable JSON boundary (isTaskRevision), plus an agent-teams/task-amended session event. So a ruling like your "delete the manifest" now travels in the task record — with the wrong verify replaced — instead of by mailbox, and the revision history shows what changed and why. Your fake-green case (package.json main pointing at the test file to make a broken node --test exit 0) is exactly the "no honest completion" pressure this removes: the captain fixes the command instead.

Captain-only: the tool is registered in TEAM_TOOL_NAMES but not MEMBER_TOOL_NAMES, so CAPTAIN_TOOL_NAMES filtering keeps it out of members' tool schemas entirely (verified E2E: a spawned member reports the tool absent from its schema; a captain-driven E2E with an intentionally wrong inScope showed the gate reject src/... is undeclared, amend → same completion passes, ledger persisted in team.json, work/terminal/refreeze all rejected). The usage protocol's quality-kates paragraph now documents the escape hatch.

Also in this commit: repairScopeFromFindings now returns string[] — the previous commit had a latent TS2322 (assigning its readonly string[] into inScope?: string[]) that failed pnpm typecheck; caught while running the real CI steps locally. capabilities.test.mjs tool-count expectations updated 13 → 14, and scripts/quality-gates-amend.test.mjs covers amendment recording, gate evaluation of the amended contract, freeze rules, work/terminal rejection, malformed-input rejection, and durable-state validation.

[Tonited]

Cross-referencing #173 / #177, because this PR touches the same function for an adjacent reason.

planQualityFollowUp also copies outOfScope: source?.outOfScope verbatim — that line is untouched by this PR. Since classifyChangedPath consults outOfScope before inScope, a broad inherited directory pattern swallows any inScope entry beneath it. A generated repair can therefore be impossible to complete honestly:

implementation outOfScope contains deploy/compose/postfix/ → the generated repair inherits it → the repair's own inScope contains deploy/compose/postfix/master.cf.inc → every honest completion is rejected as out_of_scope, and the member registers 2 of the 13 files it actually changed.

That is the same dead-lock shape this PR set out to remove, reached by a different route. Worth flagging that widening the generated inScope (Fix 1 here) increases the chance of this collision rather than reducing it: more declared fix targets, same inherited exclusion list.

#177 drops the intersecting inherited patterns (keeping non-colliding ones verbatim) and names both lists in the rejection message instead of reporting a bare out_of_scope. The two changes are complementary and touch adjacent lines of the same function, so whichever lands second may need a trivial rebase — happy to rebase #177 onto this one once it is merged.

[TSOFTP-afk]

@Tonited — your analysis was right, and it changed what this branch does.

I merged main @ 3b95edbe (v0.1.18) into the branch (merge-forward, mergeable_state is clean again) and added a guard in repairScopeFromFindings: it now takes the source task's inherited outOfScope and skips any derived candidate that list already covers. That is precisely your point about Fix 1 increasing the collision surface — widening the scope can no longer add an entry the gate only rejects. Coverage added: the covered-candidate case, the all-candidates-covered fallback, and an end-to-end planQualityFollowUp assertion that no declared path is matched by the inherited outOfScope.

Kept deliberately complementary to #177: I did not touch outOfScope: source?.outOfScope, the classifyChangedPath precedence, dedupeScope, or the rejection message — resolving the inherited patterns themselves, and naming both lists, stays yours. Because a #177-style generator fix removes the collision at its source, the new filter becomes a no-op once #177 lands. The fallback path (source?.inScope) is also unchanged, so the remaining inherited-conflict cases are still #177's to close.

On landing order, your prediction holds: the inScope: line now reads repairScopeFromFindings(findings, source?.inScope, source?.outOfScope), so whichever lands second needs the small rebase you described. If #177 lands first, I will rebase this branch on top of it; if #155 goes in first, the #177 rebase is the one-line context fix — happy to prepare it for you or leave it to you, whichever you prefer.

Also on the merged tree (Windows, Node 24): tsc --noEmit for both programs, build, and the repo verify chain (verify.mjs, fallback/member-failure/quality-gates TDD, lifecycle, stress, web-routes, harness-contract, stability, compatibility, doctor) all pass; node --test across the branch suites is 43 pass / 0 fail.

[TSOFTP-afk]

@licat2023 — following up on your comment: the amendment channel you proposed is implemented, and the branch has moved since my earlier reply.

• agent_teams_amend_task is captain-only and replaces objective / acceptance / verify / inScope / outOfScope (full lists) on one non-terminal quality task.
• Every amendment appends a TaskRevision (previous values, fields, reason, author) to the task's revisions ledger — validated at the durable JSON boundary — plus an agent-teams/task-amended session event.
• It is refused for terminal tasks, for kind=work, and once a review/requirements task with verdict=pass has passed judgment on the task.

What is new since that reply: the branch is now merged with main @ 3b95edbe (v0.1.18), so your three locks were re-checked against the current scheduling, mailbox, and member-setup code. In particular, current main derives MEMBER_DENIED_TOOLS = CAPTAIN_TOOL_NAMES, so the member denial list now covers agent_teams_amend_task automatically — it is absent from member tool schemas and denied at runtime, with no hand-written entry. The branch also gained a scope guard from the #173 / #177 discussion (derived repair candidates covered by an inherited outOfScope are skipped).

If you have a moment, an informal check of the amend path against your fake-green incident (the implementer pointing package.json.main at the test file so a broken verify exits 0) would be much appreciated: scripts/quality-gates-amend.test.mjs encodes the gate-evaluation-of-the-amended-contract and freeze behavior, and the PR body lists the local verification evidence (typecheck, build, full verify chain, 43 node --test cases).