Skip to content

fix(fleet): the test role states its real contract - #140

Open
asto18089 wants to merge 4 commits into
fix/live-test-findings-subagent-shellfrom
fix/fleet-test-role-contract
Open

asto18089 wants to merge 4 commits into
fix/live-test-findings-subagent-shellfrom
fix/fleet-test-role-contract

Conversation

@asto18089

@asto18089 asto18089 commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Slice 3 of the #126 split, based on #126 (slice 2, the bounded read-only probe lane) which is based on #139 (slice 1, stdin EOF). Full pre-split content preserved on archive/p126-pre-split-8705ccd85; the slice commit as first pushed (fe98f4454912) reproduced the reviewed combined tree byte-exactly (8705ccd852052), and slices 1+2+3 then reproduced the full pre-split content exactly. The stack was subsequently re-pushed twice with reworked slice-1/2 content; at each re-push every file this slice's own changes touch was verified content-identical (the current re-push additionally folds slice-2's worktree-lane rework into files this slice shares — subagent/mod.rs, worker_runtime.rs, the docs — without changing this slice's authority semantics), and the review-fix waves are re-parented on the current stack tip: round-1 fixes as d75b7ac83, round-2 fixes as 253f5faaf, round-3 fixes as the current head. The stack moved twice during the round-3 review: first onto #126's 8f98b3e89 (rebase replayed byte-exactly — the combined diff against the new base was identical in this slice's files), then again as #126's 1f9635a894 (slice-2's round-20 wave plus guardrail plumbing elsewhere in the stack; the rebase kept this slice's content and resolved its one subagent/tests.rs overlap by keeping the stack's new forkguard test), and every receipt below was re-proven at that final tip.

3. The test role promised test runs it could not perform — and had no shell path at all

Live evidence: the roster advertised test as "Runs the test suite and validation gates, reports pass/fail with evidence." Both test children in the session could run built-in gates only, reported BLOCKED honestly instead of lying (to their credit), and could not even gather evidence: the raw-shell clamp denied every Bash spelling, and the bounded read-only lane admitted only Scout/Reviewer/Planner — wc -c, ls, anything: no path through at all. The same session's reviewer child reported a 33-line file that wc -l says is 35 lines.

