Repository navigation
Conversation
asto18089
left a comment
There was a problem hiding this comment.
Fresh 12-lane re-review at 56105b1 (unchanged during review; base pinvou3-clean f1eb10e is the branch tip, so the PR is already exactly on base — no rebase needed). Every claim below was verified first-hand against the code; local gates were run on this machine (rustc/cargo 1.99.1) because the PR currently shows no CI runs.
Verdict: CHANGES_REQUESTED
No blockers, no smuggling, no incorrect behavioral code. The behavioral change (item 5a) is sound and well-tested. Two MAJORs block: one commit-metadata defect, one copy-vs-code divergence in the PR's own added sentence — the exact class this PR exists to eliminate.
Item matrix verification
| Item | Copy-vs-code claims | Verdict |
|---|---|---|
| 1 read | 2000/50KB matches READ_MAX_LINES/READ_MAX_BYTES (file.rs:523-524) enforced for this exact tool; image sniffing magic-byte-based, exactly PNG/JPEG/GIF/WebP; lossy UTF-8 decode (file.rs:768-770); BMP correctly unclaimed (sniffed into image branch at file.rs:643, rejected at image_attach.rs:144) |
ACCURATE |
| 2 bash | Merged stream applies to ALL invocations (contract path unconditional, one shared pipe, stderr structurally empty → finish_contract_bash_result concatenation is arrival-ordered); fresh shell per call, cwd fixed at manager construction, no cd/env persistence; unchanged rest spot-verified (caps 2000/50KB at output.rs:8-9, 120s default at shell.rs:4452, $SHELL detection) |
ACCURATE |
| 3 v4 skill | V4_BEST_PRACTICES_BODY is include_str! of the shipped asset (byte-identical, verified); read/edit/grep_files live and model-visible; zero phantom/retired names remain anywhere in assets; scope comments (v4 swept, feishu opt-in unswept) verified |
ACCURATE |
| 4 Run schema | Foreground gates direct Command::new(program)+args, no shell; background returns task_ids immediately, task_status completion surface; wait/task_shell_wait framing matches codebase language; every visible clause has a hidden-schema counterpart, no semantic drift |
ACCURATE |
| 5a fim refusal | Correct placement (after NotFound, before API call and write, side-effect-free); UTF-8 boundary-safe; locate/count semantics identical and match the new schema wording; all 3 tests non-vacuous (client check is at fim.rs:178-190, after validation); old first-occurrence behavior confirmed at base; no prompt/skill/doc teaches the old semantics | SOUND |
| 5b read_media | 256KiB budget constant + encode ladder; crop short-circuits the detail budget (read_media.rs:682-686) so the blanket "crop reads keep the full 5 MiB" is exactly right, including crop+auto; 1024/2048/4096px caps verified; auto default verified | ACCURATE |
| 5c tasks | trust_mode semantics + from_runtime hardcode false (task_manager.rs:731) across both production paths; auto_approve default false, ApprovalMode::Bypass→full_access, no prompts, GHSA cited in code; gate_run BLOCKED-before-spawn verified |
mostly ACCURATE — see M-2 for allow_shell |
| 5d lsp | Exactly diagnostics/read_lints/symbols/definition/references exist; only definition (mod.rs:513) and references (mod.rs:530) hard-require line; pinned test updated, no stale pins repo-wide |
ACCURATE |
| 5e frontmatter | Parser (skills/mod.rs:596-787) consumes only name/description/invocation/aliases-for/description_<tag>; removal is a parse no-op; base-tree sweep was exactly the 4 files; YAML/aliases-for/catalog-matrix intact |
ACCURATE — see m-2 for digest convention |
| 5f text.rs | Personality genuinely folded into constitution (prompts.rs:1043-1045; tone text inside BASE_PROMPT); CALM_PERSONALITY zero non-test references; guard tests exist as described; old header factually wrong (15e4134 already deleted the overlay composition) |
ACCURATE — see m-3 |
MAJOR
M-1 — Head commit 56105b1 message describes the wrong change. Its subject is byte-identical to 9fbce4d's ("docs(prompts): fix stale overlay doc comment") but it actually implements item 5d: the lsp.rs line schema description plus the pinned-test update. The commit body is accurate and the content is 100% in scope (test co-committed), but a docs(prompts) title on a fix(tools) schema change breaks bisectability and the sibling pattern. Please retitle (e.g. fix(tools): disclose lsp line requirement) before merge.
M-2 — New allow_shell copy diverges from actual inheritance (tasks.rs:198). "Omit to inherit the current session's allow-shell setting" — actual resolution is req.allow_shell.unwrap_or(self.cfg.allow_shell) (task_manager.rs:1683), where cfg.allow_shell is the durable-task profile snapshot from TaskManagerConfig::from_runtime → config.allow_shell() (task_manager.rs:730, config.rs:7363, default false). The interactive session's effective setting is a different accessor, interactive_allow_shell() (config.rs:7377-7378, default true). Under default config, an interactive session that omits the field produces a task with shell disabled while the copy implies it inherits the session's enabled state. Fail-closed direction, so behavior is safe — but this is precisely the copy-widening this PR declares eliminated ("no claim was widened or narrowed beyond the code"). Suggested one-line reword: "Omit to use the session's durable-task allow-shell default (false unless explicitly enabled)." No pinned test breaks (tasks:: targeted tests 15/15 pass and will re-verify).
MINOR
- v4 retirement test is now tautological.
upgrade_from_v4_installs_pack_and_retires_unchanged_v4_best_practices(system/tests.rs:557) writesV4_BEST_PRACTICES_BODY— now the post-PR body — so it no longer simulates any real install; a test pinning the actual pre-PR bytes and asserting retirement would now fail. The PR already discloses the underlying retirement neutralization; the offered "legacy-body second candidate" (pinned pre-PR byte constant) would fix both at once. - Bundled-body change convention not followed. The 4 changed SKILL.md bodies are in
BUNDLED_SKILLS, butSUPERSEDED_SHIPPED_BODY_DIGESTSgot no entries andBUNDLED_SKILL_VERSIONstays "12" (system.rs:25-32 convention; refresh only on bump at install_one). Installed gen-12 copies keep the dead lines and will be misclassified as user edits by the skills audit on a future bump. Parse-identical, cosmetic impact; append the 4 old-body digests with the next bump (old-body SHA-256 prefixes: best-of-nb72d093f, delegate169640cc, fleet-managerfa4369c7, skill-creator6fb9e921). - Renumbered header still lists a nonexistent layer. text.rs:19 "2. Approval-policy overlays" — the overlay constants were deleted as dead code in 15e4134 (ancestor of base); approval policy now travels as runtime metadata and tests assert its absence from prompts (prompts.rs:3353-3356). The PR edited and renumbered this exact block; a 2-line follow-up closes it.
- Verification-section counts don't reproduce here. Stated filter set yields 258 passed / 0 failed (not 395) — substance confirmed: 3 new fim tests pass by name, full lib suite = base + 3, all failures pre-existing env flakiness on untouched files. Clippy head == base diagnostics, so no-new-diagnostics holds, but this environment shows 6 pre-existing diagnostics (5×
too_many_arguments+ the collapsible_if), not one. Gates overall: fmt clean, tests green on substance. - Audit completeness rests on the PR's own matrix. No authoritative 2026-10-07 item list exists in either repo, history, or GitHub (only pointer: tests.rs:220 → merged #112, whose cited audit doc is also absent). All falsifiable per-item claims checked out; "No items were skipped" is unverifiable by design.
- read description omits the 5 MiB inline cap (pre-existing): >5MiB PNG/JPEG/GIF/WebP silently becomes an omission note (image_attach.rs:58,144); the sibling read_media description updated in this same PR discloses 5 MiB, which makes the omission stand out.
- Adjacent follow-up candidates (pre-existing, not this PR's scope): shell.rs:133 runtime error cites retired
exec_shell; legacyTaskRecordrestore defaultsauto_approveto true (task_manager.rs:3105, serde fallback) — fail-open adjacent to the GHSA narrative; 9 remaining bare "(action=…)" descriptions in tasks.rs; docs/READ_MEDIA.md lacks the 256KB budget.
Process / gates
- DCO verified 10/10; conventional format otherwise correct.
- No CI runs on the PR: the author isn't in
.github/APPROVED_CONTRIBUTORS, so fork-ci runs sit ataction_requiredper the repo AGENTS.md — needs a maintainer run-approval or/lgtm. - No smuggling:
git diff --name-only base..headis exactly the 14 item files; every hunk attributable; both out-of-scope disclosures verified TRUE (args/all_featuresdeny_unknown_fieldsat verifier.rs:92-99;retire_unchanged_v4_best_practicesbyte-compare at system.rs:528-543, precedent b1fcde7 confirmed). - Parent-repo follow-up after the gitlink bump (not this PR's defect): fork-modifications.md T2 entry, fork-guard fim fingerprint + EXPECTED_HEAD/COMMITS advance (20 commits now unregistered; 17 registered fingerprints already stale at the current base), a
forkguard_*behavior test for the new refusal,fork-guard.sh --fast. 0/196 registered fingerprints are broken by this PR. The fim refusal is also a reasonable upstream candidate per fork-policy §2.
Clean areas (verified explicitly)
Fim edge cases (empty anchors now refused consistently with the exactly-once contract; self-overlapping undercount is the only residual gap and is not a distinct edit site; AnchorsOverlap dead before and after — pre-existing), UTF-8 byte-boundary safety, refusal channel consistency, no external callers on the old semantics, no unbounded-read regression; bash unchanged claims; hidden-schema parity for Run; read_media crop/budget/dimension constants; lsp operation inventory; frontmatter parser and catalog contract; no contradictions with any other model-facing doc.
In-range lock refresh closing all 4 open Dependabot alerts (4 high): - undici 7.29.1 -> 7.30.1 via wrangler 4.150.0 -> 4.156.1 -> miniflare 5.20261006.2-beta -> 5.20261101.0-beta in root, web/, and telemetry-ingest/: closes GHSA-xjj6-f33q-929c / CVE-2026-109869 (alerts #139, #142, #145; fetch DoS via unhandled error on excessive content-encodings). This closes the follow-up PR #120 left open: wrangler 4.156.1 (latest stable) exact-pins miniflare 5.20261101.0-beta (2026-11-01), which exact-pins undici 7.30.1, so a plain `npm update wrangler` per directory lands there. Collateral, all npm-canonical: workerd 1.20261006.1 -> 1.20261101.1 (+ its five platform binaries), tslib 2.8.1 -> 2.9.1 in root and telemetry-ingest, `libc` metadata added to the existing @img/sharp-* entries in root and telemetry-ingest, wrangler's @cloudflare/workers-types peer range following to ^5.20261101.1, and web's nested miniflare ws 8.21.0 hoisting to the top level (content-identical). No other packages move. - @azure/msal-node 6.0.1 -> 7.1.2 in extensions/vscode/: closes GHSA-xcx6-jx8p-x3h5 / CVE-2026-109461 (alert #136; auth-code flow origin validation; first patched 7.1.0) via the native in-range path: @azure/identity 4.14.1 (2026-10-29) requires @azure/msal-node ^7.1.0 and satisfies @vscode/vsce's declared ^4.13.2, so `npm update @azure/identity` moves identity 4.14.0 -> 4.14.1 and msal-node -> 7.1.2 with no manifest change. msal-common follows upstream's native shape (top-level 16.14.2 for msal-browser 5.25.1, nested 16.15.0 under msal-node; both exact pins). msal-node 7.x keeps the confidential-client surface identity uses (jsonwebtoken ^9, node >= 20); the path is dev-only (vsce). Verified: - npm ci green in root, web/, telemetry-ingest/, extensions/vscode/ - telemetry-ingest: npm run typecheck, npm run check (wrangler 4.156.1 deploy --dry-run), npm test (171 passed) - web: npx wrangler --version -> 4.156.1, npm run cf-typegen, npm test (408 passed in 47 files) - extensions/vscode: npm run check (tsc), vsce package --no-dependencies (valid 13-file vsix), and an identity -> msal-node 7 live smoke: ClientSecretCredential.getToken() builds and parses a real AAD protocol response through msal-node 7.1.2 - npm audit: root and telemetry-ingest report 0 vulnerabilities; web reports 20 moderate advisories (glob via @opennextjs/cloudflare 1.20.9, minimatch via the eslint toolchain) ingested after this branch's initial verification; all devDependencies, none open GitHub alerts, left for a separate decision Out of scope, unchanged: npm audit reports follow-redirects (high) in integrations/{feishu,wecom}-bridge and qs + minimatch (moderate) in extensions/vscode; none of these are open GitHub Dependabot alerts, so they are left for a separate decision. Signed-off-by: asto <asto18089@126.com>
56105b1 to
9e24145
Compare
|
Fix wave pushed (56105b1 → 9e24145, 8 commits) answering the 12-lane review above. Every finding was verified against the code before fixing: MAJORs
Beyond the two posted MAJORs, the wave closed the valuable minors and caught one regression neither the review nor the PR's local gates had caught:
Gates (local, rustc/cargo 1.99.1): fmt clean; touched-module filters all 0-failed (incl. the new legacy-addendum test); full lib suite 12277 passed / 1 failed, the failure being the known load-order flake Out-of-scope follow-ups disclosed in the body: legacy Still needs a maintainer to approve the fork-ci runs (author runs sit at |
In-range lock refresh closing all 4 open Dependabot alerts (4 high): - undici 7.29.1 -> 7.30.1 via wrangler 4.150.0 -> 4.156.1 -> miniflare 5.20261006.2-beta -> 5.20261101.0-beta in root, web/, and telemetry-ingest/: closes GHSA-xjj6-f33q-929c / CVE-2026-109869 (alerts #139, #142, #145; fetch DoS via unhandled error on excessive content-encodings). This closes the follow-up PR #120 left open: wrangler 4.156.1 (latest stable) exact-pins miniflare 5.20261101.0-beta (2026-11-01), which exact-pins undici 7.30.1, so a plain `npm update wrangler` per directory lands there. Collateral, all npm-canonical: workerd 1.20261006.1 -> 1.20261101.1 (+ its five platform binaries), tslib 2.8.1 -> 2.9.1 in root and telemetry-ingest, `libc` metadata added to the existing @img/sharp-* entries in root and telemetry-ingest, wrangler's @cloudflare/workers-types peer range following to ^5.20261101.1, web's nested miniflare ws 8.21.0 hoisting to the top level (content-identical), and the nested miniflare/node_modules/undici copies in web/ and telemetry-ingest/ moving to a single top-level 7.30.1 entry (root's undici was already top-level). No other package versions change. - @azure/msal-node 6.0.1 -> 7.1.2 in extensions/vscode/: closes GHSA-xcx6-jx8p-x3h5 / CVE-2026-109461 (alert #136; auth-code flow origin validation; first patched 7.1.0) via the native in-range path: @azure/identity 4.14.1 (2026-10-29) requires @azure/msal-node ^7.1.0 and satisfies @vscode/vsce's declared ^4.13.2, so `npm update @azure/identity` moves identity 4.14.0 -> 4.14.1 and msal-node -> 7.1.2 with no manifest change. msal-common follows upstream's native shape (top-level 16.14.2 for msal-browser 5.25.1, nested 16.15.0 under msal-node; both exact pins). msal-node 7.x keeps the confidential-client surface identity uses (jsonwebtoken ^9, node >= 20); the path is dev-only (vsce). Verified: - npm ci green in root, web/, telemetry-ingest/, extensions/vscode/ - telemetry-ingest: npm run typecheck, npm run check (wrangler 4.156.1 deploy --dry-run), npm test (171 passed) - web: npx wrangler --version -> 4.156.1, npm run cf-typegen, npm test (408 passed in 47 files) - extensions/vscode: npm run check (tsc), vsce package --no-dependencies (valid 13-file vsix), and an identity -> msal-node 7 live smoke: ClientSecretCredential.getToken() builds and parses a real AAD protocol response through msal-node 7.1.2 - npm audit: root and telemetry-ingest report 0 vulnerabilities; web reports 20 moderate advisories (glob via @opennextjs/cloudflare 1.20.9, minimatch via the eslint toolchain) ingested after this branch's initial verification; all devDependencies, none open GitHub alerts, left for a separate decision. No HIGH findings remain in any of the four refreshed manifests: the base audits carried 1 HIGH for undici in root, web/, and telemetry-ingest/, and 2 HIGH for msal-node in extensions/vscode (the advisory this bump closes) Out of scope, unchanged: npm audit reports follow-redirects (high) in integrations/{feishu,wecom}-bridge and qs + minimatch (moderate) in extensions/vscode; none of these are open GitHub Dependabot alerts, so they are left for a separate decision. Signed-off-by: asto <asto18089@126.com>
asto18089
left a comment
There was a problem hiding this comment.
Fresh re-review round at 9e24145 (unchanged during review; rebase re-verified as a no-op — merge-base == base tip f1eb10e, so the branch sits exactly on latest pinvou3-clean). 13 lanes this round: smuggling/attribution, fim deep-dive, Run-schema truth, tasks truth (all 13 rewritten fields traced to execution paths), shell/read/read_media truth, skills-system mechanics, assets+guard tests, prompts headers, body/DCO/disclosure audit, design, plus fresh local gates (fmt, clippy vs base, full suite, 9 mutations). Everything below was verified first-hand against the code; gates were re-run on this machine (rustc/cargo 1.99.1) because CI sits at action_required for this author.
Verdict: useful, root-caused, complete for its claimed scope, no smuggled changes — one MAJOR copy-accuracy defect to fix before merge, plus 7 minors.
What held up under fresh verification
- No smuggling. All 45 hunks across the exact 18-file set attribute to a matrix row, a wave-commit purpose, or a disclosed note; zero whitespace churn; DCO 18/18; no duplicate subjects (the r1 dup is fixed;
f835cfebdcorrectly retitled). - Digests byte-verified. The four new
SUPERSEDED_SHIPPED_BODY_DIGESTSentries equal the SHA-256 of the base asset files;V4_BEST_PRACTICES_LEGACY_BODYis byte-identical (1772 B) to the base v4 asset; the gen bump + entries together matches the 10→11 (cd8fd0b65) and 11→12 (f1eb10efc) precedent. - Retirement repair is live and load-bearing, not dead code.
install_system_skillsruns at every startup (system.rs:580 via lib.rs:11082); base compared against the then-current body which was the legacy body, so the asset move in this very PR would have stopped retiring installs that still teachread_file/edit_file— the pin fixes that (98c90cd). Multi-generation refresh (11→13) verified working with a chained-digest end-to-end test. - Audit-pointer doubt from the previous round is refuted: #112 exists and is MERGED ("fix(tui): align model-facing text with actual behavior"); the gen-history comments in tests.rs check out commit-by-commit.
- tasks.rs: all 13 rewritten field descriptions verified TRUE against execution paths — model passthrough/required-with-provider, plan read-only mapping, allow_shell omission → durable
config.allow_shell()default false (distinct frominteractive_allow_shell), trust_mode hardcoded false atfrom_runtime, auto_approve create-default false, BLOCKED classifier refusal, timeout clamp/kill/timed_out shape, attempt group/index/count/verification/attempt_id/limit clauses including the literalAttempt not found: {id}and newest-firstsort_by_key(Reverse(created_at)). Zero bare(action=…)descriptions remain in tasks.rs. - shell/read/read_media: every clause TRUE — single merged OS pipe with dup'd write end (true arrival order;
stderrempty in snapshot), fresh shell per call, 5 MiB pre-base64 cap + BMP sniffed-then-omitted,from_utf8_lossydecode,READ_MAX_LINES/READ_MAX_BYTES= 2000/50KB,READ_IMAGE_BYTE_BUDGET= 256 KiB shared by auto+low, crop bypassing toMAX_WIRE_IMAGE_BYTES, ladder real, "roughly 350 KB" wire math fair. - prompts headers substantively true: message[0] = constitution with the Calm tone literally inside
BASE_PROMPT(text.rs:56-60);CALM_PERSONALITYused only by two tests, which do assert it stays folded/absent; approval policy travels as<turn_meta>runtime metadata (engine.rs:4270-4296). - fim behavioral change: correct, root-caused, complete.
edit(file.rs:1694-1704) andapply_patch(apply_patch.rs:1439) already refuse ambiguity — fim was the outlier; no regression case found (empty anchors now refused consistently with the "exactly once" schema wording; byte-offset slicing char-boundary-safe; perf = two linear scans vs an API round-trip). All three disclosed out-of-scope items verified TRUE, including the exactdefault_auto_approveline (task_manager.rs:3105). - Test quality: 9/9 mutations killed. fim trio RED with the refusal deleted; retire test RED with the legacy pin reverted; forkguard sweep RED on an injected
`Bash`; the canonicalno_advertised_tool_teaches_a_retired_nameguard RED in isolation on the exact historical "Bash with action 'wait'" regression; lsp pin RED. No vacuous tests; the two phantom-name guards are honestly scoped and disjoint.
MAJOR
M-1 — run_tool.rs:110: "Commands run directly as program+args, not through a shell" is false for background=true.
Foreground gates do spawn directly (Command::new(program).args(...), verifier.rs:1156-1160). Background gates do not: render_gate_command shlex-joins program+args into one string (verifier.rs:656-661) and hands it to ShellManager::execute_with_options_env_for_session → CommandSpec::shell (shell.rs:2182) → the platform shell (bash -c / pwsh -Command / cmd /C). The repo's own test comment admits the quoting is lossy in that path ("rendered with POSIX quoting and handed to PowerShell, which cannot parse a quoted absolute path with backslashes", verifier.rs:1496-1502). So for background gates the sentence is false and the shell-routing/quoting caveat is undisclosed — exactly the defect class this PR exists to eliminate, on a load-bearing schema string. Suggested fix (copy-scoped, no behavior change): state that foreground gates run program+args directly while background gates currently render the command through the detected platform shell (so args are shell-quoted; prefer program='bash', args=['-lc', …] there), or align the background path to direct spawning in a separate behavioral PR — either way the unconditional sentence has to go.
Minors
- m-1 — shell.rs:133/144 (change of this PR): the reworded hint "pass an explicit
cwd" is unactionable on the surface that emits it. The visible bash schema (shell.rs:4576-4594) has nocwdproperty andadditionalProperties:false;cwdis published only on the read-only schema (shell.rs:4674) and the hidden compat path (shell.rs:5076). On the main bash tool the actual escape hatch iscd <dir> && …insidecommand. Same defect class theexec_shellhint fix targeted — suggest "orcdinto it in the command" (optionally notingcwdwhere that parameter exists). - m-2 — text.rs:46-49:
BASE_PROMPT's doc comment still claims the runtime "composes this core with mode / approval / skills / context-management / compaction / authority-recap layers" — contradicting the corrected assembly story this PR installs 30 lines up (text.rs:16-27) and in prompts.rs:5-8. In-scope residue of 29a1ecb's "finish stale overlay header cleanup" (mode is deliberately absent, approval is turn_meta, compaction is not a system layer). Also pre-existing, outside this diff: prompts.rs:1097-1111 items 5-6 still describe compaction-relay system appends the builder no longer performs. - m-3 — tasks.rs:194 (mode): "approvals stay enabled" is false for the reachable plan +
auto_approve=truecombination.RuntimePolicyProjection::from_requestmapsauto_approve=Some(true)toApprovalMode::Bypasswith no plan override (runtime_policy.rs:55-57). The read-only headline still holds (shell/writes/MCP-non-readonly stay hard-blocked; sandbox ReadOnly), but the clause as written is wrong on that path. Suggest wording that scopes the claim, e.g. "plan's blocks are not lifted by auto_approve (which otherwise skips approval prompts)". - m-4 — run_tool.rs background: "return task_ids immediately" — the literal
task_idsarray exists only in tool-result metadata (verifier.rs:653), which the engine does not deliver to the model; the model-visible payload carriesjobs[].task_id(verifier.rs:219). Wording drift, semantics intact — suggest "returns immediately with per-gate task ids". - m-5 — fim.rs:76-84: the tool-level
description()omits the uniqueness contract that the schema properties now disclose; siblingeditdiscloses at both levels (file_tool.rs:163). Convention parity in a copy-completeness PR. - m-6 — v4 legacy pin mechanism: a ~1.7 KiB full-body raw-string const in production source diverges from the repo's own precedent (digest in prod + historical bytes in tests:
GENERATION_10_DOCUMENTS, tests.rs:652-667;fixtures/mcp-discovery-v11.md, tests.rs:752). Today the pin and the compare target are the same const, so a future transcription drift would keep every test green while real legacy installs silently stop retiring — a digest pin (e.g.crate::hashing::sha256_hex) plus a test fixture would self-check. Behavior verified byte-correct today, so this is a hardening suggestion, not a defect. - m-7 — completeness observation: 14 bare "(action=…)"-style descriptions remain in other model-visible tool schemas (automation.rs:149,153,157,174,183; rlm.rs:283,287,293,297; github/schema.rs:26,30,35,56,68(+64 with copy)) — the same copy class this PR eliminates in tasks.rs. Either fold them into this batch or disclose them as out-of-scope; the body's "nine remaining" claim itself is accurate for tasks.rs.
Nits (no action required)
- f835cfe: SoB display name
asto18089vs author nameasto(same email; DCO CI passes, but DCO.md guidance says the trailer should match). - 29a1ecb typed
fix(prompts)but is doc-comment-only; identical-content sibling 9fbce4d is typeddocs(prompts). - fim.rs:140-142 comment uses present tense ("are matched at their first occurrence") for behavior the next lines suppress; "would be matched" is exact.
- fim.rs: the
refused()helper is reused by the unique-anchors test where nothing is refused — name overclaims. - tasks.rs range restatements ("Defaults to 20; values are clamped into 1-100") duplicate the schema-declared
minimum/maximum/defaultin the same property object — keeping "clamped, not rejected" suffices. - tests.rs:86-88 comment "install or keep it" overstates: byte-exact keeps are deleted at next launch (retire now also matches the current body); the current-body retire arm also lost its dedicated test in the de-tautology fix — consider one extra case or a reword.
- Foreground
run_gatemisreports a nonexistent cwd as "{program}is not installed or not in PATH" (verifier.rs:1167-1181) while the background path errors accurately — pre-existing, adjacent to m-1's surface. commands[]items lackadditionalProperties:falsealthough unknown item keys hard-fail viadeny_unknown_fields(verifier.rs:115-122) — the strictness is real but unadvertised.
Gates (re-run fresh on this machine, PR tree @ 9e24145 vs clean f1eb10e)
cargo fmt --all -- --check: clean.- clippy
--workspace --all-targets --all-features --locked: diagnostic sets byte-identical between trees (six pre-existing: 5×too_many_arguments, 1×collapsible_ifat turn_loop.rs:5499); a--cap-lints warncompleteness pass confirms zero new diagnostics in any crate/target. (The exact command exits 101 on both trees — pre-existing workspace lint deny, unchanged by this branch.) - Full suite
cargo test -p codewhale-tui --lib --locked(RUST_MIN_STACK raised): 12275 passed / 3 failed / 13 ignored; all 3 failures classified as load-order flakes (pass in isolation; one was cross-process interference from a concurrent lane), zero PR regressions. Every touched-module count claim in the body reproduced exactly (fim 4, skills::system 29, canonical_action+run_tool 10, tasks+file_tool 37, shell trio 3, prompts 116, verifier+fim+forkguard 16 — all 0-failed). - 10/10 new and updated tests green by name; 9/9 mutations RED/GREEN as designed.
Recommendation: fix M-1 (copy reword is enough for this PR's scope) and the three one-liner minors m-1/m-2/m-3 if convenient; m-4 through m-7 can ride a follow-up. Everything else — usefulness, completeness, root-causing, elegance, hygiene — held up under fresh adversarial verification.
|
Wave 2 pushed (9e24145 → 7da8df2, 6 commits) answering this review. Every change was made against the code evidence first, and the round-2 gates were re-run before pushing: MAJOR
Minors
m-7 (14 bare "(action=…)" fields in automation/rlm/github schemas) is disclosed out-of-scope in the body instead of risked: each needs its execution path traced before truthful copy can be written — flagged as a follow-up batch, alongside the pre-existing foreground Gates re-run post-wave (this machine, rustc/cargo 1.99.1): fmt clean; clippy set byte-identical to base (six pre-existing, none introduced); full suite 12,292 tests — 12,275 passed / 4 failed / 13 ignored with all four failures the known machine-specific load-order flakes that pass in isolation ( Housekeeping note: the git endpoint was unreachable at push time, so this wave went up via the Git Data API during a window when api.github.com was still reachable — server-side commit SHAs therefore differ from the locally verified objects while carrying identical trees, messages, authors, dates, and per-topic DCO sign-offs. PR head is |
In-range lock refresh closing all 4 open Dependabot alerts (4 high): - undici 7.29.1 -> 7.30.1 via wrangler 4.150.0 -> 4.156.1 -> miniflare 5.20261006.2-beta -> 5.20261101.0-beta in root, web/, and telemetry-ingest/: closes GHSA-xjj6-f33q-929c / CVE-2026-109869 (alerts #139, #142, #145; fetch DoS via unhandled error on excessive content-encodings). This closes the follow-up PR #120 left open: wrangler 4.156.1 (latest stable) exact-pins miniflare 5.20261101.0-beta (2026-11-01), which exact-pins undici 7.30.1, so a plain `npm update wrangler` per directory lands there. Collateral, all npm-canonical: workerd 1.20261006.1 -> 1.20261101.1 (+ its five platform binaries), tslib 2.8.1 -> 2.9.1 in root and telemetry-ingest, `libc` metadata added to the existing @img/sharp-* entries in root and telemetry-ingest, wrangler's @cloudflare/workers-types peer range following to ^5.20261101.1, web's nested miniflare ws 8.21.0 hoisting to the top level (content-identical), and the nested miniflare/node_modules/undici copies in web/ and telemetry-ingest/ moving to a single top-level 7.30.1 entry (root's undici was already top-level). No other package versions change. - @azure/msal-node 6.0.1 -> 7.1.2 in extensions/vscode/: closes GHSA-xcx6-jx8p-x3h5 / CVE-2026-109461 (alert #136; auth-code flow origin validation; first patched 7.1.0) via the native in-range path: @azure/identity 4.14.1 (2026-10-29) requires @azure/msal-node ^7.1.0 and satisfies @vscode/vsce's declared ^4.13.2, so `npm update @azure/identity` moves identity 4.14.0 -> 4.14.1 and msal-node -> 7.1.2 with no manifest change. msal-common follows upstream's native shape (top-level 16.14.2 for msal-browser 5.25.1, nested 16.15.0 under msal-node; both exact pins). msal-node 7.x keeps the confidential-client surface identity uses (jsonwebtoken ^9, node >= 20); the path is dev-only (vsce). Verified: - npm ci green in root, web/, telemetry-ingest/, extensions/vscode/ - telemetry-ingest: npm run typecheck, npm run check (wrangler 4.156.1 deploy --dry-run), npm test (171 passed) - web: npx wrangler --version -> 4.156.1, npm run cf-typegen, npm test (408 passed in 47 files) - extensions/vscode: npm run check (tsc), vsce package --no-dependencies (valid 13-file vsix), and an identity -> msal-node 7 live smoke: ClientSecretCredential.getToken() builds and parses a real AAD protocol response through msal-node 7.1.2 - npm audit: root and telemetry-ingest report 0 vulnerabilities; web reports 20 moderate advisories (glob via @opennextjs/cloudflare 1.20.9, minimatch via the eslint toolchain) ingested after this branch's initial verification; all devDependencies, none open GitHub alerts, left for a separate decision. No HIGH findings remain in any of the four refreshed manifests: the base audits carried 1 HIGH for undici in root, web/, and telemetry-ingest/, and 2 HIGH for msal-node in extensions/vscode (the advisory this bump closes) Out of scope, unchanged: npm audit reports follow-redirects (high) in integrations/{feishu,wecom}-bridge and qs + minimatch (moderate) in extensions/vscode; none of these are open GitHub Dependabot alerts, so they are left for a separate decision. Signed-off-by: asto <asto18089@126.com>
The visible `read` tool described itself as text-only, but the contract reader returns sniffed PNG/JPEG/GIF/WebP files as inline image blocks and decodes everything else as UTF-8 with replacement characters instead of refusing invalid bytes (image OCR lives on the hidden legacy read path). Say so, and keep the 2000-line/50KB default window statement that matches the contract reader's effective budgets. Signed-off-by: asto <asto18089@126.com>
The small-contract bash description claimed to "return stdout and stderr", but the contract path drains both descriptors through one shared pipe and concatenates them, so the model receives a single merged stream it cannot attribute per-fd. Each call also spawns a fresh shell: `cd` and exported variables do not persist between calls. State both facts up front. Signed-off-by: asto <asto18089@126.com>
v4-best-practices taught `read_file` and `edit_file`, both hidden compatibility names that never appear in a model-visible catalog or in a `tool_search` result; hosts that derive allowlists from the wire catalog reject the call outright. Cite the visible `read`/`edit` tools instead, extend the existing retired-name guard to this asset (it is not in BUNDLED_SKILLS, so the bundled-body sweep never covered it), and drop the stale scope note. Note: installs whose SKILL.md byte-matches the previously shipped body are no longer auto-retired by `retire_unchanged_v4_best_practices`; they keep the stale copy until user-deleted, same trade-off as the prior body edit. Signed-off-by: asto <asto18089@126.com>
The visible `Run` wrapper reduced the hidden run_verifiers schema to bare one-line param docs, dropping the load-bearing clauses: custom gates run directly as program+args, not through a shell (reach for `bash -lc` only intentionally), and background gates return task_ids immediately with completion tracked in task/status state rather than pollable streams. Carry the hidden schema's wording into the visible surface, including per-field descriptions. Signed-off-by: asto <asto18089@126.com>
fim_edit matched both anchors at their first occurrence with no uniqueness check, so a repeated anchor silently generated the middle at the wrong site. Count occurrences and refuse the edit when the prefix anchor repeats anywhere or the suffix anchor repeats after the prefix, naming the match count and asking for a longer anchor; disclose the exactly-once contract in the parameter schema. Covered by three new tests. Signed-off-by: asto <asto18089@126.com>
The description and `detail` schema advertised the 5 MiB payload ceiling
but not the 256 KiB delivery budget a default read ('auto'/'low') actually
applies, while 'high'/'original' and crop reads keep the full budget.
State both numbers where the model chooses a detail level.
Signed-off-by: asto <asto18089@126.com>
tasks create advertised `allow_shell`, `trust_mode`, and `auto_approve` with bare "(action=create)" placeholders. Document what each authority flag grants (shell permission with session-default inheritance, relaxed workspace-boundary path validation, auto-approved tool calls) and that a gate_run command the safety classifier rates dangerous is refused with BLOCKED before anything runs unless the session itself auto-approves. Signed-off-by: asto <asto18089@126.com>
The line-based frontmatter parser reads flat key/value pairs and no crate consumes `short-description`; the nested `metadata:` block only parsed as a no-op "metadata" key plus a stray top-level entry. Remove the four dead blocks; names, descriptions, and aliases-for are untouched. Signed-off-by: asto <asto18089@126.com>
The module header listed the personality overlay as assembly step 2, but the runtime folds personality tone into the constitutional preamble and never composes CALM_PERSONALITY (tests pin the Calm overlay out of the default prompt). Describe the real assembly order and mark the constant as retired test-only text. Signed-off-by: asto <asto18089@126.com>
definition and references fail with "requires line (1-based)" when the line parameter is missing, but the schema described it as a plain "1-based line." with no requirement. Name the operations that need it and pin the string in the schema test. Signed-off-by: asto18089 <asto18089@126.com>
Nine task-tool properties still carried bare "(action=...)" descriptions: model, mode, timeout_ms, the attempt bookkeeping fields, verification, attempt_id, and limit. Describe each from its actual execution path (defaults, clamping, failure shapes). Also correct the fresh allow_shell copy: omitting the flag resolves through the durable-task profile snapshot (config allow_shell(), default false), not the interactive session's effective setting. Signed-off-by: asto18089 <asto18089@126.com>
retire_unchanged_v4_best_practices compared installed copies against the current asset only, so the PR edit that fixed read_file/edit_file in the shipped body stopped retiring exactly the installs that need it: those still teach retired tool names. Pin the previously shipped body as a legacy const and retire byte-exact matches of either body; user edits stay. The retirement test wrote the same const the retire fn compares against, so its assertion was a tautology and no longer simulated any real install. Write the legacy bytes instead and pin the two constants apart; an appended-line case proves only byte-exact bodies retire. Signed-off-by: asto18089 <asto18089@126.com>
…tion cleanup The four bundled SKILL.md bodies stripped of dead short-description frontmatter changed shipped bytes without the non-destructive upgrade half: no superseded digest entries and no generation bump. Per the codewhale-hq#4691 convention (bump plus entries together, as 10->11 and 11->12 did), bump to generation 13 so installed gen-12 copies refresh at next launch, and record the four previous-body digests so those copies count as unmodified shipped text instead of user edits. Signed-off-by: asto18089 <asto18089@126.com>
The previous header fix corrected the personality line but kept renumbering "Approval-policy overlays" as a composed layer; those overlays were removed as dead code and approval policy travels as request-time runtime metadata. Drop the layer and say where approval policy actually lives; align the prompts.rs module header, which still named a personality overlay step. Signed-off-by: asto18089 <asto18089@126.com>
The read description claimed images unconditionally, but the inline path caps at 5 MiB (larger images come back as an omission note) and BMP is sniffed into the image branch yet rejected by the validity check, so its bytes never reach the model as text or image. Name both cutoffs; the sibling read_media description already discloses the same 5 MiB limit. Signed-off-by: asto18089 <asto18089@126.com>
validate_shell_working_dir told the model to pass an explicit working_dir/cwd "to exec_shell" — a name retired in v0.9.3 that no catalog or tool_search result can return. The hint also offered working_dir, an alias only the hidden compatibility tool accepts. Ask for an explicit cwd, which the surfaces that reach this path can set. Signed-off-by: asto18089 <asto18089@126.com>
'deliver about 256KB' read as a typical delivered size; small images pass untouched at far less and the ladder delivers at most the budget. Use the same budget framing the low clause and docs already use. Signed-off-by: asto <asto18089@126.com>
…cisely The volatile-content-last enumeration omitted two WorldState fragments the builder really adds (the interactive continual-harness block and the imported project-instructions fragment), and the module header's 'single text module' overstated where every compile-time constant lives. Signed-off-by: asto <asto18089@126.com>
"Out-of-range values are clamped, not rejected" was false for negative integers: they are rejected as a type mismatch by the u64 parser and never reach the clamp. State the boundary per field instead (clamped into the declared range; below-1 clamp for the attempt fields), and pin the corrected sentences plus the reviewed authority claims (mode blocks not lifted by auto_approve, durable-task allow-shell fallback, trust mode full-access retry, BLOCKED gate refusal) that had no schema pin. Signed-off-by: asto <asto18089@126.com>
The receipt printed the pre-base64 encoded size as "(wire payload)", but the actual wire payload is the base64 form (~4/3 larger) — the same doc the budget bullet cites says so. Name it honestly. Signed-off-by: asto <asto18089@126.com>
Three comments survived the assembly-layer cleanup with stale claims: the section header still said every bundled layer lives in text.rs (the authority recap and locale bookends are defined here), the handoff doc still said the relay artifact is prepended to the system prompt (it is routed as the late WorldState token-budget fragment), and the environment-block doc still claimed placement before instructions and skills (it leads the workspace WorldState fragment, after the constitution prefix). Mirror the corrected module header in text.rs. Signed-off-by: asto <asto18089@126.com>
Cover the emergent counting semantics the refusal trio left unpinned: an empty prefix is refused with the char-boundary count (characters + 1, not bytes + 1 — the multi-byte fixture discriminates a byte-offset refactor), suffix occurrences before the prefix are invisible to the uniqueness check, and a suffix that only occurs before the prefix stays SuffixNotFound. Signed-off-by: asto <asto18089@126.com>
The visible background copy promises a task id per gate, null when a gate is skipped or fails to start; only the running arm had coverage. Drive both null arms directly through start_background_gates: a gate without a program reports skipped with its reason, and a gate whose working directory does not exist fails shell-manager validation and reports failed_to_start with the accurate error. Signed-off-by: asto <asto18089@126.com>
The Run schema's per-path shell scoping and background task-id copy, the bash tool's merged-stream/fresh-shell claims, and the read tool's image/decoding disclosures had no test anywhere, so a full revert of any of them was invisible to CI. Pin each corrected sentence (and the absences: no task_ids array in the visible background copy, no separate-stream promise, no text-only read claim) against the schema and description strings. Signed-off-by: asto <asto18089@126.com>
The shape test only rejects a digest equal to the current body, so a single wrong hex digit in one of the four generation-13 entries would silently leave real generation-12 installs unrefreshed. Embed the four previous bodies as fixtures (the same pattern as the v4 legacy pin and the generation-10 documents body) and hash each against its registered entry on every run; the behavioral supersede check rides along. Signed-off-by: asto <asto18089@126.com>
The model-visible harness tool's description and its overview result's runtime block told the model to keep large source material in rlm. rlm is compatibility-only (model_visible() == false, kept out of the catalog and tool_search, and its own description says not to teach it to new model turns) — the same phantom-name class this branch fixed in the v4-best-practices skill and that the PHANTOM_NAMES sweep guards. On hosts whose allowlists derive from the wire catalog, the recommended call is rejected outright. The copy now names the visible bounded-reread surfaces instead: read with offset/limit and retrieve_tool_result. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
std matches() counts non-overlapping occurrences, so a self-similar
anchor ("aa" inside "aaa") passes the uniqueness check with count 1
and the edit lands at the first occurrence. Stated bare, "must appear
exactly once" reads as overlapping occurrences to a model counting with
overlapping intuition — the sibling edit tool already discloses its
"unique, non-overlapping region" contract, so fim now says the same at
both levels (tool description and both anchor schema descriptions) and
a copy test pins all three sentences.
Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
An empty anchor is not special-cased: the refusal emerges from the general matches-more-than-once rule. The two corners therefore pass with count 1 and keep the base behavior — an empty prefix on an empty file (single char boundary) and an empty suffix after a prefix ending at EOF (empty suffix region). Both now pin that they reach the API boundary instead of a refusal; the round-4 note in the PR body had claimed this pinning existed, but no such tests landed with that wave. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
The round-4 fix that relabeled the receipt's pre-base64 size from "(wire payload)" to "(encoded payload)" carried no test, so a silent revert would be invisible to CI. The delivery-note test now asserts the encoded-payload label is present and the wire-payload wording is gone. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
"Passing true or false is always honored" was an absolute: under mode=plan the value is stored but shell stays disabled by the mode's own hard block (the mode property already discloses exactly that precedence). The allow_shell copy now says "honored subject to the mode", naming the plan carve-out, and the authority-copy pin test asserts both new sentences. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
The module header still ended on "Keeping every layer's text in one module makes prompt tuning a single-file operation." — contradicting the corrected constant-split header directly above it (authority recap and locale bookends live in this module) and surviving two header cleanup waves. The handoff test whose name still said "prepended" is renamed to what it actually pins (inclusion); the prepending doc claim it echoed was corrected in the round-4 wave. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
74e8187 to
71b61ea
Compare
The contract reader now refuses NUL-bearing payloads (any NUL in the first 8 KiB) and UTF-16 BOMs as binary before the lossy decode (contract_read_payload_is_binary, added to file.rs by the subagent-isolation work while this PR was in review), so the read description's 'bytes that are not valid UTF-8 become replacement characters instead of failing the read' was an absolute the engine no longer honors: stray high bytes and NUL-free heads still decode lossily, the refused classes do not. Scope the clause with the refusal exception, pin the disclosure in the description pin (and forbid the unscoped sentence end from coming back), and correct the refusal message's 'read returns text only' absolute - read returns inline images for sniffed formats - to 'read returns text or inline images, never raw binary', pinned in the behavioral refusal test. Signed-off-by: asto <asto18089@126.com>
The wave-5 harness fix (stop teaching the hidden rlm compatibility surface; name the visible bounded-reread surfaces instead) landed with copy only - no test guarded it, so re-adding both original rlm mentions passes every guard and suite (mutation-proven in review). Pin the removal in both model-visible strings: the description and the overview result's runtime.context must carry no rlm and must keep naming read with offset/limit and retrieve_tool_result. Signed-off-by: asto <asto18089@126.com>
Two header residuals the wave cleanup missed: the consolidation sentence still claimed in the present tense that the whole prompt contract reads top-to-bottom in this one module, contradicting the corrected header directly above it - the authority recap and the locale bookends never lived here, so the claim is now scoped to the constants that moved. And the edit-loop command ran 'cargo test -p codewhale-tui --bin codewhale-tui prompts', which matches zero tests (the prompts tests live in the lib target); it now names --lib. Signed-off-by: asto <asto18089@126.com>
Base #131 changed the approval pump to auto_approve && !approval_force_prompt, so on a host that arms ask-on-guardrail guardrails, guardrail-class calls force a real human approval even for an auto_approve=true task. The "no approval prompts" absolute (and the mode row skip parenthetical) now carry that exception, the mode row discloses that trust_mode's effects still apply in plan (the retry path has no mode re-check), and the pins assert the exception clauses and forbid the unscoped absolute from returning. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
The tasks description pointed at task_shell_start unconditionally, but the registry registers the runtime task shell tools only under ShellPolicy::Full (allow_shell on, non-plan mode) — on the default durable-task surface the named tool is absent from the catalog. The task_id property grouped read with cancel as requiring an id, while read resolves through the same active-task default as pr_attempt_*. Both sentences are scoped and pinned. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
The item-5b budget sentences on the description and the detail schema were the one audited copy surface with no revert guard; a mutation probe flipping the figure passed the whole suite. Both surfaces are now pinned. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
The schema copy discloses that anchor occurrences are counted without overlap, but no test exercised a self-overlapping anchor, so an overlapping-counting refactor would have kept every test green while making the live copy false. "aa" in "aaa" has exactly one non-overlapping site and reaches the API boundary; an overlapping counter refuses. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
The requested-directory arm (the model-reachable one) only forbade the literal "resume/fork" token on the saved-workspace path, so a paraphrased revival of the false advice there passed CI; the pin now forbids resume and fork wording on that arm outright. The saved-workspace not-a-directory branch (shell.rs Restore-it advice) had no test at all and gets one. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
BASE_PROMPT's doc summary listed sections the constant does not have (output format, toolbox reference, "When NOT to use", sub-agent sentinel protocol live elsewhere), and the AUTHORITY_RECAP doc claimed it is appended at the end of the system prompt when the locale closer follows it for non-English locales — it is a per-request trailer after the WorldState fragments. Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
round-8 从零复审终局:APPROVE 级(零 P0/P1)
从零 19-lane 全新复审,锚定 head Rebase:merge-base = base tip 关键复核全部通过:
P2 非阻塞台账(1)
P3 非阻塞台账(要点)
合并后动作(parent 仓)
APPROVE 级 — 建议进 merge queue。 |
JensenChen28
left a comment
There was a problem hiding this comment.
已审查固定 head eab45d4aa8fe8ed4a574e52fb5af2310e7adef33,base 11cba403dc6bd6ca5c4dca55042785b03449ac8e。完整 29 文件差异的规范与需求两个轴均未发现实质问题,批准本 PR。
已核对模型文案对应的实际读取、媒体预算、shell、Run、tasks 与提示词组装路径;FIM 的非重叠计数/后缀搜索区间与披露一致,歧义拒绝发生在 provider 调用和写入前。Skill generation 13 的四份 gen12 fixture 及 v4 历史 fixture 均逐字节匹配 base 资产,五个 SHA-256 均与迁移登记相符,用户修改保护路径保持。
实际验证:fetch、完整源码/测试内容与 consumer 审查、git diff --check、五份历史资产的字节/摘要计算和 JSON catalog 解析,并只读核对该 head 远程检查(完成的检查成功,CodeQL neutral、sync skipped)。未在本机执行 Cargo/Rust、FIM 实际生成/写入、启动迁移或原生图像/provider 测试;作者的大规模测试/变异计数不是本轮执行结果。父仓 fork 登记、指纹调整和 gitlink 集成仍由对应父仓变更处理,未声称已验收该集成。发送前已核对最新 head/base 及讨论未变化。
zhuowp
left a comment
There was a problem hiding this comment.
Approve the source change at eab45d4aa8fe8ed4a574e52fb5af2310e7adef33 against actual target 11cba403dc6bd6ca5c4dca55042785b03449ac8e.
The cumulative 29-file/108-hunk scope and 35 unique affected registered test bodies are covered. I checked the actual tool registration, prompt/skill loading, migration preservation, FIM refusal-before-provider/write, media budgets, Run execution paths and task-policy consumers. No blocking defect was confirmed.
Two nonblocking P3 notes:
- Pin the five new historical Markdown fixtures to LF. In a normal
core.autocrlf=truecheckout these unpinnedinclude_str!inputs retain CRLF, while the two historical digest self-checks require LF hashes. This is a test-portability defect, not an observed Windows CI failure or proof of a new production migration regression: the actual target's shipped assets already have LF attributes. - Scope the new
tasks.timeout_mspromise to the platform guarantees. The Unix path kills a process group; the non-Unix fallback kills the direct child and retains the actual exit-status code. The timeout/status behavior itself is unchanged here.
Exact-tree Linux CI run 42173780690 / check 127747440420 contains ordinary PASS output for all 35 affected names. Its aggregate is 15026 passed (1 slow, 2 leaky), 15 skipped; the two unrelated LEAK warnings remain a resource-validation boundary. Current checks contain 12 success, one CodeQL neutral and one sync skip. Author-reported earlier runs/mutations are not treated as this review's execution.
No new local runner, compile, probe, model/provider or browser was used. Windows/macOS execution, live FIM generation and installed-user upgrades were not performed. This source approval does not authorize merge or release; final live-version and repository gates still apply.
Summary
Delivers the remaining engine-side items from the 2026-10-07 model-facing text audit: every string below is what the model reads, corrected to match what the engine actually does. English wording follows each surface's surrounding style; no claim was widened or narrowed beyond the code.
Six review fix waves ride on top: a 9-commit wave answering the first 12-lane review, a 6-commit wave (2026-11-02) answering the second 13-lane review — including one real regression the pinned canonical-action guard caught — a 7-commit wave (2026-11-04) answering a third from-scratch 14-lane re-review, a 7-commit wave (2026-11-04, round 4) answering a fourth from-scratch 14-lane re-review, a 6-commit wave (2026-11-04, round 5) answering a fifth from-scratch 16-lane re-review, and a 3-commit wave (2026-11-05, round 6) answering a sixth from-scratch 15-lane re-review, which also rebased the branch onto the
pinvou3-cleantip that had moved seven commits ahead, and a 6-commit wave (2026-11-06, round 7) answering a seventh from-scratch 18-lane re-review (details below). Commit SHAs in the wave sections refer to the current post-rebase chain.Item matrix
readdescription claimed text-only while the contract reader returns imagesread_filecompatibility tool — still 500 lines/16KB in file.rs, and not advertised in the model-visible tool catalog (it remains reachable by external MCP clients through the server's defaultfile_read→read_fileexpose mapping; that legacy copy is recorded as a follow-up below) — it never described the visibleread. The visible read's contract window defaults to 2000 lines/50KB (READ_MAX_LINES/READ_MAX_BYTES, file.rs), so the numbers in the description matched the code. The remaining inaccuracy — "Read a text file" while sniffed PNG/JPEG/GIF/WebP files return as inline image blocks and non-UTF-8 bytes decode lossily — is now disclosed, including the 5 MiB inline cap and BMP's omission-note behavior (fix wave). After the round-6 rebase moved the base onto the subagent-isolation work, which added a binary refusal before the lossy decode, the decode clause also carries the refusal exception: a payload with a NUL byte in its first 8 KiB, or beginning with a UTF-16 byte-order mark, is refused as binary (wave 6).bashdescription hid fresh-shell and merged-stream semanticsshared_output_pipe) and concatenates them (finish_contract_bash_result), and each call spawns a fresh shell. Description now states both, with the redirect-to-file escape hatch. (The #126-era line was confirmed at shell.rs:4557 on the audit-era base; the line has since moved with base evolution.)read_file/edit_fileread/edittools. Extended the retired-name guard (PHANTOM_NAMESsweep + the read_file/exec_shell loop) to this asset and fixed the stale scope comment that documented the gap.Runvisible schema stripped verifier semanticscommandsnow carries the program+args-not-a-shell clause and per-field descriptions;backgroundnow states task-id return, task/status tracking, and the wait-only-for-output/barriers rule. The first cut carried the hidden schema'sBashwording verbatim into the visible schema, which the pinnedno_advertised_tool_teaches_a_retired_nameguard rejects — the wave rewrote it to nametask_shell_wait, the published waiter the background metadata itself advertises inpoll_with. The second wave scoped the not-through-a-shell claim per execution path (background gates render through the platform shell) — see wave 2.fim_editfirst-occurrence anchors, no uniqueness checkread_media256 KiB default budget undocumenteddetailschema now give the ~256KB auto/low delivery budget vs the full 5 MiB budget kept by high/original and crop reads; docs/READ_MEDIA.md records the budget beside the other guards (fix wave). The result receipt's size line now says "(encoded payload)" — it prints the pre-base64 size, and the actual wire payload is the base64 form ~4/3 larger (wave 4).tasks createauthority flags bare "(action=create)"; gate_run BLOCKED undisclosedallow_shell(shell permission; omitted resolves through the configured durable-task allow-shell setting, false unless enabled in config),trust_mode(relaxed workspace-boundary path validation, defaults false — productionTaskManagerConfig::from_runtimehardcodes the default),auto_approve(auto-approved tool calls, full-access posture, defaults false per GHSA-72w5-pf8h-xfp4 opt-in).gate_run'scommanddiscloses the BLOCKED refusal for dangerous classifications unless the session auto-approves. The wave also discloses the nine remaining bare "(action=…)" properties (model, mode, timeout_ms, attempt bookkeeping, verification, attempt_id, limit) from their execution paths.lspline requirement for definition/references undisclosedlinedescription now names the operations that require it; pinned schema test updated.short-descriptionfrontmatter blocksaliases-foruntouched; catalog-matrix contract unaffected (name+invocation only). Generation bumped to 13 with the four previous-body digests recorded, so installed gen-12 copies refresh instead of fossilizing as user edits (fix wave); the four digests are anchored to embedded generation-12 fixtures (wave 4).CALM_PERSONALITYas assembly step 2No items were skipped: all audit targets still existed on the
origin/pinvou3-cleanbase this audit ran against (f1eb10e at the time; the branch now sits on the further tip 11cba40, with the audit surfaces re-verified there in round 6) and are addressed above. Item 1's number half is recorded as already-accurate rather than fixed, with the code evidence in the table.First review fix wave (2026-11-02)
From the fresh 12-lane review of this PR; every wave change was verified against the code first:
bfa732d91— retitle of the head commit: it implemented item 5d (lsp.rs) but carriedbca4382c9'sdocs(prompts)subject verbatim; nowfix(tools): disclose lsp line requirement.ca2af5ff6— allow_shell copy correction: omitting the flag resolves through the durable-task profile snapshot (config.allow_shell(), default false), not the interactive session's effective setting (interactive_allow_shell(), default true); the first cut's "inherit the current session's allow-shell setting" was wrong under default config. Plus the nine remaining bare "(action=…)" schema fields disclosed.4447e58b8— v4 retirement repair:retire_unchanged_v4_best_practicescompared against the current body only, so item 3's edit stopped retiring exactly the installs that still teach retired names. The previously shipped body is now pinned and retired byte-exactly; the retirement test writes the legacy bytes (it previously wrote the same constant the retire function compares against, making its assertion a tautology), and an appended-line case pins the exact-match boundary.2ad8e553a— non-destructive upgrade half for 5e: generation 12 → 13 plus the four previous-body SHA-256 digests, per theSUPERSEDED_SHIPPED_BODY_DIGESTSconvention (bump and entries together, as 10→11 and 11→12 did); catalog matrix fixture generation follows.4499d3983— finishes 5f: drops the dead "Approval-policy overlays" layer from the text.rs header (the overlay constants were removed as dead code; approval policy travels as request-time runtime metadata) and corrects the prompts.rs module header that still named a personality overlay step.1a190462f— read description now discloses the 5 MiB inline cap and BMP's omission-note behavior (BMP is sniffed into the image branch and rejected byprepare_tool_image_bytes, so its bytes reach neither image nor text output).26ea31623—validate_shell_working_dirno longer tells the model to pass a parameter "to exec_shell" (retired in v0.9.3) and drops the hidden-tool-onlyworking_diralias from the hint; asks for an explicitcwd.e454f70eb— docs/READ_MEDIA.md delivery-budget bullet.969624f87— Run schema regression fix described in matrix row 4.Second review fix wave (2026-11-02, round 2)
From the fresh 13-lane re-review at
969624f87(rebase re-verified as a no-op at the time); one MAJOR plus minors, each fixed against code evidence:5f86b1af6— Run verifier copy scoped per execution path (review MAJOR): "Commands run directly as program+args, not through a shell" was false forbackground=true— only foreground gates spawn directly (Command::new(program).args(...)); background gates render the command into a single line (render_gate_command) and hand it to the detected platform shell viaShellManager, where POSIX quoting can fail under PowerShell (the repo's own background-gate test documents exactly that). The copy now scopes the claim per path. Also:backgroundno longer names a literaltask_idsfield (that array exists only in tool-result metadata the model never sees — the visible payload carries per-gate task ids), and thecommands[]items object now carriesadditionalProperties: falseso the advertised schema matchesCustomVerifierInput'sdeny_unknown_fieldshard fail on unknown item keys.6521c3418— bash working-dir hint made actionable: the reworded hints said "pass an explicitcwd", but the visible bash schema accepts onlycommand/timeout/sandbox_permissions/justification(contract_bash_legacy_inputrejects the rest), so the suggestion failed with "unexpected bash parameter(s)" on the very surface that surfaced the error — and the call is refused before spawn, socdinside the command cannot help either. Surfaces that do acceptcwdadvertise it in their own schema, so the hint now names only recovery options that work everywhere (restore/remap, resume/fork).6935e8e12— task mode approval claim corrected: "'plan' … and approvals stay enabled" was false for the reachable plan+auto_approve=truepath —RuntimePolicyProjection::from_requestmapsauto_approvetoApprovalMode::Bypassregardless of mode, while plan's hard blocks (shell, writes, sandbox) hold. Reworded to say exactly that. Range restatements duplicating schema-declaredminimum/maximum/defaultin the same property (timeout_msrange,limitdefault+range,attempt_index/attempt_countrange) folded to one clamp clause (corrected to its exact type-error boundary in wave 4); defaults the schema does not carry ("Defaults to 120000/1") kept.6ee7116dd— fim tool-leveldescription()now carries the uniqueness clause (siblingeditdiscloses at both levels); in-code comment tense fixed; test helperrefusedrenamed toexecution_error(the unique-anchors test reused it where nothing is refused).5c012817f— finishes the header cleanup:BASE_PROMPT's doc no longer claims runtime "mode / approval / … / compaction" layers (contradicting the corrected module headers); the volatile-content-last doc lists the real sequence (composed constitution prefix → WorldState fragments → authority-recap/locale-closer trailers, with the compaction relay explained as a user-message template); remaining bare "overlay" wordings (CALM_PERSONALITY section/doc,StaticPromptCtxfields, embedder hook) aligned with the retired framing.52d999609— v4 legacy pin hardened to digest + fixture (review m-6): the full-body source constant was both the compare target and the test fixture, so transcription drift on either side would keep every test green while real legacy installs silently stop retiring. The pin is now a SHA-256 digest (theSUPERSEDED_SHIPPED_BODY_DIGESTSmechanism) and the historic bytes moved to a test fixture that asserts every run that they still hash to the pinned digest — drift on either side fails the suite (both directions mutation-proven, see Verification). Also restores dedicated coverage for the current-body retire arm (upgrade_retires_byte_exact_current_v4_body_installs) and aligns two comments with actual behavior (byte-exact keeps are retired at the next launch; feishu has no install path at all).Note: wave 2 was originally pushed via the Git Data API during a git-endpoint outage, so at the time its server-side SHAs differed from the locally verified commits while carrying identical trees, messages, authors, and dates; the wave-3 push reconciled the branch, and from 2026-11-04 until the round-6 rebase the server-side history was byte-identical to the locally verified commits. The round-6 rebase (2026-11-05) rewrote every SHA again onto the moved base — the current chain is the locally verified one.
Third review fix wave (2026-11-04, round 3)
From a fresh from-scratch 14-lane re-review at
52d999609(rebase re-verified as a no-op at the time — the branch then sat exactly on thepinvou3-cleantip, which is ahead ofmainin this fork). One blocker plus follow-ups, each fixed against code evidence:ba047292d— shell working-dir recovery advice made true on every surface (round-3 blocker): "resume/fork the session from an existing workspace" was false — resume re-anchors to the same saved (missing) directory on every surface (TUI restore and headless resume both run the saved pair, so the advice looped), a CLI fork copies the saved workspace, and "remap" named no engine facility. The advice now splits on the same source flag as the label: recreate-or-switch for the saved session workspace (naming/workspace <path>and/forkafter switching — the real command names;/cddoes not exist), and recreate-or-use an existing directory for a requested working directory (the surfaces whose schemas advertisecwd, such as the task and verifier gates). A new test pins the not-a-directory branch and the absence of the false resume/fork advice. (Round-6 note: tool callers always resolve a concrete working directory, so today the rendered arm is the requested-directory one; the saved-workspace arm is truthful but currently reachable only from direct shell-manager callers — disclosed below.)6c6937860— trust_mode's second effect disclosed: under auto_approve, a tool call the sandbox denies is retried with full access instead of being denied (approval_decisionmaps trust_mode toRetryWithFullAccess). One-line schema addition completing the interplay disclosures alongside the mode row.fc416b7c4— background gates return a task id per gate, null when a gate is skipped or fails to start; the copy no longer implies every gate gets an id.36636d20c— the hiddenrun_verifiersschema's unscoped "not through a shell" sentence now matches the visible Run schema's per-path wording (background gates render through the detected platform shell). Its background description is unchanged —task_idsand the Bash action-wait wording are accurate for that surface's metadata-level consumers.95fbd6ebc— read_media budget copy reframed as a budget ("fit a ~256KB delivery budget"), matching the low clause and docs; small images delivering far less than 256KB are no longer implied to be atypical.1eb944fb1— new test pins the crop carve-out (crop+auto delivering past the ~256KB detail budget within the 5 MiB wire budget); mutation-proven RED when the crop arm is inverted, GREEN restored.bf1ab736c— prompts docs: the volatile-content-last fragment list now names the continual-harness block and the imported project-instructions fragment the builder really adds, and the module header no longer claims every compile-time constant lives in thetextmodule (authority recap and locale bookends live in prompts.rs).Also disclosed for completeness (round-3 finding, no code change): empty fim anchors — accepted at base and edited at the first char boundary — are now refused with their boundary count ("matches N locations"), consistent with the exactly-once contract; the count is non-overlapping, so "locations" means non-overlapping sites, not start positions. Two corners pass through with count 1 and keep the base behavior: an empty suffix after a prefix ending at EOF, and an empty prefix on an empty file (round-4 review; now pinned by tests — the wave-4 note had claimed this pinning, but the tests only landed in wave 5).
Fourth review fix wave (2026-11-04, round 4)
From a fourth from-scratch 14-lane re-review at
bf1ab736c(rebase re-verified as a no-op at the time; zero P0/P1 across the review, fork-guard registry fully intact). Body corrections plus residual-copy and pin fixes, each verified against code evidence:c27064ace— "out-of-range values are clamped, not rejected" was false for negative integers: the u64 parser rejects them as a type mismatch (as_u64,type_mismatch) and they never reach a clamp. Per-field boundary copy instead (clamped into the declared range; below-1 clamp for the attempt fields; the negative/non-integer rejection named), plus pins for the corrected sentences and for the reviewed authority claims (plan blocks not lifted by auto_approve, durable-task allow-shell fallback, trust_mode full-access retry, BLOCKED gate refusal) that had no schema pin.da8a0276f— the read_media result receipt printed the pre-base64 encoded size as "(wire payload)"; the actual wire payload is the base64 form (~4/3 larger, as the image-attach code's base64 note and docs/READ_MEDIA.md's wire-size bullet describe). Now "(encoded payload)".0b09401aa— three header comments survived the assembly-layer cleanup: the prompts.rs section header still said "every bundled prompt layer lives in text.rs" (the authority recap and locale bookends are defined in prompts.rs), the handoff doc still said the relay artifact is "prepended" to the system prompt (it is routed as the late WorldState token-budget/continuity fragment), and the environment-block doc still claimed placement "before configured instructions / skills" (it leads the workspace WorldState fragment, assembled after the constitution prefix). The text.rs module header aligned with the corrected prompts.rs header.bcb0dae84— fim counting edges pinned: an empty prefix is refused with the char-boundary count (characters + 1, not bytes + 1 — the multi-byte fixture discriminates a byte-offset refactor); suffix occurrences before the prefix are invisible to the uniqueness check; a suffix that only occurs before the prefix stays SuffixNotFound.8d0e997cb— background task-id null arms pinned throughstart_background_gates: a gate without a program reportsskippedwith its reason, and a gate whose working directory does not exist fails shell-manager validation and reportsfailed_to_startwith the accurate error.bfee93377— the load-bearing Run/bash/read copy sentences pinned against silent reverts: the per-path shell scoping (foreground direct spawn vs. background render through the detected platform shell, POSIX-quoting-under-PowerShell caveat), the task-id-null clause (and the absence of a literaltask_idsfield in the visible copy), the bash merged-stream/fresh-shell claims (and the absence of a separate-stream promise), and the read image/BMP/UTF-8 disclosures (and the absence of a text-only claim).5add71c4a— the four gen-13 superseded digests anchored to embedded generation-12 fixtures, hashed against their registered entries on every run (the existing shape test only rejects digest == current, so a single wrong hex digit would have silently fossilized real generation-12 installs); the behavioral supersede check rides along.Also disclosed for completeness (round-4 findings, no code change):
modeproperty advertises a closed enum (agent/plan/operate), but execution passes the string through toRuntimePolicyProjection::from_request, which still accepts the legacy aliases (yolo/4/bypass/bypass-permissions/bypasspermissions) and maps them toApprovalMode::Bypasseven whenauto_approveis false or omitted; unknown modes fail closed. Enforcing the advertised enum (or disclosing the aliases in the schema) is a behavioral decision, separate follow-up (runtime_policy.rs).pr_attempt_recordvalues above u32::MAX forattempt_index/attempt_countwrap throughas u32after the below-1 clamp (e.g. 4294967296 records as 0, violating the advertised minimum); absurd-input hardening, separate follow-up (tasks.rs).Fifth review fix wave (2026-11-04, round 5)
From a fifth from-scratch 16-lane re-review at
5add71c4a(rebase re-verified as a no-op at the time — the base tip was still the merge base; zero P0/P1 across the review; smuggling verdict CLEAN — every hunk of the then-38 commits attributed, linear history, one topic each; performance NEUTRAL — per-turn catalog strings are memoized per registry build, the fim scans run before a network call, and the startup digest hashing is a one-time microsecond cost). Three P2s and three in-theme residuals fixed, each verified against code evidence:33395ff9d— the model-visibleharnesstool no longer teaches the hiddenrlmsurface: the description's "Keep large source material in rlm" clause and the overview result'srlmruntime-context line named a compatibility-only tool (model_visible() == false, and its own description says not to teach it to new model turns) — the exact phantom class item 3 fixed in the v4 skill and that the PHANTOM_NAMES sweep guards; allowlist-deriving hosts reject the very call the copy recommended. The copy now names the visible bounded-reread surfaces (readwith offset/limit,retrieve_tool_result). (The removal went in unpinned; the round-6 wave adds the pin — see wave 6.)d6d957228— the fim "exactly once" copy now discloses the non-overlapping counting rule ("counted without overlap" in the tool description and both anchor schema descriptions), matching the siblingedittool's "unique, non-overlapping region" wording; a new copy test pins all three sentences. (Formerly the disclosed out-of-scope bullet below — now fixed.)279b313c3— the two fim pass-through corners are now pinned by tests: an empty prefix on an empty file proceeds to the API boundary (count 1 at the single char boundary) and an empty suffix after a prefix ending at EOF proceeds (count 1 in the empty region). The round-4 note had claimed this pinning existed; it did not — the tests land here.3f07d01cb— the read_media receipt's "(encoded payload)" label is pinned, with the absence of "(wire payload)" asserted — the one load-bearing round-4 copy change no test guarded.8b6d17e0f—allow_shell's "Passing true or false is always honored" absolute is scoped to the mode ("honored subject to the mode — plan keeps shell disabled even when this is true"), matching the mode property's own precedence disclosure; both sentences pinned in the authority-copy test.71b61ea5d— prompts.rs: the module header no longer ends on the stale "Keeping every layer's text in one module makes prompt tuning a single-file operation." sentence that contradicted the corrected constant-split header directly above it, and the handoff test whose name still said "prepended" (the doc saying that was corrected in wave 4; the test asserts containment only) is renamed to what it actually pins.Also disclosed for completeness (round-5 finding, no code change): the skills refresh path treats an unreadable or non-UTF-8 installed SKILL.md as empty (
read_to_string(...).unwrap_or_default()), so such a file is refreshed without the superseded-digest user-edit check — pre-existinginstall_onebehavior exercised by every generation bump, adjacent hardening, separate follow-up.Sixth review fix wave (2026-11-05, round 6)
From a sixth from-scratch 15-lane re-review (smuggling CLEAN — every hunk of all 44 then-commits attributed across the 27-file delta; body-truth audit; commit-hygiene audit; base-interaction audit; mutation suite; performance re-measurement), run after rebasing the branch onto the
pinvou3-cleantip11cba403d, which had moved seven commits ahead (subagent claim/worktree isolation #123/#124, run-completion backport #129, guardrail prompts #131, fetch_url copy drop #133, locale-pack mode-copy drop #134, dependency bumps #135). The rebase replayed all 44 commits with zero conflicts, and the moved head was verified to differ from the pre-rebase head by exactly the base delta (line-set identity). The moved base both answered prior interaction questions (the new shell kill-path, authority-transition notice, fetch_url and locale copy drops do not contradict any corrected copy; guardrail arming was checked against the unarmed default and its armed-only exception was missed — corrected in wave 7) and introduced one real contradiction on this PR's own surface, fixed with two residuals:4a36f4be6— read decode clause scoped to the new binary refusal: the subagent-isolation work addedcontract_read_payload_is_binary(a NUL byte anywhere in the first 8 KiB sniff window, or a leading UTF-16 byte-order mark → refused with extractor suggestions before any decoding), which made this PR's own "bytes that are not valid UTF-8 become replacement characters instead of failing the read" absolute false at the new tip — stray high bytes and NUL-free heads still decode lossily; the refused classes do not. The clause now carries the refusal exception; the description pin asserts the exception and forbids the unscoped sentence end from returning; and the refusal message's own "read returns text only" absolute (the same tool returns inline images for sniffed formats) is corrected to "read returns text or inline images, never raw binary", pinned in the behavioral refusal test. All three changes mutation-proven RED/GREEN.bcf944c59— the wave-5 harness fix is now pinned: the rlm removal landed with copy only, and the round-6 mutation suite showed re-adding both original rlm mentions passed every guard and suite — the one unpinned copy fix in the PR. A new test pins the absence ofrlmand the presence of the visible bounded-reread surfaces (readwith offset/limit,retrieve_tool_result) in both model-visible strings (the description and the overview result's runtime context); mutation-proven RED when either mention returns.5fbf16822— two header residuals the cleanup missed: the consolidation sentence still claimed in the present tense that the whole prompt contract reads top-to-bottom in this one module, contradicting the corrected split header directly above it (the authority recap and locale bookends never lived here — the claim is now scoped to the constants that moved); and the header's edit-loop command rancargo test -p codewhale-tui --bin codewhale-tui prompts, which matches zero tests (the prompts tests live in the lib target) — it names--libnow.All 53 commits carry DCO
Signed-off-by, one per topic.Seventh review fix wave (2026-11-06, round 7)
From a seventh from-scratch 18-lane re-review — per-surface truth lanes for read/bash/fim/Run/tasks/skills/prompts, a full hunk-by-hunk commit attribution audit, a body-truth audit (all 41 cited SHAs, all counts, the recovered pre-rebase chain and its line-set identity), an independent base-interaction audit, 16 mutation probes, a full 12,448-test suite run, and fork-guard — at the unchanged
pinvou3-cleantip11cba403d(the rebase check was a verified no-op; CI green). The review confirmed every earlier wave's fixes and found one real contradiction, two pin gaps, and two schema residues, each fixed against code evidence:a326ba9ce— the auto-approve copy now carries the armed-guardrail exception (round-7 blocker): base feat(engine): host-armed guardrail prompts + authority-transition notice #131's host-armedask_on_guardrailconsent path changed the approval pump toauto_approve && !approval_force_prompt(runtime_threads.rs:9498, pinned by the base'sforkguard_pump_never_auto_approves_a_force_prompt_hold), so on a host that arms guardrail prompts, guardrail-class calls (the non-bypassable tool set, typed ask rules, repo-law holds, sandbox escalation) force a real human approval even for anauto_approve=truetask — falsifying this PR's own "no approval prompts" absolute and the mode row's "which otherwise skips approval prompts" parenthetical (both written 2026-11-02, three days before feat(engine): host-armed guardrail prompts + authority-transition notice #131 landed). The round-6 audit had listed "guardrail arming" among base changes that "do not contradict any corrected copy" — that check was answered for the unarmed default and missed the armed-only exception, so the fix lands in this PR rather than a disclosure, per the wave-6 precedent. The mode row also now discloses that trust_mode's effects still apply in plan (the sandbox-retry path has no mode re-check — the "'plan' runs read-only" corner), and the pins assert both exception clauses while forbidding the unscoped absolute from returning (mutation-proven RED).4f00c64fe— the tasks description's "Use task_shell_start for long-running shell work." pointer is scoped to the surfaces that register the tool (allow_shell on, non-plan mode — the registry gates the runtime task shell tools behindShellPolicy::Full), and thetask_idproperty no longer lists read among the actions requiring an id (only cancel does; read andpr_attempt_*resolve through the same active-task default). Both pinned, mutation-proven.7936c7eff— the read_media delivery-budget sentences (the direct item-5b fix) are pinned against silent reverts; the round-7 mutation suite showed a probe flipping "~256KB" to "~2MB" passed the whole suite before this pin.8374e64c8— a behavioral pin for the fim non-overlapping counting rule: a self-overlapping prefix ("aa" in "aaa") has exactly one non-overlapping site and reaches the API boundary; an overlapping counter would refuse (mutation-proven RED via the same guard).64202f1d9— the shell working-dir advice pin is hardened: the requested-directory arm (the model-reachable one) now forbids resume/fork wording outright, and the saved-workspace not-a-directory branch gets its own test. Mutation-proven RED for a paraphrased revival of the false advice.eab45d4aa— two constant-doc claims the header cleanup missed: BASE_PROMPT's doc summary listed sections the constant does not have (output format, toolbox reference, "When NOT to use", sub-agent sentinel protocol live in other layers), and the AUTHORITY_RECAP doc said it is "appended at the end of the system prompt" when the locale closer follows it for non-English locales (it is a per-request trailer after the WorldState fragments).Also reviewed and left unchanged: the trust_mode sentence's deny-retry face is a different flow from the armed approval-card face (the
SandboxDeniedretry path does not consult arming), so its wording stands; the run/verifier hidden-schema per-path wording matches the visible copy in substance though it omits the PowerShell parenthetical; and the harness rlm, digest-fixture, and bash/read pins all held under the fresh mutation probes (13/16 RED; the three GREEN gaps are the ones this wave closes).Deliberately out of scope (disclosed, not changed)
args/all_featureshard-fail underaction=verifiers(deny_unknown_fields). The visible schema already carries the two load-bearing semantic clauses — "(action=tests; rejected by action=verifiers)" on both descriptions, landed via fix(tui): align model-facing text with actual behavior #112 and pre-existing on this base; the advertise-then-reject shape of those two test-only params is a behavioral question left for a separate decision.TaskRecordrestore defaults a pre-field record'sauto_approveto true (default_auto_approve, task_manager.rs:3105) — a fail-open restore path adjacent to the GHSA narrative; behavioral decision, separate follow-up.modeenum is advertised but not enforced (legacy aliases escalate to Bypass; see the fourth-wave disclosures above) — behavioral follow-up. Thegateproperty is the same advertise-not-enforced class: any string is accepted and only clippy/test get dedicated failure classification — same follow-up batch.model_visible() == false). Same copy class as item 5c, but each needs its execution path traced before truthful copy can be written — follow-up batch, not risked here.run_gatemisreports a nonexistent gatecwdas "{program}is not installed or not in PATH" (verifier.rs:1167-1181) while the background path errors accurately — pre-existing behavioral asymmetry, adjacent to the Run schema copy; separate behavioral fix.exec_shellextensively as a live tool name; that is user documentation, not model-facing copy — separate docs follow-up.exec_shell_cancel" (subagent/mod.rs write-scope contention and bounded-file-target refusals) — no tool is registered under that name (it is a canonical-action label), and the engine's own unknown-tool guidance points attool_searchinstead;task_shell_start's relayed background results and the foreground timeout-recovery metadata nameBash/hidden-waiter recovery paths on surfaces whose published waiter istask_shell_wait; the hidden verifierleveldescription's quick/full split is inverted for Go (quick already runsgo test ./...—add_go_gatesadds the test gate at every level); and the MCP relay's default expose serves the legacyread_file/write_filecopy to external clients (item 1's matrix note) — legacy-copy class, separate decision.primitive_image_mime): a text file beginning with the literal bytes "BM" is routed to the image arm and returned as an omission note. Pre-existing sniff behavior, adjacent to item 1's BMP clause.model="deepseek-v4-flash"(parse-accepted but unadvertised on the agent schema),type="verifier"(a legacy alias; the advertised enum spells ittest), and "(thinking off)" prose the example call does not realize — rewording the body would require a fresh digest pin for the intermediate shipped bytes; follow-up./workspace//forksuggestions) is currently unreachable from model-facing tool paths: every tool caller resolves a concrete working directory (an omitted cwd falls back to the session workspace, which then validates as a requested directory — truthful and actionable, but the/workspace//forkadvice renders only for direct shell-manager callers today). The false round-2 advice is gone from every arm; the reachability asymmetry is a behavioral follow-up.allow_shell's durable-task fallback resolves against the TaskManager's startup-time configuration snapshot: a mid-session config change reaches newly created tasks only after a restart. The schema copy describes the mechanism, not the snapshot freshness; freshness disclosure is a follow-up.max_tokenstruncates throughas u32(4294967296 reaches the API as 0) — same absurd-input hardening class as the disclosed attempt-field wrap, separate follow-up.Verification
cargo fmt --all -- --check: clean (re-run after the sixth wave).5fbf16822, including the round-6 read-refusal pin and the harness rlm pin. (The 719 figure quoted in earlier rounds was recorded at the pre-rebase base with a different filter composition; module-filter counts drift with the base's own test population — zero failures at every round.)cargo test -p codewhale-tui --lib --locked -- --test-threads=1 forkguard: 285 passed / 0 failed post-round-6 (up from 240 at the previous base entirely via base-inherited tests; this branch adds no forkguard tests). At default parallelism the forkguard filter currently deadlocks on the reference machine — an ABBA ordering between the tests' sharedtest_env_lockand engine-state locks, inherited from base-side commits, reproducible independently of this branch; single-threaded is how fork-guard.sh itself runs this lane, and it is green there.no_advertised_tool_teaches_a_retired_nameguard is green (it lives intools::canonical_action, not under theforkguardname filter).cargo clippy -p codewhale-tui --lib --all-features --locked: byte-identical between base11cba403dand head under local clippy 1.99.1 — eight pre-existing diagnostics (sixclippy::too_many_arguments, twoclippy::collapsible_if), none introduced or touched by any wave. (The "six diagnostics" figure in earlier rounds was recorded against the previous base f1eb10e and is base-dependent; the exact command exits 101 on both trees — a pre-existing workspace lint deny, unchanged by this branch.)runtime_api::tests::events_endpoint_respects_since_seq_cursorbefore reaching a tally; the identical abort reproduces standalone on a clean checkout of the base, so it is machine-specific and not introduced by this branch (fork CI, which runs the full gate, is green on this branch). The earlier post-wave-2 tally — 12,292 tests, 12,275 passed / 4 failed / 13 ignored, the four failures being machine-specific load-order flakes (remote_control×3,client::operation_tests×1) that pass in isolation — was recorded on this machine before that abort appeared.upgrade_from_v4_installs_pack_and_retires_unchanged_v4_best_practicesRED, and a one-byte fixture edit turns the same test RED at the pinned-digest self-check. The round-2 review's 9-mutation suite (fim refusal trio, retirement repair, phantom-name sweep, canonical Run-schema retired-name guard, lsp pin) was RED/GREEN as designed. The round-6 review ran a 13-probe suite over the pre-wave-6 head: 12 RED as designed and one GREEN — the unpinned wave-5 harness rlm removal, which the wave-6 pin now closes (re-proven RED after the wave).11cba403dand head: the 27 registered needles touching files this branch modifies are all present at head, and the one needle absent from its registered file at head (complete returned tool result at, registered oncore/engine/context.rs) is an inherited base-side relocation — base-side commit fix(subagent): 固定任务身份与完整输出恢复 #124 moved the string totools/truncate.rs, and it is absent from the new base as well, so it is a parent-repo registry re-point, not a branch regression. Theforkguard_*identifier floor (64) holds at 301 at head (255 at the previous base; the growth is entirely base-inherited).No-Issue: issues are disabled on this repository; audit-batch corrections to already-shipped model-facing text, tracked through the review rounds in this PR.