Fix:

  • the roster description, the type schema description, VERIFIER_AGENT_INTRO, and both SUBAGENTS role tables now state the real contract: built-in verification gates with test-selection arguments, plus a bounded read-only shell for evidence gathering; custom commands are refused — use a write-capable role for those (custom-command execution remains a deliberate posture boundary, as fix(subagent): release interrupted claims and inherit worktree isolation #123 already disclosed);
  • the Verifier joins role_requires_read_only_shell and allows_bounded_readonly_bash (which now reads the role set from that one predicate instead of enumerating it again), gaining the same classifier-bounded probe lane as the inspection roles; shell authority (holds_shell_authority, behind posture_permits_tool's Required branch and shell_is_denied) keys on the held shell policy plus one named exception — the Verifier keeps the built-in Run surface (default/selection-bounded, reject_unbounded_verification still in force) through its read-only narrowing, because running the workspace's own gates is the role's purpose — pinned by verifier_child_keeps_the_default_and_selection_bounded_run_surface;
  • the machine catalog a child sees now omits bash/Bash whenever the envelope's shell authority is not ReadOnly (tools/registry.rs), so catalog visibility agrees with the dispatch wall (enforce_tool_authority) for every shell-less envelope — this also removes the bash entry from the fleet consultant's machine catalog, which over-advertised a surface its envelope refuses; pinned by machine_catalog_omits_bash_without_shell_authority;
  • review round 2 found the first cut's re-key (on role_posture_permits, i.e. the role's default posture) flipped shell authority for every role whose default shell is full, so on the plain agent nesting path a Builder or Custom child of a read-only parent started the test runner its parent cannot use. The carve-out is now Verifier-only; every other narrowed role holds no process authority, as before — pinned by read_only_shell_children_of_other_roles_hold_no_shell_authority (Builder/Worker/Custom with a read-only shell: Run refused at posture and envelope, absent from the catalog). Scout/Reviewer still cannot dispatch Run (bounded_inspection), planner/advisor still reach no process tool, and legacy Bash stays denied for the Verifier behind two independent walls (pinned);
  • fleet receipts now report the verifier's effective shell as read_only (the old full label overclaimed a raw shell the clamp always denied), and the narrowed-shell label pin covers the verifier.

The docs also gain the classifier's compound-segment and cd-position grammar sentences (en+zh) — slice-2 behavior this slice's role tables sit next to, documented for completeness.

The foreground stdin contract from fix 1 has one visible consequence documented and pinned in slice 1 (detached_foreground_command_reports_its_closed_stdin_on_interact in shell/tests.rs): a foreground command later detached to the background keeps running with its stdin at end-of-file, and interact on it says so and points at background=true instead of failing with an opaque "stdin is not available". A reviewer-prompt sentence that an earlier revision bundled in was dropped as out of scope; it can be proposed on its own.

Round-3's authority escape and its re-key fix are pinned here (they are test-role behavior): a Verifier child of a read-only role parent must not keep the built-in Run surface because the carve-out keyed on the role alone — the registry records whether the child's shell was full after the parent clamp, and holds_shell_authority keys on that grant; the chain pin covers the propagation side (hop 2 is spawned through the carve-out verifier's real agent tool and read back from production's own record), the capture ordering itself is covered by the carve-out pins.

Review round 1 (2026-11-04)

Fresh 6-lane review found no functional, security, or performance defect and no smuggled scope; the actionable findings are fixed in this branch (first pushed as ae52233aac, re-created on the current stack as 15b563920):

  • Gates conditionality stated correctly (was the PR's own remaining overclaim): a Verifier child of a read-only-shell parent keeps the probe lane and loses the gates — the exact shape clamped_verifier_child_of_read_only_parent_holds_no_run_surface pins. The gates clause now carries that conditional in intro sentence 1, the roster description, the type description, and the docs bullet + role tables (en+zh); the in-code copy pins are extended (the markdown docs edits themselves are not test-pinned, as docs normally are).
  • The chain test chains: sub_subagent_chain_of_a_carveout_verifier_holds_no_extra_authority builds hop 2 under the carve-out verifier's published narrowed profile and asserts the Builder below holds no gate surface and no lane at posture/envelope/catalog/dispatch. The stale "covers the re-key ordering" claim is gone.
  • Lane-absence pins for non-verifier children: Builder/Worker/Custom with a read-only shell pin bash's absence from their catalogs — a widened role set must not silently hand the classifier-bounded shell to write-capable children (widening mutation reddens both this and the chain pin).
  • Docs consistency: the explore row (en) and scout row (zh) drop the stale "bounded verify" claim the reviewer-row fix left behind; the scout brief attributes the probe lane, not the verification surface (en+zh); the fleet-envelope contract statement gains its English counterpart.
  • Copy honesty sweep: fleet roster blurb and FleetRole::Verifier rustdoc state the real contract; network_posture_warning_for_task names the effective (narrowed) shell like the receipts do; three stale comments updated (role_posture_permits docstring, the narrowing comment, the delegation-refusal comment).
  • Test walls corrected: the legacy-Bash alias provability pin moved to the dispatch wall that owns it (posture's Auto classification admits provable shapes for any role), and the half-wrong "at any provability" comment fixed.

Review round 2 (2026-11-05)

Fresh 6-lane review at the rebased head (claims/root-cause, elegance, smuggle audit, authority-model bug hunt, perf + test quality, docs/i18n parity) again found no functional, security, or performance defect and no smuggled scope; the authority matrix (role × parent-shell, all four walls plus fleet/in-session split, alias and case escapes, sentinel interplay, mid-session posture flips) came back clean. The actionable findings are fixed in the round-2 commit:

  • The chain pin now drives the real spawn path. The round-1 rewrite still derived hop 2 from runtime_profile — the field set at registry construction — so the one production line that actually publishes the narrowed profile to the verifier's spawns (child_runtime.worker_profile) had no pin: reverting that publication line alone left every test green while a chain Builder regained the gate surface. The pin now starts the Builder through the verifier's own agent tool (an auto-approving fixture variant) and reads the child's recorded runtime_profile — production's worker_profile_for_spawn → derive_child output — before asserting the no-Run, lane-less shape below. Mutation-proven: the publication-line revert reddens exactly this test.
  • The agent tool description's test blurb states the real contract: it still said "runs tests and reports evidence" — the same unconditioned family this PR calls an overclaim elsewhere, on the third model-facing surface the repo's own test names ("the roster entry, the type enum text and the tool description are the three places a model reads before picking"). Now "built-in verification gates; custom commands refused", with the canonical-vocabulary pin extended and the description's 1024-token budget guard still green.
  • Diff hygiene: dropped the 570 pure blank-line insertions that churned subagent/tests.rs (56% of its diff, not required by any formatter; the base file's style has none).
  • Comment honesty: the fleet receipt comment now names the verifier's real fleet surface (gates with no bash at all — the probe lane is in-session only); the executor envelope comment no longer implies a fleet member holds the lane; worker_profile's scout arm no longer claims "the bounded verification surface" for scout/reviewer.
  • Docs: the zh verifier bullet and zh FLEET envelope paragraph use the established 信封 term (the same PR used both spellings), the zh FLEET lane claim carries the envelope-grant qualifier its en counterpart has, and the ChildAuthority::clamp path reference points at fleet/role.rs (en+zh; the impl never lived in fleet/exact.rs — only its pinning test does).
  • Claim corrections: network_posture_warning_for_task's effective-shell label change (round 1) is kept but disclosed for what it is — every built-in role defaults network-on, so the warning's Some branch is unreachable today and no discriminating test can exist without a production change; the label alignment is for future network-off roles, matching the receipts.

Review round 3 (2026-11-05)

Fresh from-scratch 9-lane review at the re-parented head (claims/root-cause completeness, spawn-path authority, wall/fleet authority matrix, smuggle audit, elegance/wheel-reuse, feature-behavior, performance, test quality + mutation gaps, docs/copy en↔zh parity) again found no functional, security, or performance defect and no smuggled scope — all 60 diff hunks map to a claimed purpose, the Verifier carve-out stays grant-gated and Verifier-only at every wall, catalog and dispatch agree at both layers, and every behavioral delta is an O(1) predicate change. The actionable findings are fixed in the round-3 commit:

  • The fleet roster slot blurb is pinned. The blurb this PR rewrites (FleetRoster::built_in_members' verifier slot, a model-facing surface via AgentProfile.description) had no test — reverting it left every test green. The copy pin now covers it (anchors: built-in gates, holds no shell, no "builds and tests" re-overclaim); mutation-proven: revert → exactly 1 red.
  • The type description's contract tail is pinned. Only its prefix was anchored; dropping the "custom commands are refused" / parent-clamp clauses stayed green. The pin now anchors both clauses; mutation-proven: drop → exactly 1 red.
  • The missed user-facing surface fixed: the "Useful roles" tables in docs/GUIDE.md (en) and docs/zh_hans/GUIDE.md still described the verifier as "running checks" — the same unconditioned family this PR calls an overclaim everywhere else. Both rows now say the built-in verification gates (en) / 内置验证门禁 (zh).
  • Diff hygiene completed: the 11 residual standalone blank-line separators in subagent/tests.rs that round 2's strip predicate missed (item heads mod/const/impl/// were not in its pattern) are gone; 6 blank lines that belong to the new tests' own code remain.
  • Claim correction: the capture-ordering mutation receipt below said 4 red; re-run at this head it is 7 red (it also flips the run-surface pin, the ceiling pin, and the legacy-refusal pin — over-determined, which is the safe direction).
  • Disclosures: (1) the machine-catalog conjunct also hides bash/Bash from the ACP plan-mode catalog (the other production tool_authority site — same shell-less shape, previously advertised-but-always-refused, now hidden; strict alignment, same authority-keyed mechanism as the fleet consultant). (2) the agent tool description now sits at exactly the 1024-token conservative-estimate cap — the next copy edit anywhere in that description must shorten something else first. (3) docs/FLEET.md (en) has no fleet-envelope paragraph at all while docs/zh_hans/FLEET.md has one (pre-existing asymmetry); this PR edits only the existing zh paragraph and does not author a new en one.

The stack's second move landed mid-verification; every receipt was re-proven at the final tip: the targeted suites pass 1049 / 0 there (the rework adds two suites), the two new copy pins' mutations were proven at the round-3 head, and the capture-ordering mutation (7 red) and the spawn-publication mutation (exactly 1 red: the chain pin) both redden identically after the rebase.

Merge order

Based on #126 (slice 2); GitHub retargets this PR to pinvou3-clean when that merges. Shared-file positions against #123/#124/#136 are re-swept in this slice's own review round; the #123 refusal-copy conflict and the shell.rs git-hardening table conflict live in slice 2, not here.

Testing

  • Targeted suites (tools::subagent:: + tools::registry:: + fleet::) at the round-2 fix head on the previous stack tip: 1019 passed / 0 failed; re-run at the same content re-parented on the current stack tip: 1047 passed / 0 failed (the rework adds suites).
  • cargo fmt --all -- --check — clean; fork-CI-exact clippy (--workspace --all-features --locked + allows + -D warnings) — clean at both fix heads.
  • Full cargo test -p codewhale-tui --locked --lib (RUST_MIN_STACK=16777216) at the round-2 fix head: 12429 passed / 0 failed / 13 ignored.
  • Tree assertion: fe98f4454912^{tree} == 8705ccd8520522192a78a946f3a9f1b751ec44d7 (the reviewed fix(shell): compound read-only probes behind the operand gates #126 head tree) — the split reproduced the full pre-split content exactly at that commit; the review-fix waves then intentionally diverge in the files they fix.
  • Mutations, each reddening exactly its own pins (baseline green after each revert), re-proven at the round-2 head:
    • the spawn-publication line reverted to the parent-runtime profile → exactly 1 red: the end-to-end chain test (this is the round-2 pin; under the round-1 test shape this mutation stayed green);
    • grant captured after the narrowing → 7 red (corrected in round 3 from the earlier 4-red receipt: the carve-out pins plus the run-surface pin, the ceiling pin, the catalog pin, and the legacy-refusal pin);
    • role-set widened (Builder added to role_requires_read_only_shell) → 11 red incl. both catalog lane-absence pins and the chain pin;
    • the agent-description blurb reverted to the stale copy → exactly 1 red: the canonical-vocabulary pin;
    • (earlier rounds) clamp ignored at the recording site → 1 red: the clamped-child test; machine-catalog conjunct reverted → exactly 1 red with the scout twin green.
  • Round 3 (at the round-3 fix head on the first rebase, then re-proven at the final re-parented tip after the stack moved again): targeted suites 1047 passed / 0 failed and 1049 passed / 0 failed; cargo fmt --all -- --check clean; CI-exact clippy (--workspace --all-targets --all-features --locked + the CI's three allowances + -D warnings) reports 3 collapsible_if in core/engine/turn_loop.rs, tools/subagent/mod.rs (~:8036) and tools/verifier.rs — all three shapes are byte-identical on the base branch and all three sites are outside this PR's diff (inherited from the stack, fail the release gate, not this PR's scope); the two new copy pins are mutation-proven (roster-blurb revert → exactly 1 red; type-description clause drop → exactly 1 red).
  • Full cargo test -p codewhale-tui --locked --lib (RUST_MIN_STACK=16777216) at the final re-parented head: 12545 passed / 0 failed / 13 ignored (run with -- --test-threads=4: with libtest's default parallelism the run stalls in the parallel core::engine::forkguard_* family on this board class — those tests are green both in isolation and in the capped run; environment interaction, not content).
  • The deleted no-shell verifier catalog test is replaced by the probe-lane variant; role/carve-out/chain pins carry the round-3 and round-13 findings, and the review fixes carry the fresh-review findings.

Checklist

  • Docs updated (docs/SUBAGENTS.md, docs/zh_hans/SUBAGENTS.md, docs/zh_hans/FLEET.md, roster/schema/intro strings, refusal-adjacent copy).
  • Tests added for the carve-out, receipts label, catalog honesty, and the spawn-publication chain.
  • Verified TUI behavior manually if UI changes (no UI change).

No-Issue: slice 3 of the #126 split; tracked in the parent PR (#126), no dedicated issue.

@asto18089 asto18089 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fresh review (round 1 of this slice) — approve-grade, one accuracy fix + two test-strength fixes requested before merge

Fresh 6-lane review (authority model / fleet integration / subagent intro + shell surface / test quality / docs / scope audit) plus an independent build lane and 4 executed mutations. Head fe98f4454912 re-verified anchored: 139 (e13a366fd, on main f1eb10efc) → 126 (f5aaaafd4) → 140, and HEAD^{tree} = 8705ccd852052… exactly as the body claims — the slice stack reproduces the combined tree byte-for-byte. No rebase was needed (parent chain is current).

What I verified as real and correct

  • Root cause fixed at the source, not cosmetically. Pre-PR, the test role promised test runs it couldn't perform and had zero shell path — confirmed independently: role_requires_read_only_shell excluded Verifier, allows_bounded_readonly_bash's own literal excluded it, and role_blocks_unhardened_process_tool treated lowercase bash as raw shell, so even a classifier-provable pwd was denied. The PR fixes both halves at the predicate level: contract copy (roster description, SUBAGENT_TYPE_DESCRIPTION, VERIFIER_AGENT_INTRO, role tables en+zh) now states the real surface, and the Verifier joins the existing classifier-bounded lane with no second grammar invented — same predicate at posture, envelope, and execute, same operand gates and budgets as Scout/Reviewer/Planner.
  • Authority model holds. The grant sentinel (parent_shell_authority) is captured after the parent clamp and before role narrowing (mod.rs:14656-14669), the carve-out is Verifier-only and requires the recorded grant (holds_shell_authority, mod.rs:15446-15451), the ceiling published to spawns reads the effective shell, and legacy Bash stays denied behind two genuinely independent walls (role/name clamp + classifier-lane rule; the deny list is a third belt). I could not construct an authority escape: V→V→V chains lose the grant at every hop because the recording input is the parent's effective narrowed profile; the fleet machine path never sees the sentinel at all (capability envelope + catalog filter). The dropped role_posture_permits(Required) conjunct is behavior-equivalent on every reachable state (enumerated all roles). reject_unbounded_verification stays in force; the run_tests operator sentinel still overrides the carve-out.
  • All 7 body claims TRUE-IN-DIFF (scope audit, hunk-by-hunk). Smuggle: CLEAN — every content hunk serves a declared claim; 4 LOW undeclared items are in-theme (reviewer-row doc fix, compound-gh pin, slice-2 docs paragraph, blank-line churn). No debug leftovers, no leakage, archive branch archive/p126-pre-split-8705ccd85 verified with tree = 8705ccd852052….
  • Build + tests: cargo fmt --all -- --check clean; fork-CI-exact clippy (--workspace --all-features --locked + the 5 allows + -D warnings) clean; targeted tools::subagent:: + tools::registry:: + fleet:: + verifier:: 1028 / 0; full cargo test -p codewhale-tui --locked --lib (own target dir, RUST_MIN_STACK=16777216) 12424 passed / 2 failed / 13 ignored — both failures in the known rotating remote_control::tests flake family, untouched by this PR, both green in isolation (re-ran: 2/0 in 0.14 s).
  • Mutations 4/4, each reddening exactly its own pins, baseline 9/0 green after each revert:
    • M1 grant captured after the narrowing → 4 red (a_verifier_runs_bounded_test_selections…, verifier_child_keeps_the_default_and_selection_bounded_run_surface, verifier_child_still_refuses_raw_shell…, read_only_shell_children_of_other_roles…).
    • M2 recording site ignores the parent clamp (allows_shell() instead of matches!(Full), the round-3 escape shape) → exactly 1 red: clamped_verifier_child_of_read_only_parent_holds_no_run_surface. (Deleting the conjunct outright only reaches a -D warnings "field never read" error — the grant is load-bearing even to compile.)
    • M3 Verifier dropped from role_requires_read_only_shell → 4 red (probe, receipt label, raw-shell wall, clamped child).
    • M4 machine-catalog shell conjunct reverted → exactly 1 red (machine_catalog_omits_bash_without_shell_authority), scout twin correctly green.

Requested fixes (P2)

  1. The PR's own new copy inverts the gates/probe-lane conditionality — the exact dishonesty class this PR exists to fix. For a Verifier child of a read-only-shell parent (a supported, pinned shape — clamped_verifier_child_of_read_only_parent_holds_no_run_surface), the truth is inverted: the probe lane is kept and the built-in gates are refused. But in all four surfaces the gates clause is unconditional while the lane clause carries the conditional:

    • VERIFIER_AGENT_INTRO sentence 1 (mod.rs:16641): "Your job is to run the requested gates with your bounded validation tools…" — unconditional; sentence 2 carefully scopes the lane ("When the session grants it — for in-session delegations…"). The scoping sits on the wrong sentence.
    • docs/SUBAGENTS.md:281-284 + zh mirror zh_hans/SUBAGENTS.md:123: "its shell is the bounded built-in verification surface plus — on in-session delegations whose envelope grants it — the probe lane": conditions the lane and states the gates flat.
    • FleetRole::description() (fleet/role.rs:195-197) and the table rows (SUBAGENTS.md:74 / zh :34): same unscoped gates phrasing ("in-session delegations" scopes only the lane there).
      Fix: key the gates clause on shell authority ("on envelopes that grant shell authority") in intro sentence 1, the roster description, the docs bullet, and the table cell. One-sentence change ×4 surfaces + zh parity.
  2. sub_subagent_chain_of_a_carveout_verifier_holds_no_extra_authority doesn't chain and doesn't pin the ordering it documents — mutation-proven. It hand-sets the post-narrowing runtime (allow_shell=true + shell=ReadOnly) and asserts session_permission_ceiling, then builds an independent Builder registry via the fixture; nothing derives hop-2 from a carve-out verifier, and under M1 (the exact round-3 re-key ordering bug the comment names) it stays green while four other pins redden. Fix: construct the second hop from the first hop's actual output (run new_with_owner/worker_profile_for_spawn on the carve-out fixture and feed the result into the Builder registry), or rename it (e.g. a_carveout_verifier_publishes_a_read_only_spawn_ceiling) and drop the ordering claim from the comment.

  3. No pin that a non-verifier read-only-shell child loses the probe lane. The Builder/Worker/Custom loop asserts Run absence (posture/envelope/catalog/execute) but never bash absence from their catalogs, and its only bash probe uses rm -rf src, which the classifier refuses even on the lane — confounded. A one-token widening (adding another role to role_requires_read_only_shell) would silently hand write-capable children the classifier-bounded shell with nothing red. Fix: assert !names.contains("bash") in the loop (catalog) — one line.

Should-fix (P2, consistency)

  1. Partial-fix inconsistency in the very tables this PR edits: the reviewer row's "read-only (net + bounded verify)" was corrected to "read-only (net)" (SUBAGENTS.md:72 / zh :32), but the byte-identical stale claim survives on the explore row (SUBAGENTS.md:70) and the zh scout row (zh_hans/SUBAGENTS.md:30) — bounded verify is Verifier-only (executor.rs:256-263), so both leftovers are wrong, and the table is now internally inconsistent. Same family, pre-existing: the scout-brief sentence (SUBAGENTS.md:218-220 / zh :85) attributes "the bounded verification surface" to scout.
  2. EN/zh parity: the fleet-envelope contract statement exists only in docs/zh_hans/FLEET.md:124 — no English counterpart anywhere (EN FLEET.md has no test-role tool-contract paragraph). The project rule is EN + zh equivalence; please add the EN paragraph or point EN FLEET.md at it.

Notes (P3, non-blocking)

  • Receipt label per-path truthfulness: fleet_effective_permissions reports verifier shell read_only, but on the executor path the envelope grants shell=None (gates only, no bash at all) — documented as by-design in three places, and enforcement is honest, but the label answers the profile question, not the envelope question. Consider keying the label off the envelope for executor-dispatched workers or a doc note on FleetEffectivePermissions.
  • network_posture_warning_for_task (worker_runtime.rs:1506-1541) prints the requested shell, so a network-off verifier task logs "shell=full" while its receipt says read_only.
  • Leftover copy: fleet/roster.rs:482-486 seeded blurb "Runs builds and tests to verify claims…" (unqualified, parallel user-visible string); FleetRole::Verifier rustdoc (role.rs:80-84); stale comments — role_posture_permits docstring (mod.rs:14495-14507, Required arm no longer consulted), mod.rs:14726 ("Scout/Reviewer/Planner calls" — the predicate now also proves Verifier), mod.rs:14653 (omits Planner).
  • Resume path: a resumed carve-out verifier re-records the grant from the preserved pre-narrowing derived profile — restores exactly the original authority (no widening) but is subtle and unpinned; a resumed-verifier pin would close it.
  • Test polish: the raw-shell wall test's comment claims the alias fails "at any provability" but only drives rm — one posture_permits_tool("Bash", Some(pwd)) line pins the exact-name asymmetry; refusal-message disjunctions ("not available" || "not allowed") could false-green; carve-out verifier's bash catalog presence is unpinned (dispatch is).
  • Body nits: the Testing receipt cites 058a78ea6, which exists nowhere (local or remote) — the actual head is fe98f4454912 (the load-bearing tree assertion does verify at it); "documented and pinned here" for detached_foreground_command_reports_its_closed_stdin_on_interact — that pin and its shell.rs copy are slice-1 (e13a366fd) content, not in this PR's reviewable diff.
  • Diff hygiene: ~586 of the 943 subagent/tests.rs insertions are blank-line-only hunks across unrelated tests (~62% of the test diff) — consider keeping reformat churn out of slice PRs.
  • Merge-order heads-up (swept): the stack has real semantic overlap with #123 in shell.rs + subagent/mod.rs read-only refusal strings (two grammar families colliding in one literal's context); when #123 lands, resolution must union both admitted families, not pick a side. #124/#136: no risk from this slice.
  • CI: the link gate ("PR closes an issue") is red on this head — the body needs Closes #N or a No-Issue: <one-line why> line (slice-PR opt-out fits; #139 needs the same). Everything else green.

Verdict

The slice does exactly what it claims, at the root, with clean scope and real pins — I found no functional, security, or performance defect, and no smuggled behavior. What holds it back from clean-approve is that its own new contract copy still overpromises gates in the clamped-child shape (fix 1) and two of its pins don't yet pin what their names/comments claim (fixes 2-3). With those three — all small, none behavioral — this is merge-ready. (Self-authored PR, so posted as a comment review.)

@asto18089
asto18089 force-pushed the fix/live-test-findings-subagent-shell branch from f5aaaaf to e1e2458 Compare November 4, 2026 13:23
@asto18089
asto18089 force-pushed the fix/fleet-test-role-contract branch from fe98f44 to e4c2de0 Compare November 4, 2026 13:23
@asto18089

Copy link
Copy Markdown
Collaborator Author

Stack re-aligned: rebased onto #126's e1e2458 (which sits on #139's wave 6337b0e). Clean rebase, own files byte-preserved (blank-insensitive assert 0 drift); gates at e4c2de0: fleet:: 361/0, tools::subagent:: 609/0. Review round re-sweep still pending.

@asto18089
asto18089 force-pushed the fix/live-test-findings-subagent-shell branch 2 times, most recently from 456babf to 945c98a Compare November 4, 2026 14:04
@asto18089

Copy link
Copy Markdown
Collaborator Author

Fix wave pushed for the round-1 review: ae52233aac (re-parented on the re-pushed slice-3 head e4c2de0a — see the process note at the end), one commit, DCO-signed.

What was fixed (all actionable round-1 findings)

  1. Gates conditionality — the PR's own remaining overclaim is gone: the gates clause now carries the read-only-parent conditional ("clamped child keeps the probes and loses the gates") in intro sentence 1, the roster description, the type description, and the docs bullet + role tables (en+zh). Copy pins extended (verifier_intro_states_the_evidence_contract, verifier_roster_description_states_the_bounded_contract).
  2. The chain test now chains — sub_subagent_chain_of_a_carveout_verifier_holds_no_extra_authority derives hop 2 from the carve-out verifier's published narrowed profile via derive_child and asserts the Builder below holds no gate surface and no lane at posture/envelope/catalog/dispatch. Its job is now honest: it guards the propagation side of the round-3 fix and says so; the capture-ordering side stays with the carve-out pins.
  3. Lane-absence pins — Builder/Worker/Custom read-only-shell children pin bash's absence from their catalogs; a widened role set flips both this and the chain pin.
  4. Docs consistency — explore (en) / scout (zh) "bounded verify" staleness dropped; scout-brief sentence now attributes the probe lane (en+zh); the fleet-envelope contract statement gained its English counterpart.
  5. Copy honesty sweep — fleet roster blurb, FleetRole::Verifier rustdoc, network_posture_warning_for_task (now names the effective narrowed shell like receipts do), three stale comments (role_posture_permits docstring, narrowing comment, delegation-refusal comment).
  6. Test walls corrected — the legacy-Bash alias provability pin moved to the dispatch wall that owns it (posture's Auto classification admits provable shapes for any role — the old posture-level pin was at the wrong wall, and the "at any provability" comment was half-wrong; both fixed).

Deliberately not changed: the receipt label answering the profile (not envelope) question — documented by design in three places; the blank-line reformat churn already in the slice; FLEET.md's pre-existing zh-only permissions paragraph (the fleet-envelope fact now has EN parity via SUBAGENTS.md).

Verification receipts at ae52233aac

  • Targeted tools::subagent:: + tools::registry:: + fleet:: + verifier::: 1028 / 0; cargo fmt --all -- --check clean; fork-CI-exact clippy clean.
  • Full cargo test -p codewhale-tui --locked --lib (RUST_MIN_STACK=16777216): 12425 / 1 / 13 ignored — the 1 in the documented rotating remote_control flake family (untouched files, green in isolation).
  • Mutations, each reddening exactly its own pins, baseline green after each revert:
    • grant captured after the narrowing → 4 red (carve-out + dispatch pins); the chain test stays green by documented design — it guards propagation, not the capture ordering;
    • registry publishing a pre-narrowing profile to spawns (effective_profile.shell assignment dropped) → exactly 1 red: the rewritten chain test — the propagation bug its comment names is now demonstrably caught;
    • role-set widened (Builder added to role_requires_read_only_shell) → 2 red: both catalog lane-absence pins — the silent-widening gap is closed.
    • (A fourth candidate — deleting the parent_shell_authority conjunct outright — only reaches a -D warnings "field never read" compile error: the grant is load-bearing even to compile.)

Process note: the stack moved mid-fix

The stack was re-pushed while this fix was in flight (reworked slice-1/2 content, same author dates, new SHAs: 6337b0e0a → e1e2458dd → e4c2de0a, plus a newer slice-2 456babf28 on the base branch). Re-parenting safety: all 7 files this fix touches were verified blob-identical between the old slice-3 content (fe98f4454912) and the re-pushed head (e4c2de0a), so the fix is exactly the same 7-file change applied to the new stack content; the git endpoint was down, so the commit went up via the Git data API with the tree built on e4c2de0a's tree (fast-forward ref update, no force). The body now carries the corrected SHAs, the scope-corrected tree claim, and a No-Issue: line for the link gate.

@asto18089
asto18089 force-pushed the fix/live-test-findings-subagent-shell branch from 945c98a to 3406aeb Compare November 4, 2026 14:43
@asto18089
asto18089 force-pushed the fix/fleet-test-role-contract branch from ae52233 to 184984a Compare November 4, 2026 16:40
@asto18089
asto18089 force-pushed the fix/live-test-findings-subagent-shell branch from 3406aeb to f063d51 Compare November 4, 2026 17:03
@asto18089
asto18089 force-pushed the fix/fleet-test-role-contract branch from 184984a to a257025 Compare November 4, 2026 17:03
@asto18089

Copy link
Copy Markdown
Collaborator Author

Re-aligned onto the wave-2-based #126 (f063d51): your two commits (slice-3 + round-1 review fixes) are preserved byte-for-byte (blank-insensitive assert 0 drift on both), now at d705d35 + a257025. Gates: fleet:: 361/0, tools::subagent:: 609/0, fmt clean. Heads moved again due to the push race with the other session — pull/rebase before pushing local state.

@asto18089
asto18089 force-pushed the fix/live-test-findings-subagent-shell branch from f063d51 to d6d4143 Compare November 4, 2026 18:23
@asto18089
asto18089 force-pushed the fix/fleet-test-role-contract branch from a257025 to 15b5639 Compare November 4, 2026 18:23
@asto18089

Copy link
Copy Markdown
Collaborator Author

Re-based onto the re-aligned #126 (d6d4143, on the rebased #139/9148c1785 over main 449edbf): both your commits replayed clean, now 1f7e2a7 + 15b5639. Gates at 15b5639: tools::shell:: 270/0/1, tools::subagent:: 637/0, fleet:: 361/0, fmt clean, clippy green. If you have local state, rebase on this head before pushing.

@asto18089

Copy link
Copy Markdown
Collaborator Author

Fresh round-2 review (2026-11-05) — approve-grade, no new P0/P1/P2 defects

Fresh review from scratch at the rebased head, 6 lanes (claims & root-cause, engineering elegance, smuggled-change audit, authority-model bug hunt, perf + test quality, docs/en↔zh parity), on top of the parallel stack re-push that landed mid-review. Every round-1 fix survived the re-push verified (7 of 12 touched files byte-identical; the 4 differing files differ only by slice-2's worktree-lane rework folding through shared files — this slice's authority semantics unchanged and re-verified intact).

Verdict: approve-grade. The claims are true at the head, the root cause is addressed at the mechanism (role gating in the registry authority model, not copy alone), the smuggle audit came back CLEAN (every hunk maps to the slice-3 scope; all −91 deletions accounted with replacement coverage; zero dependency/config churn), the authority matrix (9 roles × parent shells × four walls: posture / envelope / catalog / dispatch, plus fleet-vs-in-session split, alias and case escapes, sentinel interplay, mid-session posture flips, resume paths) came back clean with no catalog-advertises-what-dispatch-refuses hole — the PR actually closes one such pre-existing mismatch for shell-less envelopes — and no performance regression (the re-key is a static matches!; the catalog conjunct runs once per memoized build; parent_shell_authority is a plain bool).

Findings fixed in this push (round-2 commit 75f9a592c)

  1. The chain pin did not guard the line it claimed (the one real gap): the round-1 rewrite derived hop 2 from runtime_profile — the field set at construction — so the publication line that actually hands the narrowed profile to the verifier's spawns (child_runtime.worker_profile) had no pin; reverting that line alone left every test green while a chain Builder regained the gate surface. The pin now starts the Builder through the verifier's real agent tool and reads the child's recorded runtime_profile (production's worker_profile_for_spawn → derive_child output). Mutation-proven: publication-line revert → exactly 1 red (this test), lane-absence twin stays green — on the previous test shape that mutation stayed green, so the body's old "exactly 1 red" claim is now actually true.
  2. The agent tool description still said "test (runs tests and reports evidence)" — the third model-facing surface (per the repo's own "three places a model reads" contract) that the copy sweep missed, same unconditioned family the PR calls an overclaim elsewhere. Now "built-in verification gates; custom commands refused", pin extended, 1024-token budget guard still green.
  3. Diff hygiene: dropped the 570 pure blank-line insertions that were 56% of subagent/tests.rs's diff (not formatter-required; base style has none).
  4. Comment honesty: the fleet receipt comment now names the verifier's bash-less fleet surface (it said the verifier's bash is the probe lane — the fleet envelope grants no bash at all); the executor envelope comment scopes the probe lane to in-session delegations; worker_profile's scout arm no longer claims "the bounded verification surface" for scout/reviewer.
  5. Docs: zh verifier bullet + zh FLEET paragraph use 信封 consistently (the same PR used both spellings); zh FLEET lane claim gains the envelope-grant qualifier; ChildAuthority::clamp path corrected to fleet/role.rs (en+zh).
  6. Claim corrections: the network-warning label change is disclosed as currently unreachable-delta (all built-in roles default network-on, so the Some branch can't fire — the change aligns copy for future network-off roles and no discriminating test can honestly exist); the machine-catalog conjunct and the docs grammar sentences are now disclosed as body bullets.

Accepted (no action)

role_posture_permits's dead-by-callers Required arm (documented in-place), receipt label granularity vs the fleet envelope (documented divergence, display-only, nothing branches on it), loose two-way refusal disjunctions in tests (repo idiom; the load-bearing registry legs pin exact copy), the trivially-true Bash half of the catalog pin, pre-existing items outside the diff (in-session Run commands schema vs dispatch refusal; hand-written classifier-proven bash beyond the hidden catalog — capability still ≤ the parent's own lane).

Verification at the pushed head

  • Targeted suites (tools::subagent:: + tools::registry:: + fleet::): 1047 passed / 0 failed at 75f9a592c; cargo fmt --all -- --check and fork-CI-exact clippy clean.
  • Full lib at the round-2 content on the previous stack tip: 12429 / 0; a full-lib rerun at this head was still in flight under heavy build contention from the parallel stack work and will be reported in a follow-up if anything real surfaces (targeted suites cover every file this slice touches).
  • Mutations, each reddening exactly its own pins with baseline green after revert: publication-line revert → exactly 1 red (end-to-end chain test); stale-copy revert → exactly 1 red (description pin); role-set widening (Builder into the lane set) → 11 red incl. both lane-absence pins and the chain pin.

Process note: the stack moved twice during this round (slice-1 gained the PTY-EOF and test-helper commits; slice-2 folded in the worktree-lane rework; slice-3 was re-created preserving the round-1 fixes). The round-2 commit is re-parented on the current tip as 75f9a592c (rebase clean, no conflicts; slice-2 rework verified intact, slice-3 semantics verified intact both directions).

@asto18089
asto18089 force-pushed the fix/fleet-test-role-contract branch from 75f9a59 to 00e6506 Compare November 5, 2026 02:14
@asto18089
asto18089 force-pushed the fix/live-test-findings-subagent-shell branch from 8f98b3e to 1f9635a Compare November 5, 2026 02:24
@asto18089

Copy link
Copy Markdown
Collaborator Author

Follow-up as promised in the round-2 review: the full-lib verification at the pushed head 75f9a592c is now complete and clean.

  • cargo test -p codewhale-tui --locked --lib -- --test-threads 2 (the fork-CI thread count): 12487 passed / 0 failed / 13 ignored, 320 s, zero slow-test warnings — the earlier "2 failed under contention" observation was environmental (a parallel session's cross-repo build saturating the shared board; those runs also showed 60s+ watchdog warnings on untouched core::engine tests).
  • Attribution detail for the default-thread hang I flagged: it reproduces at slice-3's round-1 head 15b563920 without this PR's round-2 commit, so it belongs to the stack rework, not this slice — full recipe and attribution posted on fix(shell): compound read-only probes behind the operand gates #126 (issuecomment-6648616675). Under --test-threads 2 everything passes, which is why the gates stay green.

Round-2 verification is now: targeted suites 1047/0, full lib 12487/0 (CI-exact threads), fmt + fork-CI-exact clippy clean, CI gates green on 75f9a592c, and all three mutation probes red-exactly-own-pins with green baselines.

Third slice of the #126 split: the `test` role's honest surface.

- The roster description, the type schema, VERIFIER_AGENT_INTRO, and
  both SUBAGENTS role tables state the real contract: built-in
  verification gates with test-selection arguments, plus the
  classifier-bounded read-only shell for evidence gathering; custom
  commands stay refused.
- The Verifier joins `role_requires_read_only_shell` and
  `allows_bounded_readonly_bash` (the latter now reads that one
  predicate), gaining the same bounded probe lane as the inspection
  roles; the carve-out is Verifier-only and keys on a recorded
  parent-shell grant, so a read-only-parent's child never regains
  authority through the role name alone (chain pins cover the
  sub-subagent path).
- Shell authority (`holds_shell_authority`) keys on the held policy
  plus the grant sentinel; fleet receipts report the verifier's
  effective shell as `read_only`; the worker catalog omits bash from
  envelopes without read-only shell authority and pins the refusal
  copy's load-bearing claims.

Tests: verifier contract/authority/carve-out pins incl. the
sub-subagent chain, catalog and receipts labels; the deleted
no-shell verifier catalog test is replaced by the probe-lane
variant. Targeted: `tools::subagent::` + `tools::registry::` +
`fleet::` suites green; tree of this commit equals the reviewed
#126 (stacks on fix/live-test-findings-subagent-shell
after fix/shell-stdin-eof); full content preserved on
archive/p126-pre-split-8705ccd85.
Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
…he chain honestly

Review round 1 on this slice found three should-fix items and a set of
consistency nits; this wave fixes everything actionable:

- The gates claim is now conditional everywhere it appears (intro
  sentence 1, the roster description, the type description, the docs
  bullet and role tables in en+zh): a Verifier child of a read-only
  shell parent keeps the probe lane and loses the gates, which is the
  exact shape clamped_verifier_child_of_read_only_parent_holds_no_run_
  surface pins. The unconditional phrasing was the PR's own overclaim.
- sub_subagent_chain_of_a_carveout_verifier_holds_no_extra_authority
  now really chains: hop 2 is derived from the carve-out verifier's
  published narrowed profile through derive_child, and the Builder
  below it is asserted to hold no gate surface and no probe lane at
  posture, envelope, catalog, and dispatch. It guards the propagation
  side of the round-3 fix (a registry that leaked a pre-narrowing
  profile to its spawns would redden it), no longer claiming ordering
  coverage it did not have.
- Read-only-shell children of other roles pin the probe lane's
  absence from their catalogs: a widened role set must not silently
  hand the classifier-bounded shell to write-capable children.
- Docs consistency: the explore row (en) and scout row (zh) drop the
  stale 'bounded verify' claim the reviewer row fix left behind; the
  scout brief attributes the probe lane, not the verification surface;
  the fleet-envelope contract statement gains its English counterpart.
- Copy honesty sweep: the fleet roster blurb and the FleetRole::Verifier
  rustdoc state the real contract; the network posture warning names
  the effective (narrowed) shell like the receipts do; three stale
  comments updated (role_posture_permits docstring, the narrowing
  comment, the delegation-refusal comment).
- Test walls corrected: the legacy-Bash alias provability pin moved to
  the dispatch wall that owns it (posture's Auto classification admits
  provable shapes for any role), with the half-wrong comment fixed.

Targeted tools::subagent:: + tools::registry:: + fleet:: + verifier::
suites 1028/0; cargo fmt and fork-CI-exact clippy clean.

Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
…end and finish the copy sweep

- the chain pin now drives hop 2 through the real spawn path: the
  carve-out verifier's agent tool starts a Builder and the recorded
  runtime_profile is the profile run_subagent derived from the runtime
  the registry published, so a publication-line regression
  (child_runtime.worker_profile reverted to the parent-runtime profile)
  reddens it;
- the agent tool description's test blurb states the real contract
  (built-in verification gates, custom commands refused) instead of the
  unconditioned "runs tests" family, with the canonical-vocabulary pin
  extended;
- the diff drops the file-wide blank-line churn in subagent/tests.rs
  (570 pure blank-line insertions that no formatter requires);
- comment honesty: the fleet receipt test names the verifier's real
  bash-less fleet surface, the executor envelope comment scopes the
  probe lane to in-session delegations, and worker_profile's scout arm
  no longer claims the bounded verification surface;
- docs: the zh verifier bullet and zh FLEET envelope paragraph use the
  established 信封 term and carry the envelope-grant qualifier, and the
  ChildAuthority::clamp path reference points at fleet/role.rs (en+zh).

Round-2 fresh 6-lane review found no functional, security, or
performance defect and no smuggled scope; these are its actionable
findings.

Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
…te the guide rows, finish the blank-line strip

Signed-off-by: asto18089 <asto18089@users.noreply.github.com>
@asto18089
asto18089 force-pushed the fix/fleet-test-role-contract branch from 7302247 to 6e2ed34 Compare November 5, 2026 05:10
@asto18089

Copy link
Copy Markdown
Collaborator Author

Stack note (round-21 wave): this branch re-based onto 1f9635a894 mid-round-21 — thank you, that closed the round-20 open item. PR #126's round-21 review (id 5780792840) then landed one fix wave on the slice-2 branch: 6386b876b (restore the six #123/#124 test fns + the mod identity_selection; declaration the round-20 rebase resolution had silently dropped, re-pin the Do not retry variations fragment, one doc-comment correction; forkguard 285/0, tools::subagent:: 648/0). This branch is now behind by exactly that one commit — please base-follow 6386b876b before merge (mechanical; your round-3 fixes replay cleanly — the only same-file activity is the replacement test's claim array, which gained one entry inside read_only_inspection_roles_cannot_write_through_text_filters, and your insertion after it does not overlap). Full context: PR #126 round-21 review + body round-21 update.

@asto18089

Copy link
Copy Markdown
Collaborator Author

Fresh round-3 review (2026-11-05) — approve-grade, no new P0/P1/P2 defects

Fresh from-scratch review per the round-3 request: the stack was re-parented onto the latest base before the review (#126's 8f98b3e89, itself on the rebased #139 over current main), and — after the base moved a second time mid-review — re-parented again onto #126's 1f9635a894 (slice-2's round-20 wave plus the guardrail plumbing the stack picked up elsewhere; the rebase kept this slice's content and resolved its one subagent/tests.rs overlap by keeping the stack's new forkguard test). Review head: 6e2ed34fb843b91afe2494bedbfa0f4366b3a592.

Verdict: approve-grade — 0 P0/P1/P2 functional, security, or performance defects; smuggle audit clean. Nine lanes re-derived everything from the diff: claims/root-cause completeness, spawn-path authority, wall/fleet authority matrix, smuggle audit, elegance/wheel-reuse, feature behavior, performance, test quality + mutation gaps, docs/copy en↔zh parity.

What the lanes verified sound

  • Claims (12/12 implemented and complete). Every body claim traces to a real diff hunk and is complete: roster description, type schema description, agent-tool description blurb, VERIFIER_AGENT_INTRO, SUBAGENTS role tables (en+zh); Verifier joins role_requires_read_only_shell with allows_bounded_readonly_bash re-keyed to that one predicate; shell authority keys on the held policy plus the named Verifier exception; the machine-catalog conjunct agrees with enforce_tool_authority; fleet receipts label the verifier read_only. The pre-state is verifiably broken exactly as the body's "live evidence" says (base advertised "runs the test suite and validation gates" while the clamp denied every Bash spelling and the lane excluded the Verifier), and the fix operates at the root cause — one role-set predicate plus one recorded parent-shell grant — not on symptoms.
  • Authority model (spawn path + walls). The grant is captured at exactly one site (subagent/mod.rs:15729 at this head), after the parent clamp and before the role narrowing; the publication line (child_runtime.worker_profile = effective_profile, :15743) is the only path delegation can carry authority through; resume re-clamps through the same construction, so no path widens beyond a fresh spawn. Eleven escape attempts failed with identified blocking lines (carve-out theft via chains, stale-profile leaks, re-key regression, alias/case re-entry, exec_shell/task_shell/terminal spellings, catalog/dispatch divergence, workflow bypass, fleet-member lane theft, sentinel manipulation, mid-session flips). The four-wall matrix (posture/envelope/catalog/dispatch × role × parent-shell, in-session and fleet) is coherent; catalog never advertises a surface dispatch refuses on any read-only envelope, and reject_unbounded_verification is untouched by the diff.
  • Smuggle: clean. All 60 hunks of the 13-file diff map to a claimed purpose or its pin; no production code touches shell.rs, stdin, PTY, or classifier grammar; commits carry valid Signed-off-by trailers; the round-2 blank-line strip is real.
  • Elegance: no wheel reinvented. The re-key consolidates (two remaining trio literals are the genuinely different fleet-envelope set, disclosed at both sites); the grant bool has no pre-existing facility to reuse; the registry conjunct consumes the envelope's own authority value so catalog/dispatch agree by construction; copy repetition across surfaces is inherent and cross-pinned.
  • Performance: no regressions. Every behavioral delta is an O(1) predicate swap or a once-per-spawn bool; no new allocations/clones/string builds in non-test code; the per-step catalog rebuild path is pre-existing and untouched in cost.

Fixed in the round-3 commit (found by this review's lanes)

  • The fleet roster slot blurb is now pinned (FleetRoster::built_in_members' verifier slot — a model-facing surface via AgentProfile.description had no test; reverting it left every test green). Mutation-proven: revert → exactly 1 red.
  • The type description's contract tail is now pinned (only the prefix was anchored; dropping "custom commands are refused" / the parent-clamp clause stayed green). Mutation-proven: drop → exactly 1 red.
  • Missed user-facing surface fixed: docs/GUIDE.md (en) and docs/zh_hans/GUIDE.md still described the verifier as "running checks" — the same unconditioned family this PR calls an overclaim everywhere else. Both rows now say the built-in verification gates / 内置验证门禁.
  • Diff hygiene completed: 11 residual standalone blank-line separators in subagent/tests.rs (missed by round-2's strip predicate, which didn't cover mod/const/impl/// item heads) are gone; the 6 blanks that belong to the new tests' own code remain.
  • Claim correction: the capture-ordering mutation receipt said "4 red"; re-run it is 7 red (run-surface pin, ceiling pin, catalog pin, and legacy-refusal pin redden in addition to the carve-out/dispatch pins) — over-determined, the safe direction. The body's Testing section is corrected.
  • Disclosures added to the body: the catalog conjunct also hides bash/Bash from the ACP plan-mode catalog (same shell-less shape, previously advertised-but-always-refused — strict alignment); the agent tool description now sits at exactly the 1024-token conservative-estimate cap; en docs/FLEET.md has no fleet-envelope paragraph while the zh one exists (pre-existing asymmetry; this PR edits only the existing zh paragraph).

Accepted residuals (all P3, disclosed or pre-existing)

role_posture_permits' Required arm is production-dead (honest docstring kept; deleting it would churn four pin assertions); the shell != None guard inside the carve-out is unreachable-but-defensive; fleet receipts' read_only label is narrower than "no bash at all" on the fleet path (label layer is lossy by design; disclosed); the fleet writer (ScopedWrite) machine catalog still lists bash while dispatch refuses it (pre-existing on the writer path this PR deliberately doesn't touch, fails closed); network_posture_warning_for_task's Some branch stays unpinnable without a production change (disclosed); zh-only repo-wide canonical/legacy role-name inversion and stale zh sync markers predate this PR.

Verification at the final head 6e2ed34fb

  • Targeted suites (tools::subagent:: + tools::registry:: + fleet::): 1049 passed / 0 failed (the rework adds two suites); cargo fmt --all -- --check clean.
  • Full cargo test -p codewhale-tui --locked --lib (RUST_MIN_STACK=16777216): 12545 passed / 0 failed / 13 ignored. Environment note for whoever runs suites on this board class: with libtest's default parallelism the run stalls in the parallel core::engine::forkguard_* family (tests verified green both in isolation at the base and in the capped run); with -- --test-threads=4 the entire lib completes in ~3 minutes, exit 0.
  • CI-exact clippy: 3 collapsible_if in turn_loop.rs/subagent/mod.rs(~:8058)/verifier.rs — all three shapes exist on the base branch and all three sites are outside this PR's diff (inherited from the stack; the release gate owns them, not this PR).
  • Mutations re-proven at the final tip: capture-ordering → 7 red; spawn-publication revert → exactly 1 red (the chain pin); roster-blurb revert → exactly 1 red; type-description clause drop → exactly 1 red (proven at the round-3 head; strings and pins byte-identical after the rebase).
  • PR is MERGEABLE/CLEAN at 6e2ed34fb; CI gates (Signed-off-by, link, credentials) green.

@JensenChen28 JensenChen28 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已完成 head 6e2ed34fb843b91afe2494bedbfa0f4366b3a592 的完整 15 文件切片审查,规范与需求两个轴均未发现本切片新引入的实质问题。Verifier 的 Run 例外绑定父级实际 Full grant,捕获顺序先于角色收窄;发布给嵌套子代理的是收窄后的 profile。canonical bash 仍经 exact-name/classifier,legacy/raw shell、自定义命令及无 grant 子代理的 Run 保持拒绝,Fleet 与 in-session 表面区别已披露。

本轮暂不批准整个堆栈,原因是实际依赖阻塞,而非 CI 尚未完成:当前 head/基线包含 #139 已报告且尚未解决的私密 P2;请先在依赖中处理,再复核组合。该问题不是本切片新增,因此未重复创建相同 finding,详细证据按 SECURITY 保留私密。

基线核对:PR API 仍报告 1f9635a894d8c405d0467503ba3262bc92f42d25,但 fetch/ls-remote 的实际目标分支已到 6386b876b7bc3df3fb73a4d07596c150580cd974。已检查这一步仅 shell 注释和恢复测试,并以只读 merge-tree 确认与本 head 可无冲突组合(tree 5877def2fe2f7a437c6952ed4465ee4d74e6f38f);未将旧 API SHA 当成最新目标,也未声称已验收整个 #126/#139 栈。

实际验证:最新 fetch、完整 diff/权限调用链及新增测试源码、git diff --check、最新目标差异和只读组合。未运行 Cargo/Rust、原生平台或 provider 测试;作者测试/变异数字未采用为本轮执行结果。未修改 PR、合并、入队或改变 auto-merge;发送前已复核 head/base、实际目标 tip 和最新讨论。

@zhuowp zhuowp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes on fixed head 6e2ed34fb843b91afe2494bedbfa0f4366b3a592: one confirmed P2 concerning preservation of an explicit delegated-tool restriction.

The new delegation path can fail to preserve an operator/caller restriction in a supported configuration. Keep explicit restrictions authoritative at both catalog admission and dispatch, separately from automatic role restrictions, and add focused negative coverage for that distinction. Technical conditions and source evidence are retained locally for private disclosure under SECURITY.md; no public reproduction is included. The confirmed scope is limited: this review does not claim unrestricted execution, workspace writes or a sandbox escape.

The cumulative 15-file/62-hunk source scope, all 17 affected complete registered test bodies, and the actual target's separate six new plus one changed test bodies were reviewed. Actual target is 6386b876b7bc3df3fb73a4d07596c150580cd974, while merge-base/cached API base is 1f9635a894d8c405d0467503ba3262bc92f42d25. Their clean combined tree 5877def2fe2f7a437c6952ed4465ee4d74e6f38f differs from the feature head tree. This is not an approval of the wider stack.

Current checks are five success and one skip. They cover issue-link policy, sign-off and credentials; no exact-head native build/test output was obtained for this target branch. No prior PR's PASS or author-reported runs/mutations are borrowed. Missing native CI is a validation limit, not the reason for this code finding.

No new local runner, Cargo/compile, probe, smoke, model/provider or browser was used. Static producer-to-consumer evidence supports this scoped finding. Merge and release remain separate repository and authorization decisions.

@JensenChen28 JensenChen28 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

按固定 head 6e2ed34fb843b91afe2494bedbfa0f4366b3a592 复核后,更新为 CHANGES_REQUESTED。

规范与需求双轴独立确认:该 PR 自身仍有一个 P2,某个受支持的委派配置未能保留显式工具限制。此前 COMMENT 仅记录了 #139 的继承阻塞,对该 PR 自身差异的结论不完整;本次补充这一独立问题。请在目录准入与实际分发两处都保持显式限制的权威性,并补充相应负向覆盖。

依据 SECURITY.md,具体文件位置、条件、调用链及修复建议已保存到本地私密审核证据,公开线程不披露操作性细节。该结论不声称任意命令执行、工作区写入、沙箱逃逸或已发生泄漏。#139 原有私密阻塞仍单独保留。

实际验证:已重新 fetch 并核对当前 head、实际目标 6386b876b7bc3df3fb73a4d07596c150580cd974、API base/merge-base 1f9635a894d8c405d0467503ba3262bc92f42d25;复核完整累计差异及新反馈涉及的生产链,diff 检查通过。未运行本机 Cargo/Rust、原生平台或攻击验证,未采用其他审核者的运行计数。当前结论来自具体源码问题,非因缺少原生 CI 而请求修改。发送前已重新核对 head/base 与讨论。

@asto18089

Copy link
Copy Markdown
Collaborator Author

已收悉(reviews 5802131592 / 5802645559,同项 P2)。按本仓 SECURITY.md 约定:该事项走维护者认可的私密渠道处理,公开线程不携带任何技术条件、位置或边界细节。#139 的同渠道事项已在公开侧出具收悉(6687032924);本 PR 与栈组合的复核依赖在私密渠道闭环后随之解除。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants