feat(specialist): zeromaxing posture and orchestrate plan execution - #829
Draft
gnanam1990 wants to merge 111 commits into
Draft
feat(specialist): zeromaxing posture and orchestrate plan execution#829gnanam1990 wants to merge 111 commits into
gnanam1990 wants to merge 111 commits into
Conversation
zeromaxing is thorough with double the turn budget again: 320 tool-turns, effort high, self-correction armed. It fills only what the caller left unset, exactly as the other profiles do, so an explicit flag and a --mode preset both still win — precedence stays enforced by ordering, not by a new check. ONE NAME. /effort zeromaxing, /profile zeromaxing, --exec-profile zeromaxing and --reasoning-effort zeromaxing all name the same thing, and nothing else does. No aliases: "max", "deep" and "deepmode" resolve to nothing, asserted by test, because a second spelling is a second thing to keep in step. "max" is RESERVED and untouched. ValidReasoningEffort still accepts ReasoningEffortMax, so /effort max and --reasoning-effort max behave exactly as they did — parsed as a raw provider level, rejected by the flag parser, and unsupported by every current model. That spelling stays free for a real provider rung; claiming it for a Zero posture would burn it. HONEST DELTA, stated to the user rather than left in a commit message: the turn budget is the ONLY mechanical change over thorough. Effort is already at the ceiling and self-correction is already on. execprofile.Delta carries that sentence to /effort, /profile and the exec selection notice. ReasoningEffort is "high", pinned by test rather than by comment. TestZeromaxingReasoningEffortStaysHigh explains why: every level above "high" falls through the providers' effort mappers into their default arm — anthropic and gemini both compute a thinking budget of 0 (extended thinking DISABLED) and the openai mapper drops the field entirely. Raising it would silently turn reasoning off while the UI claimed a higher posture. Raising the real ceiling means teaching the providers those tiers first. The raised budget propagates to spawned sub-agents. Deliberate — this is a maximal posture — so it is asserted by test and stated in Delta rather than left to be discovered. SelectionRefusal is the single authority both selection paths consult. The headless exec path and the TUI paths already apply a profile's knobs through different code with different state; letting each decide SELECTION independently is how a rule gets applied to one call path and omitted from its sibling. Config gating follows mergeProjectConfig's tighten-only template: a project .zero/config.json may set profiles.disableZeromaxing (DISABLE) but can never clear it, so a cloned repo cannot switch a cost multiplier on for whoever opens it — an attempted enable is dropped silently, exactly like an ignored network "allow".
Four model-facing notices tell the model when its own posture flips: enter (once, on the first turn after activation), a budget guideline beside it, still-on (every continuing turn), and exit (once, after deactivation). WHERE THEY GO IS THE WHOLE DESIGN. Every notice is appended to the CONVERSATION tail as a user-role message, on the same channel as the existing diagnostics nudge and the failure/plan hints. None of it reaches buildSystemPromptParts. The system prompt and tool definitions are the provider's cached prefix — the anthropic mapper puts its cache_control breakpoint on the last system block and the last tool — and Gitlawb#760 made that prefix build once per run precisely so it stays byte-identical across turns. Per-turn text above that breakpoint invalidates the cache every turn, roughly doubling input cost, and nothing in the system reports it. So ZeromaxingStillOnNotice is a fixed literal: no turn counter, no remaining budget, no timestamp. A test asserts it carries no digits at all, because the hazard is not today's placement but a future edit that adds a varying part and then moves the text into prompt assembly. The reminders also promise NO orchestration — no workers, no fan-out, no workflow tool. Phase 1 ships none of that, and a prompt advertising capabilities the run does not have is a prompt-level lie the model will try to act on. A test enforces the vocabulary. The state machine is one pure function of (posture, turn) with no memory, so "enter exactly once" and "still-on never on the first turn" are assertions about a table rather than about observed side effects. Proof, both shapes of the hazard, each caught by a DIFFERENT assertion: - per-turn text written into messages[0] -> the prefix-extension assertion - static text baked into the system prompt -> the explicit leak assertion TestRunPreservesRequestPrefixAcrossTurnsUnderZeromaxing is the zeromaxing variant of the existing prefix test; the openai one is its wire-level sibling, proving the mapper serializes an append-only conversation into an append-only request body. A third openai test pins that the posture name can never appear in a serialized request at all. Options.Zeromaxing is separate from Options.Profile because the posture arms no escalation triggers, so Profile.Policy() returns nil for it and could not carry this. Its zero value is Off, leaving every existing caller byte-identical.
…paths
/effort zeromaxing is the primary way in; /profile zeromaxing is its alias. It
is handled beside "auto" with an EARLY RETURN, before the ReasoningEffort
conversion and before ValidReasoningEffort — it is a posture name, not a
provider level, and must never become one.
It DELEGATES to handleProfileCommand rather than re-applying the knobs, so
"both entry points resolve identically" is true by construction instead of by
two implementations a test hopes agree. The CLI mirrors it:
normalizeZeromaxingEffort folds --reasoning-effort zeromaxing into the
equivalent --exec-profile selection before mode/profile expansion, leaving the
documented precedence ordering untouched. A conflicting --exec-profile is a
usage error rather than a silent winner.
THE LOAD-BEARING GUARD: forwardedReasoningEffort refuses to forward the posture
name for any model, including catalog-unknown ones where unrecognized values
otherwise pass straight through. Normalization already prevents it reaching
there; the guard is what makes a regression upstream fail loudly instead of
sending a provider a parameter value no provider defines.
"max" stays RESERVED and unchanged through all of this: the flag parser still
rejects it, the normalizer passes it through untouched, and /effort max still
reports it unsupported exactly as before.
Driving the real binary is what found the one real gap here: the flag parser
rejected "zeromaxing" outright, so the entire --reasoning-effort entry point was
dead at the user surface while every unit test passed, because they called the
normalizer directly. TestReasoningEffortFlagAcceptsZeromaxingButNotMax is that
regression, and it fails in both directions.
Degrade honestly — this closes a real asymmetry. exec already told the user via
reasoningEffortNotice when a model could not take the requested effort; the TUI
skipped the fill in SILENCE. Same rule, one of two call paths. Both status
surfaces now render ONE shared resolved-state line
("effort: high · profile: zeromaxing · turns: 320") plus the real delta, so a
user sees what actually reached the provider rather than just a posture name.
reconcileProfileAfterModelSwitch is the sibling re-derive site and records the
same reason, so a switch that drops the effort cannot leave the status implying
a raise the run is not making.
revertExecProfile gains a FOURTH knob. It is knob-by-knob, which is exactly
where a new one gets forgotten, so the posture's restore is asserted alongside
budget, effort and self-correct — and gated on zeromaxing specifically, so
/effort auto under fast/thorough keeps its existing meaning of "clear the
effort" instead of dropping the whole profile.
The footer carries a ZEROMAXING chip beside the effort chip while the posture is
on, and drops it while Exiting — by then the posture is already off and only its
announcement is pending.
…wn state
Two corrections to what zeromaxing tells the user, both cases of documentation
describing behaviour that is not happening.
1. --spec-reasoning-effort zeromaxing stays rejected — it is a run posture and
has no meaning for a spec draft — but the generic "Expected low, medium, or
high" read like a bug to anyone who had just learned the name works on
--reasoning-effort. It now says why it does not apply and names both flags
that do what the user was reaching for. A genuinely unknown value keeps the
plain message: the explanation is specific to the posture name, not a new
blanket wording.
2. Delta's self-correct clause described the change from THOROUGH, not from the
caller. Telling a user sitting on the LSP-only default that self-correction
is "already armed" while silently moving them to the full project test plan
is exactly the failure this posture's honesty rules exist to prevent. Delta
is now a function of the caller's actual state and renders one of three
transitions, in /selfcorrect's own vocabulary:
self-correct: lsp -> tests (the common case)
self-correct: unchanged (tests) (genuinely unchanged)
self-correct: lsp (your /selfcorrect choice ...) (user override wins)
The third exists because the TUI lets a user turn it back off after selecting
the posture; reporting a raise there would be a third way to describe
behaviour that is not happening. The budget and effort clauses stay fixed —
effort genuinely is caller-independent, since "high" is the ceiling every
provider accepts.
The headless path captures self-correct BEFORE applyExecProfile, which arms it
as a side effect; reading it afterwards would make every run report "unchanged".
The TUI reads its LIVE state rather than selection-time state, so a later
/selfcorrect off stops the status claiming a raise the session no longer has.
A unit test on the transition helper cannot catch the capture being read at the
wrong moment, because it never runs the code that captures — so the ordering is
pinned by a test that drives the real exec path and reads stderr. That gap was
found by mutation, not by review.
…t vouch for Three defects found by running the real binary, all in what the posture reports or applies. 1. THE FILL SILENTLY DID NOT HAPPEN on a custom endpoint. /effort zeromaxing left the effort at "auto" while --exec-profile zeromaxing on the SAME model sent reasoning_effort:"high" on the wire. Same posture, same model, two answers — the fifteenth instance of one rule applied to one of two call paths. The fill site used reasoningEffortAllowed, which returns false both for a model KNOWN to lack the level and for a model the catalog has never heard of. The headless path already distinguishes them (an unknown model forwards the requested effort "since no support claim can be made for it"), and so does the TUI's own model-SWITCH site, which takes a ringKnown flag for exactly this reason. Only the profile-fill site conflated them. profileEffortApplies now fills when the model lists the level OR when no catalog entry exists, and declines only when an authoritative empty ring says the model has no reasoning controls. A model name that is empty is a third case and still declines — there is nothing to make a support claim about. That last distinction was caught by the pre-existing fast-posture test, which the first version of this fix broke. THE HOLE THAT HID IT: every test asserted a surface against its OWN expectation — the TUI tests against the TUI's rule, the CLI tests against the CLI's. Both suites were green while the two paths disagreed, because nothing compared them against each other for the same model. A unit test on either helper could not have caught it. TestProfileEffortFillAgreesWithTheHeadlessPath is that missing comparison, run across a catalog reasoning model, an inferred one, a catalog model with an authoritative empty ring, and an unknown endpoint. 2. THE OUTPUT CONTRADICTED ITSELF: "reasoning effort: unchanged — already at the highest level" and "NOT raised to high: the active model does not support that level", in the same card. Both cannot be true. The fixed clause lived in Delta while the refusal lived in a separate line, so they were only ever consistent by coincidence. Delta now renders every clause from one DeltaState, with the effort clause a single switch over EffortTransition. A contradiction is unrepresentable rather than merely unlikely, and the separate line is gone. 3. THE BUDGET CLAUSE compared against thorough — "(thorough uses 160)" — which is information about two profiles, not about the user. It now states the caller's own transition, "turn budget: 80 → 320", with distinct renderings for an already-at-320 session and an unknown current budget.
…ing controls
PRE-EXISTING defect, surfaced while investigating the zeromaxing posture and
verified on origin/main. It is not something the posture introduced.
availableReasoningEfforts() returns an EMPTY ring for two very different
situations: a catalog model that genuinely has no reasoning controls (gpt-4.1),
and a model with no catalog entry at all (the reporter's glm-5.2 on
ollama-cloud, any custom openai-compatible endpoint). Every TUI consumer treated
the second as if it were the first.
That single fact produced two visible failures at once:
- /effort listed no levels and reported "no reasoning controls on this model"
- /effort high was REFUSED outright
- and it is why a profile's effort fill was declined with "the model does not
support that level"
Meanwhile the headless path has always disagreed. On origin/main, for the same
glm-5.2: forwardedReasoningEffort returns "high" and emits no notice, because an
unknown model "forwards the requested value as-is, since no support claim can be
made for it". So the CLI forwarded the level while the TUI refused to let the
user set it — one rule, two answers, predating this feature.
There are THREE consumers of "does this model take this level?": a manual
/effort, a profile's fill, and the headless forwarding decision. They now share
one rule — a catalog entry is authoritative and refuses; no entry means Zero
cannot vouch either way and does not block. TestEffortSettabilityAgreesAcross-
AllThreeConsumers pins that they agree, per model, including the reporter's.
/effort also distinguishes the two states in its output, because "this model has
no reasoning controls" and "Zero has no entry for this model" are different
facts and rendered identically before. Setting a level on an unlisted model now
says plainly that support is unconfirmed and an unsupported value is ignored by
the provider.
The regression tests drive the real /effort path rather than the helper. A green
helper test alongside an empty user surface is how this feature has now produced
four defects: the flag-parser hole, the self-correct capture point, the identity
test passing through the wrong gate, and this.
ZeroMaxing Phase 2: an `orchestrate` tool that accepts a plan as typed tool
arguments, validates it, records it as session events, and runs it SEQUENTIALLY
through the same specialist path a Task call uses. The deliverable is DATA — is
fan-out worth building at all — not concurrency.
ADDITIVE. With the posture off, the advertised tool set, the tool-definition
bytes and the assembled prompt are byte-identical to a build without this
feature, proved by TestPostureOffPrefixUnchangedByRegisteringTheTool: two
registries differing only in whether the tool is registered, asserted equal
under BOTH auto and unsafe. The enforcing mechanism is Safety() returning
PermissionDeny, not Deferred() — deferral only hides anything when it is ACTIVE,
and an unsafe session with deferral inactive would otherwise have advertised the
tool. Deferred() stays as a second layer and is now asserted rather than assumed.
VALIDATION CANNOT BE SKIPPED. Plan's fields are unexported and ParsePlan is the
only constructor, so there is no path from tool arguments to an executable plan
that bypasses it — the prototype's validator was reachable, correct, and never
called. Rejected by default: ids by ALLOW-LIST charset, unique; every depends_on
resolves (unknown edges rejected, never skipped); the graph is acyclic by Kahn's
with the involved ids NAMED (audit U24 — nothing in this tree checked, and a
cyclic graph hangs forever); task count capped; tools read-only and within the
parent's grant; a budget with max_tokens required and max_workers exactly 1,
REJECTED rather than coerced so the field stays meaningful for Phase 3; and
depth checked at admission with the remaining headroom named, rather than an
opaque failure mid-plan.
ONE counting function — a length, not a text scan. The prototype counted source
text, so `agent ("x")` with one space counted as zero and executed anyway.
BUDGET ENFORCED AT DISPATCH, not merely validated. Under this posture every
child inherits a 320-turn ceiling, so a twenty-task plan authorises 6,400 child
turns from one tool call. The grant is intersected again at dispatch too, so a
validator bug cannot widen a task's authority.
FAILURE IS NOT SUCCESS. A failed task skips its transitive dependents, each
RECORDED with the dependency that blocked it; independent siblings still run;
the plan runs to exhaustion. Partial is its own terminal status and the tool
returns StatusError for it, so nineteen of twenty failing can never surface as a
clean result (audit RC-F).
THE METRIC is value, not safety: max_speedup = sequential_total / critical_path,
computed from recorded per-task durations over the declared DAG and surfaced in
the summary and in Meta. Kill criterion: median >= 2.0x across >= 20 real plans,
or Phase 3 is not built. Independence-violation would answer safety, but
read-only tasks are always safe to parallelise, so it would read 0 and decide
nothing.
PermissionAllow when the posture is on, deliberately, with the reasoning in the
code so Phase 3 does not inherit it blind: the approval surface renders only the
tool name and a static sentence — PermissionRequest carries Args but no renderer
reads it — so a prompt could not show the plan it was gating and would train
click-through. What bounds it instead is enforced, not advisory: read-only at
validation and dispatch, a required budget enforced at dispatch, and the posture
itself as explicit consent. When tasks can WRITE, an approval gate becomes
necessary and needs a real renderer first.
Plan state is recorded as five ordinary session events beside the specialist
ones; no new store. Recording is best-effort and never fails the run.
…it 4 Wires Phase 2 into the real binary and maps a plan that did not fully complete onto the existing incomplete exit path. POSTURE CALLBACK — a POINTER, not a closure. The TUI model is a VALUE type (every handler takes and returns `m model`), so a func() bool closing over it would capture a copy frozen at registration and report the posture as it was when the session started, forever. Re-registration fails for the reason decision 2 rejected it earlier and worse: the TUI clones the registry per run and the clone copies tool POINTERS, so a replacement registered into the session registry would never reach a run already holding a clone. specialist.PostureGate is one shared atomic flag the tool holds for the process's life; every clone sees the same one and a posture flip needs no re-registration. PLAN RUNNER — captures only run-INVARIANT state (executor, workspace, parent identity and policy) and NO context. The ctx it uses is the one ExecutePlan hands it per task, so a cancelled run cancels the task in flight; it also checks ctx.Err() before launching, so a cancelled plan does not spend another task's budget. Capturing a context at construction is precisely how the prototype's background goroutine kept running after cancellation. The runner is synchronous and holds no goroutine, so it cannot outlive its plan. PLAN RECORDER — preserves execSessionRecorder's best-effort contract exactly. Every bridge method returns nothing, so a recording failure has no path back into ExecutePlan and cannot abort a plan mid-flight; nil is a no-op at every level. The bridge is built before the session exists and its inner recorder attached afterwards, so events before that point are simply not recorded rather than fatal. PARTIAL -> EXIT 4, folded into the EXISTING result.Incomplete path rather than a second exit route, so a partial plan and a stalled loop report the same way. It never overrides an incompleteness the loop already found. A DEFECT FOUND BY DRIVING THE BINARY, not by any test: the budget was enforced at dispatch against a counter that never moved. NewPlanRunner never populated TaskResult.Tokens, so a plan with max_tokens:1 ran all three tasks. The unit test passed because its fake runner fabricated its own token counts — the same "test the helper, miss the wiring" pattern that produced the flag-parser hole, the self-correct capture point, and the identity test's wrong gate. ExecResult gains an additive TotalTokens, populated from the stream summary the accounting path already computes. Verified at the real surface: orchestrate absent with the posture off and present as [shell/allow] with it on in the same auto-mode session; a 4-task diamond running in topological order with results verbatim and max_speedup 1.33x (hand-checked: four equal tasks over three hops = 4/3); cyclic, write-tool, max_workers and missing-budget plans each rejected with the offending ids or values named; and budget exhaustion reporting partial with `echo $?` showing 4.
Closes the two gaps left by the registration step.
TUI GATE. handleProfileCommand and revertExecProfile write the shared
PostureGate on every transition, threaded through tui.Options in the same shape
as ZeromaxingDisabled. What is proved without a TTY:
- the gate is written on posture ON and OFF through the REAL handlers, for
BOTH entry points (/effort and /profile) — not through a helper, which is
what missed the wiring four times in this feature
- a REFUSED selection (disabled workspace) leaves the gate off
- a nil gate is safe, so a caller that never wires one simply has no tool
- THE cloneToolRegistry HAZARD, proved rather than assumed: a tool reached
from a CLONE taken BEFORE a flip still observes the flip. That is the exact
ordering a captured copy or a closure over this value-typed model would get
wrong, and it is why the gate is a pointer
- concurrent Set/read, for -race: the TUI writes from its update loop while a
run's tool dispatch reads from the agent goroutine
FAILING-TASK VERIFICATION. The stub gained a prompt sentinel that makes one
specific child fail with a real provider-stream error, so a dependency failure
runs end to end rather than being asserted only at the executor. A 4-task
diamond where b fails now reports, verbatim:
Plan partial: 2 succeeded, 1 failed, 1 skipped max_speedup: 1.49x
- a [succeeded] full result verbatim
- b [failed] provider error: sentinel failure
- c [succeeded] independent sibling unaffected
- d [dependency_failed] skipped: dependency "b" did not succeed
Run incomplete (not reported as success) EXIT=4
Every level of the RC-F seam is now covered by something that can fail:
executor status, tool status, recorder, and the process exit code.
THE BUDGET DEFECT GETS ITS OWN GUARD. TestRealRunnerFeedsTheBudgetMeter drives
NewPlanRunner over an Executor whose child reports usage, and asserts a
max_tokens=1 plan launches exactly one child. A fake runner that fabricates its
own token counts cannot catch a producer that never populates them — which is
precisely how the defect survived every executor test. A second test pins that
the runner honours the per-call context rather than a captured one.
registerSpecialistTools regains its original name and takes the wiring
argument directly; the transitional wrapper is gone now that both call sites
pass wiring, so no dead indirection ships.
…n empty
The rule "a plan task may narrow the parent's grant, never widen it" was
declared, documented and validated against -- and both production call sites
left ParentTools nil, so neither the validation check nor the dispatch-time
intersection ever ran. A run restricted to grep handed a plan task read_file.
The empty case was worse. planToolGrant returned an empty slice when a task
declared no tools and the parent grant was empty; resolvedToolAllowlist read an
empty tool list as UNSPECIFIED and expanded it to the default read-only
category. The narrower the parent, the wider the child -- on the default path,
where a task declares no tools at all.
Three changes:
- The grant is now DERIVED inside registerSpecialistTools from specialist's
own exported read-only list, narrowed by the registry and the run's
operator filters. The filters are explicit parameters rather than wiring
fields, so a call site cannot omit them without failing to compile.
- Both intersections are unconditional. The "skip the check when the parent
list is empty" escape hatch is what made the rule inert; an unsupplied
grant is now a wiring bug that fails closed.
- Manifest gains ToolsResolved, so a resolved tool list can say
"deliberately nothing". A bare []string cannot: absence and emptiness are
both len()==0, and omitempty erases an empty slice entirely, so a nil check
would not survive a round trip. Same shape as audit finding M24. The flag's
meaningful value is true, so omitempty only drops the meaningless false.
planToolGrant now refuses an empty result rather than returning one, and
ExecutePlan records that as a task failure with a reason without dispatching
it, so an ungrantable plan reports partial/failed and exits 4.
reconcileProfileAfterModelSwitch called reasoningEffortAllowed directly while
the fill site called profileEffortApplies, added in 009f12b precisely to stop
treating an uncatalogued model's empty ring as a refusal. The two disagreed on
exactly that case, which is both of this user's configured models (glm-5.2 and
grok-4.5 are catalogued=false with an empty ring): selecting the posture on one
of them filled high, and switching to one with the posture already active
dropped the effort to auto and reported it as a level the model would not take.
e782470 was meant to fix this class at the cause across all consumers. It
updated one door and not the other -- the seventeenth instance of the pattern,
introduced by the fix for the sixteenth.
The rule is now ONE function, profileEffortAppliesOn, that both doors call, and
the ring's authority is passed through rather than inferred from its emptiness.
reconcileProfileAfterModelSwitch takes ringKnown, which its caller already had
and did not forward.
The comment at command_center.go described the old conservatism as deliberate
("only ever applies where support is known"). That is now the bug's
description, so it is replaced rather than left for the next reader to restore.
TestProfileEffortDoorsAgree asserts the doors against EACH OTHER across
catalogued / catalogued-with-no-ring / uncatalogued / no-model. Both doors had
tests before and both passed; nothing compared them, which is why the
disagreement shipped.
Three existing tests used kimi-k2.7-code:cloud as their "unsupported"
destination -- an UNCATALOGUED model, so they were asserting the defect. They
now use gpt-4o, which the catalog vouches has no reasoning controls, so the
genuine drop case stays covered.
task_completed carried session_id and tokens; task_failed carried neither. So the one task a user needs to open was the one task the event log could not point them at -- and the per-task token records did not add up to the plan's own total. Executor.Run already goes out of its way to preserve the child session id on a post-start failure, with a comment saying it exists so a failed child stays drillable. The recorder discarded it one layer up. A dependency or budget skip records an empty session id rather than omitting the field: that task never started a child, and present-and-empty says so, where an absent field would look like the same drop being fixed here. The regression test compares the two events against EACH OTHER. Both already had tests and both passed; nothing compared them, which is how the asymmetry survived.
PlanContext.Depth was unset at both call sites, so Limits.CurrentDepth and the executor's own nesting check both measured a constant zero. The admission headroom check could never fire. It was only harmless because plan tasks run at --auto low, where specialist tools do not register, so nothing could re-enter -- an inert guard that reads like a live one, which is the shape someone later trusts. Wired rather than deleted: the value exists (--depth), it costs two lines, and the check it feeds is the one thing standing between a nested run and maxSpecialistDepth if a future phase ever lets a plan task spawn. The TUI passes an explicit 0 with a comment: it is always a root session, so zero is the measured value there, not an unset one. Proven at the process boundary rather than by a unit test, since the defect was the call site and not the helper: before, `exec --depth 7 --exec-profile zeromaxing` ran a four-task plan; after, it refuses with "a plan's tasks would run at depth 8, which reaches the maximum nesting depth of 8", while --depth 6 still runs.
…ch other RULES.md §3: for every rule you touch, find its sibling and prove by test that both agree -- one test comparing the paths on the same input, not two tests each asserting their own expectation. The authority rule has two enforcement points and Workstream A changed both; two sites fixed together is exactly the shape that drifts apart later. The relationship asserted is SUBSET, not equality, and the direction is the point. Admission is stricter: it rejects a task naming anything outside the grant. Dispatch narrows: it drops such a tool and grants the rest. Requiring them to refuse identically would assert a symmetry the design does not want -- dispatch only runs on an already-admitted task, so when it disagrees its job is to hand over less, never more. Mutation-checked: dropping the parent check from the dispatch intersection is caught with "the second layer widened authority", which is the defect the "BELT AND BRACES" comment promises cannot happen.
A plan ran silently: sequential, synchronous, no output until it finished. Four things stood between the existing live-progress machinery and the plan tool, all of them wiring rather than missing capability. loop.go gated the progress callback on `call.Name == "Task"`, so the second sub-agent-spawning tool got nil. Keyed now on the TOOL'S OWN DECLARATION (tools.ChildProgressStreamer) rather than its name: `|| call.Name == "orchestrate"` would have been the same defect one name later, which is this codebase's most repeated shape. Every tool that does not declare keeps the nil callback it has today -- swarm tools included, deliberately. NewPlanRunner built TaskRunOptions without Progress and RunWithOptions never read options.Progress. Either half alone leaves a plan's children streaming to nobody, so they are ONE change with one end-to-end test. This is finding 7's class again: a second construction path that does not carry what the first one did. PlanRunner now takes a PlanTaskRequest struct rather than a widening parameter list, because each new positional argument is another chance for that omission. app.go never set Recorder, so the TUI recorded NONE of the five plan lifecycle events (finding 9). The recorder is now a required POSITIONAL parameter of registerSpecialistTools, like the operator filters: forgetting it is a compile error rather than a silent nil. TaskCancelled is a distinct outcome. Cancellation marked every remaining task failed with "context canceled", so a deliberate Ctrl-C on a twenty-task plan read as nineteen defects. PlanCancelled joins it as a terminal status, and terminalStatus counts it -- without that a plan with two successes and two cancellations reported "completed", which is RC-F exactly. Also fixes a live render-cache collision this change would have made certain. A rowSpecialist row carries no id and no text; every distinguishing field lives behind row.specialistInfo, and none of it was in the cache key. Two specialist cards in one run hashed identically, so a failed sub-agent rendered as a successful one. Rare with Task (one child per run is typical), guaranteed with a plan. Proven by test before fixing, then asserted with four cards including a failure and a cancellation. Additivity re-proven: posture off, all five permission modes byte-identical against an origin/main binary with DeferThreshold unset, hashes unchanged.
Gate 1a made a plan's tasks visible as cards. Cards cannot show the one thing that makes a plan a plan: its shape. This adds the panel. The shape is carried on planAdmittedMsg, not assembled as tasks finish. ParsePlan already proved the graph acyclic and computed the topological order, so the panel is complete before the first task starts rather than growing a node at a time. Each task is ranked by dependency depth -- one more than its deepest dependency -- and indented by it, so a diamond reads as one node, two beside each other, one node. A chain steps in once per link; independent tasks share a rung. All three are asserted. Mounted above the pinned update_plan panel, since it describes work running right now. It renders NOTHING when no plan has been admitted -- not an empty box, not a blank line -- which is what keeps a posture-off session unchanged: the panel never runs, rather than running and producing nothing. Named orchestrate throughout. /plan (update_plan's TODO list) and /plans (this) are genuinely different things with near-identical names, so every type, field and function here says orchestrate and only the user-facing command says plans. The command lists every task including any the pinned panel had to drop, and names each task's dependencies explicitly, so the shape survives copied text and narrow terminals where indentation does not. Bounded at 12 rows and it states what it hid: a silently truncated list reads as a complete one. Summaries cut on rune boundaries. The full result stays in the tool output -- the panel summarises, and a display formatter on a data path is how a 583-rune work product became 200 mangled runes. The header clock stops when the plan ends, and when the run ends without a terminal plan event. It previously kept counting while the turn that produced the plan carried on, so a 20-second plan read as minutes. TestSuggestionOverlayCapsRowsWithoutMoreText pinned the first and last entries of the command palette's first window; inserting /plans pushed /ps out of it. The window's start and size are unchanged, so the tail anchor moves to /permissions and the comment says why it is incidental.
The struct comment always claimed a plan task "inherits exactly the parent's policy". It did not: both production call sites left PlanTaskContext.ParentModel empty, so appendModelArgs received no parent model and passed no --model, and the child fell back to whatever its own config resolved. After a /model switch or a --model flag, plan tasks ran on a different model than the run that launched them. Decided: the documentation states the safer behaviour, so the code moves to match the doc. Explicitly NOT a router decision -- choosing a cheaper model per task is a real future feature and would arrive as an explicit plan field on top of correct inheritance. Silently failing to propagate is not that feature. The fix is not "populate the field at registration", which would have been wrong for the TUI: its registry is built once per session while /model changes the model between runs, so a captured value is stale by design. That is why the fields were empty rather than merely unset -- they were in the wrong home. ParentSessionID/ParentModel/ParentReasoningEffort now travel per call on PlanTaskRequest, read from the same tools.RunOptions fields the Task tool reads, and are gone from PlanTaskContext so there is no second source to drift from. Before: parent ran --model parent-chose-this, child reported stub-model. After: child reports parent-chose-this. Asserted per call, so a second orchestrate call with a different model launches on that model. The parent session id travels with it, which is what links a plan task back to the run that spawned it rather than leaving it an orphan.
Launching a specialist and resuming one are two doors onto the same question -- which model does this specialist run on -- and they answered it differently. BuildResumeArgsInput carried no model fields at all and BuildResumeArgs never called appendModelArgs, so a resumed child was launched with no --model and fell back to whatever its own config resolved. The same specialist could change model between its first turn and its second. Verified by building both argument sets side by side rather than by reading: FRESH args: exec --init-session-id … --model parent-chose-this … RESUME args: exec --resume … (no --model) The fix calls the SHARED helper rather than repeating its rules, so the manifest-pins-its-own-model precedence is applied once and cannot drift between the two paths. The relationship the regression test asserts is EQUALITY (RULES.md §3): fresh and resumed launches of the same specialist must produce the same model and the same reasoning effort. A second test drives Executor.Run with Resume set, since the builder having the fields proves nothing if the call site never fills them -- which is exactly how the two paths diverged. PRE-EXISTING main defect, not ZeroMaxing: it affects every resumed specialist. Separate commit so it can carry its own issue. Note for Stage 2c: runResume also resolves the manifest by session.AgentName and ignores params.Manifest, which is why inline-manifest children are unresumable. Same function, still open, deliberately not fixed here.
planReadOnlyTools bounds what a plan task may hold; readOnlySpecialistTools decides whether a specialist counts as read-only for the Task tool's permission gate. Different questions, same answer -- and they disagreed: lsp_navigate was in the first and not the second, so a manifest holding exactly the plan grant failed the read-only check. The omission was chronology rather than a decision. readOnlySpecialistTools was written in Gitlawb#243 (2026-06-18); lsp_navigate arrived in Gitlawb#276 two days later and nobody went back. The tool declares EffectReadOnly with the safety reason "reads files, modifies nothing" and resolves its path through the same scoped confinement its read-only siblings use, so it belongs in the set. Pinned as a SUBSET, not equality, and the direction is the point: anything a plan task may hold must be something the wider gate also calls read-only. The reverse is not required -- update_plan is read-only for a specialist and deliberately not a plan tool. NOT derived from the tools package's declared capabilities, which would be the real class fix: update_plan is EffectInteractive, so deriving the set would silently drop it and change the Task tool's permission gate. That is its own change with its own reasoning, not a drive-by.
A read-only run carried ~5 KB of confirmation policy governing actions it could not take. Measured on a real plan-task child: 8,365 bytes of the 17,903-byte system prompt -- 47% of it -- were policy for capabilities the child provably lacked (no write, no edit, no shell, no ask_user), and the system prompt is 71% of that request. Multiplied by every task in a plan and every specialist sub-agent. Keyed on the RESOLVED TOOL SET, never a name or a manifest flag, and fail closed at every step: a nil registry, a run holding nothing, and any tool whose effect is not positively read-only all keep the policy. EffectUnknown -- MCP tools, plugin tools, the specialist tools -- keeps it, which is why an ordinary session is untouched. Two things this got wrong first, both caught rather than reasoned away: It used ToolVisible, which applies the permission mode's ADVERTISING rules on top of the operator filters. A write tool auto mode does not advertise is still held and still callable once approved, so a run launched with --enabled-tools read_file,grep,glob,write_file counted as read-only and lost its policy -- a fail-OPEN exactly inverse to the purpose. Found by driving the binary; every unit test passed at the time. It now asks ToolAllowedByFilters, which answers "does this run hold the tool". Reading the registered tool set made registering a posture-gated tool change the prompt with the posture OFF, breaking the additive-only guarantee. TestPostureOffPrefixUnchangedByRegisteringTheTool caught it, which is what that test exists for. A tool the run can never call now says nothing about what the run can do -- and a tool that varies its permission by argument is never ruled out from its static safety, however that reads. Verified on the wire: --enabled-tools read_file,grep,glob drops it (12,310-byte prompt); adding write_file or bash keeps it (18,789). Additivity re-proven byte-identical across all five permission modes. Only the confirmation policy is gated. The remaining ~3.3 KB (Editing discipline, Testing gate, Permission and safety) lives inside system_prompt.md and needs that file split, which touches the prompt every run shares. Separate change.
Two changes from driving a real six-task chain. THE TOKEN BUDGET IS NOW OPTIONAL, AND OMITTED MEANS UNBOUNDED. max_tokens was required and capped at 200_000, and a chain asking for exactly that spent 469,555 -- 2.3x over -- and still lost its last task. The check runs only BETWEEN tasks: a task is dispatched whenever any budget remains and then spends whatever it spends, so the cap bounded nothing while stopping real work from finishing. A number that neither bounds spend nor lets a heavy plan complete is worse than no number, because it reads like a guarantee. Spend is still METERED and reported everywhere it was -- PlanReport.TokensUsed, the plan_completed event, the panel footer -- so an unbounded plan is not an unmeasured one. A caller that wants a bound still sets max_tokens and gets the same dispatch-time behaviour as before, asserted by test in both directions. This removes the only ceiling on what one orchestrate call can spend. Under the zeromaxing posture a twenty-task plan authorises 6,400 child turns. That is the user's decision, taken after seeing the cap fail to do the job it existed for. THE PANEL COLLAPSES. A six-task chain took seven footer lines for the whole run, pushing the conversation up for detail that is one keypress away. The header alone is the state; the tasks are detail. Click the header line to open it -- the interaction that was actually asked for -- or Ctrl+G. NOT Ctrl+O, which already toggles the detailed transcript view: the first attempt bound it there and the existing handler silently won. Not Ctrl+P either, which belongs to the update_plan panel. The dependency indent is capped at five levels. A twenty-task chain adds a rung per link, which would indent forty columns and run off a narrow terminal; the depths still reflect the real graph, only the drawing stops. Past the cap every task sits at one depth, which is honest for a chain.
TWO SEPARATE THINGS FROM DRIVING A REAL PLAN.
Finished tasks now drop out of the panel after a 5s linger, so it tracks live
work instead of accumulating a transcript of it. Long enough to see a task land,
short enough that a six-task chain does not fill the footer with history.
Matches how the AGENTS sidebar already retires finished agents.
A faded task is hidden, not forgotten: the header still counts it, and /plans
still lists the whole plan. That matters, because the dependency shape fades
with the tasks -- a diamond stops looking like a diamond once its first task
goes. /plans is where the shape stays readable for the whole run, and it names
each task's dependencies explicitly rather than relying on indentation.
/effort NOW SAYS WHAT IT ACCEPTS.
On glm-5.2 the card reported "available: not listed - model is not in Zero's
catalog" and stopped. Nothing typeable, on the model this user actually runs.
And on a catalogued model it listed low/medium/high while omitting zeromaxing --
which is selected through this very command, and which the actions line already
offered.
"available" (what the CATALOG vouches for) and "you can set" (what the command
accepts) are different questions and were conflated into one line. They are now
two fields.
The distinction is enforced, not decorative:
- catalogued with a ring -> exactly that ring
- catalogued with NO ring -> nothing but the posture. gpt-4o genuinely has no
reasoning controls and the command refuses every
level there, so offering them would advertise
what the command rejects
- not catalogued -> low/medium/high, because Zero cannot vouch
either way and the headless path forwards them
- posture disabled -> the posture is dropped from BOTH the list and
the actions line, since suggesting a command the
run will refuse is worse than saying nothing
The card and the command are two doors onto "which efforts can I set", so the
parity test feeds every advertised value back through the real command rather
than comparing two lists -- a list comparison would pass while the command
rejected all of them.
The popup that /effort opens showed a single "auto" row on glm-5.2. Reported from use, with a screenshot; I had just told the user no such picker existed, which was wrong -- newEffortPicker has been there all along. It read availableReasoningEfforts(), the CATALOG's answer. A model with no catalog entry has an empty ring, so the picker offered nothing settable -- on the model this user runs. And it never offered the zeromaxing posture on ANY model, even though picking one routes straight into handleEffortCommand, which accepts it. The picker now shares settableEfforts() with the /effort card, so the three surfaces that answer "which efforts can I set" -- card, picker, command -- cannot disagree. Catalog authority is preserved: gpt-4o, which the catalog vouches has no reasoning controls, still offers only auto and the posture. It also preselects the active posture. Under zeromaxing m.reasoningEffort holds "high" -- the level the posture FILLED -- so preselecting from that alone would highlight the wrong row. The parity test now feeds every value from BOTH surfaces back through the real command, and asserts the two offer the same set. The picker is the surface that actually failed in use while the card's own test passed, which is the argument for testing them against each other rather than each against its own expectation. Two existing tests encoded the old behaviour. One asserted "[auto] as the only effort option on an unsupported model" -- the reported bug, written down as an expectation; an uncatalogued model is not an unsupported one, and its levels are forwarded. The other pressed enter expecting to land on auto, which only held while auto was the sole row; it now selects auto deliberately and asserts the preselection instead.
All five from one screenshot of a six-task chain, all in what the run REPORTED rather than in what it did. "error (exit code 0)" on a failed task, directly above a body reading "Subagent failed (exit 4)" -- the card contradicting its own detail. A plan task's failure carries no exit code, and rendering the zero value claimed one it did not have. Only shown when non-zero now. Dependency-skipped tasks rendered "error". specialistCancelled landed in specialistStatusString's default arm, so a plan with ONE real failure showed three errors: the failure plus every task skipped behind it. The default stays "error" so an unmapped status still fails closed. "0 tokens" in the specialists rollup while the plan reported 130,135. Nothing has ever populated specialistInfo.tokenCount -- not for plan tasks, not for Task sub-agents -- so the total was always zero. The segment is now omitted at zero, matching the per-card rule (M18): a number that looks measured and is not is worse than no number. A plan task's spend now reaches its card, so the rollup adds up to what the plan reports. Task sub-agents still bridge no usage; that gap is unchanged and now shows as an absent segment rather than a false zero. The sidebar said "no active plan" while the panel two lines below showed one mid-flight. Its PLAN section reads the update_plan TODO list and knew nothing about orchestrate plans. It now shows a one-line summary -- name, done/total, failures, the task in flight -- when an orchestrate plan is running. EXACTLY one line, because the FILES section below computes its click offsets from the lines above it. The sidebar test drives renderContextSidebar rather than the line helper: the first version called the helper directly, and the mutation that removes the section from the sidebar entirely still passed.
The footer read "budget 0/200000 tokens" for an entire run while the cards directly above it showed 84,953 spent. tokensUsed was assigned in exactly one place -- from plan_completed, which arrives when the whole plan ends -- so the one number a user watches that line for was zero for as long as it mattered. Spend now accumulates as each task finishes. The executor's own total still wins at the end, since it counts every task including any whose message the panel dropped as stale, but a plan that reports no total no longer erases what was counted.
Pressing the PLAN header opens a two-pane drill-in: every phase down the left, the selected phase's live agent state on the right — status, elapsed, dependencies, tool count, spend, its prompt, what it is doing right now, and where it ended up. Up/down moves, enter drills into the child session, esc closes. An OVERLAY, in the same slot the help and picker overlays use, so it composites over the chat rather than replacing it. Nothing about the transcript, footer or composer changes, and it renders nothing when closed or when no plan has been admitted — which is what keeps a posture-off session unchanged. No separate posture gate: a plan only exists under the posture, so without one there is no PLAN line to press. The panel's own expand/collapse stays on ctrl+g, for a glance without leaving the conversation. Opening lands on the task worth reading: the one running, else the first that failed, else the first task. The selection clamps rather than wraps. Enter is offered ONLY when the selected task has a child session to open. A running task is still keyed by a temporary id, and a hint for a key that does nothing trains the user to ignore hints. The phase list marks the task in flight. Running and pending both fell into the default glyph, so the list could not show what was actually happening. Prompts are SUMMARISED and say when they were cut; the full text stays in the tool output. A task that never started shows what it is waiting on rather than a row of zeroes. Eight mutations, all caught. Two needed the tests fixed first: the running-task assertion compared whole rows, which differ by the selection marker anyway, so removing the in-flight glyph passed; and nothing drove the click path at all.
The right-hand column showed one grey line — "trace-chain 3/6 · helpers" — for a plan the panel below it was rendering in full. It now carries the plan properly: a progress count in the header and one coloured line per task, in the same shape the update_plan steps use so the section reads consistently whichever plan is in it. green ✓ done · red ✗ failed · accent • in flight · faint ○ pending faint ⊘ skipped or cancelled — deliberately NOT red, since neither is a defect The header count turns red on a failure and green when everything is done, so the one thing worth noticing carries from across the screen. THE LINES LIVE INSIDE sidebarPlanLines, and that is not a detail. sidebarFileSelectables computes the FILES section's click offsets from len(sidebarPlanLines); the previous one-liner sat beside the placeholder and was safe only because it was exactly one line. Rendering a whole section there would have misdirected every file click by the number of lines added, silently. Inside the function the arithmetic stays correct by construction. Live tasks first, so a long plan shows what is happening rather than what already happened, bounded at six lines with the remainder stated — the column is 26 to 40 cells wide and shares its height with AGENTS, FILES and ACTIVITY. The panel and the detail view are where a whole plan is read. update_plan's own steps still win the section when it has any: the two are different things and the TODO list is the one the model is actively editing. The plan's NAME is not in the sidebar. At this width it would cost a task row, and it is on the panel and in the detail view.
…live detail
The overlay was wrong and the screenshot showed why: composited into the
transcript column, a twelve-task plan drew its phase list straight through the
user's own prompt. It is gone.
The right column already owns "what is happening" — agents, files, activity —
and had half a screen padded with blank lines down to the token floor. The plan
lives there now:
PLAN 1/8
█████████░░░░░░░░░░░░░░░░░ 1/8
✓ t-sandbox x
✗ t-agent y
• t-tools Read the Go package at…
○ t-redaction
+2 more
TASK t-tools
running
52 tool calls · 21,400 tok
blocks pair-a
↳ read_file internal/tools/registry.go
Read the Go package at internal/tools
A progress bar that keeps done, failed, skipped and running in their own
colours inside one span, sized to the column rather than to a fixed width, and
never rounding a segment away — one failure in thirty still gets a cell,
because a failure the bar hides is a failure it does not report.
TASK fills what was dead space, and yields first when the column is short: the
list above it is what the section is for. It shows the REVERSE dependency edge
too — what a task blocks — because a failure matters in proportion to what
waits on it and the forward edge alone does not say.
Click a task to select it, click PLAN to collapse the section, ctrl+g to cycle
the selection from the keyboard. The sidebar is not focusable, so a mouse-only
affordance would not be one.
THE OFFSET ARITHMETIC IS THE RISK HERE, not the drawing. The progress bar sits
between the header and the first task, so every clickable row below it moves
down one — in the plan's own hit table AND in the FILES section beneath. Both
are adjusted, and both are asserted against where the row actually RENDERS
rather than against a re-derivation of the sum: the first version of the FILES
test recomputed the arithmetic and would have passed with the bar unaccounted
for on both sides.
Nine mutations, all caught. Two needed the tests rewritten first — the FILES
one asserted nothing, and the bar one used numbers that never round to zero.
…s offered The posture raises a cost multiplier — 320 turns per run, inherited by every sub-agent — and it was a word in amber, in the same weight as everything beside it. "Is it on?" should be answerable from across the room. The footer chip is now a FILLED badge: the label sits on the amber rather than in it, which is what reads as lit rather than as another label in the row. Off still means ABSENT, not dim — the footer is byte-identical without the posture. It breathes while a turn is in flight, ◦ • ● ◉ ● •, on a 1.4s cycle. Slow on purpose: this marks a standing state, not activity, and a fast blink beside a working spinner reads as a second thing happening. THE PULSE IS FREE, and that constraint shaped it. It advances on the spinner tick that is ALREADY running during a turn and holds a steady full glyph otherwise. ensureSpinnerTick deliberately schedules no timer on an idle session — "an idle plain session schedules no timer" — and a chip is not a reason to break that. Steady rather than frozen mid-cycle, because a half-lit chip on an idle session would read as a rendering fault. Reduced motion pins it, like every other animation here. The /effort picker marks the posture row too, so what you are about to turn on looks like what you will see once it is on. Marked with a glyph rather than merely coloured: the selected row already owns its background, so colour alone would be invisible exactly when you are looking at it. The marker is display only — the VALUE handed to the command stays the bare name, or selecting it would be refused as an unknown effort. Seven mutations, all caught. Two needed the tests fixed first: the chip test called the helper rather than the footer, so reverting the footer to plain amber text passed it; and the value test located the row BY value, so the moment the marker leaked into the value the row stopped matching and the test passed by finding nothing.
…is clickable The chip is now clickable — pressing it opens /effort, where the posture can be turned off or changed — so it highlights under the cursor. A clickable chip that looks identical to a label never teaches anyone it can be pressed. Not while a turn is in flight: /effort refuses mid-run, and opening a dialog that cannot be acted on is worse than opening none. Its hit region is measured from the RENDERED footer rather than assumed. The chips before it — permission mode, effort — vary in width with the session, so a fixed offset would drift the moment either changed. The sidebar's plan rows highlight too, joining the agent, plan-step and file rows that already did. The hover is held by TASK ID, not row index, for the reason the others hold identities: the rendered row set changes as tasks finish and fade, with no mouse motion in between to re-resolve it, and a cached offset would light up whatever slid into the slot. The /effort picker already highlighted its rows on hover — selectGenericPicker- AtMouse live-previews on motion — so the zeromaxing row needed nothing. Six mutations, five caught. The sixth is belt-and-braces rather than a gap and the comment now says so: the posture-off guard in the chip's hit test is a fast path, not the enforcement — with the posture off the footer carries no chip, so the span lookup fails anyway.
Family-specific prompt guidance was selected by matching the model id against a list of prefixes - gpt, o1, o3, o4, gemini, claude - and nothing else. Of the providers in the catalog, only a handful ship a default model that matches any of them. The rest, including the recommended default and every gateway a user is most likely to reach for, matched nothing and silently received no guidance at all. Nobody decided that; the switch simply fell through. OpenAI's own OAuth entry was correct for one reason: "gpt-5.5" happens to begin with "gpt". OpenAI has already shipped o1, o3 and o4, each of which needed a new arm added here to keep a first-party provider working, and the next id is under no obligation to help. The provider knows the answer without being asked twice. So the catalog carries the fact. Descriptor.ModelFamily is set ONLY where a provider is first-party and single-family - openai, chatgpt, chatgpt-proxy, anthropic, google - and left empty everywhere else, which is the honest answer for a gateway: Ollama Cloud speaks openai-compatible and serves Qwen, and reading the wire format as a family would hand GPT-shaped guidance to a model that never asked for it. Empty means "ask the model id", and the id is genuinely the only thing that separates openai/gpt-4.1 from z-ai/glm-4.6 on one endpoint. Options.ModelFamily carries it to prompt assembly, which prefers it over the guess. In the TUI it is read from the LIVE profile inside the per-run assembly rather than resolved once at startup - /model can switch provider mid-session, and a family fixed at launch would keep describing the provider the session began on, which is the staleness that had plan discovery assigning from the wrong provider's model list. What this does NOT do: the providers that declare nothing still receive no addendum. Whether GPT-shaped rules help Qwen or GLM is untested, and inventing guidance on a guess is the failure this replaces rather than a fix for it. The difference is that the silence is now a declared position, and a family can be added in one line when there is evidence for it.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
A saved plan is stored args re-admitted through ParsePlan, which makes it
exactly repeatable and completely fixed. "Audit internal/tui" cannot be run
against internal/cli without opening the file and editing the prompt of every
task, so in practice a plan gets copied per target and the copies drift from
the one that was reviewed.
Task prompts may now contain ${name}, and `params` fills them in.
WHAT MAY BE SUBSTITUTED IS DELIBERATELY NARROW: prompts and the plan
description. Not `tools`, which is the authority grant; not `id` or
`depends_on`, which are the graph; not `model` or the budget. A parameter that
could reach the grant would turn "run the sweep plan with scope=x" into a way
to widen what the plan may do, and one that could reach an id would let a
single argument silently detach a dependency edge. Those are fixed when the
plan is saved and reviewed, and they stay fixed.
Expansion happens BEFORE ParsePlan, so the plan that runs is the plan that was
validated - same tool-grant narrowing, depth cap and budget checks - rather
than a template of it.
It fails closed in both directions. A placeholder with no value would leave a
literal "${scope}" in a prompt, which reads to the model as a directory that
does not exist. A value matching no placeholder is almost always a typo, and
ignoring it runs the plan against the wrong target while reporting success.
Both are refused, naming the offender and what the plan actually takes.
The stored plan is never mutated: LoadPlans caches by path, so rewriting it
would leave the next run carrying the previous run's arguments. A plan with no
placeholders is untouched, so every plan saved before this is inert to it.
MaxTurns is a proxy for cost that does not track cost. A measured heavy run reached its 320-turn limit having spent 35,781,390 tokens; a cheap run reaches the same limit at roughly a tenth of that. The turn count bounds round trips. Nothing bounded spend. Options.MaxTokens does. Zero is unbounded, which is every caller that does not set it, so nothing existing moves. Spend accumulates from what the provider actually reported rather than from an estimate, at the same point the compaction estimator is calibrated - past error recovery and past any reactive compaction that re-sent the request, so it counts the exchange that happened rather than one that was abandoned. Checked at the TOP of a turn, for the same reason the turn count is: the previous turn's tool calls have run and their results are in hand. Stopping mid-turn would discard work the run has already paid for, which is pinned by a test. Crossing it takes the same ending MaxTurns takes - one final call asking for a summary - so a budget expiry produces a written report of what was done instead of a run that simply stops. Reusing that path rather than inventing a second ending also means there is one place where a run can end early. WHICH BOUND FIRED NOW REACHES THE READER. A turn-count stop still says "max-turns limit"; a spend stop says "token budget", in IncompleteReason and in the prompt the model is given. Reporting the first for the second is how someone comes to raise a turn limit that was never what ran out - which is exactly what happened when the 320-turn stop was read as a turn problem. This is the mechanism only. Nothing sets MaxTokens yet, so the posture-off request body stays byte-identical; wiring the profile's budget and choosing its value is a separate change against a more delicate path.
320 turns was cutting real work short. A measured run spent every one of them on legitimate sequential edit-verify work and was forced into a summary before it could write up what it had found - and the check that this was not simply wasted turns is in the data: of 49 turns that looked batchable, 43 were repeat reads of the SAME file, where the next call's arguments depend on the previous result. The realistic saving from perfect batching was 4 turns out of 321. So the ceiling moves to 480, which is triple thorough's rather than double. The multiple stays exact so the relationship remains something a reader can check, and the doc comment moves with it. NOT HIGHER, and the reason is the token curve rather than caution. In that run the prompt grew monotonically from 35k to 186k tokens per turn and had not flattened, so turns near the end cost five times what early ones did. 640 would land near 360k per turn and push the run into compaction - which is where "paste the actual terminal output" quietly becomes a summary of the output, the exact failure the evidence contract exists to prevent. AND THE TWO MOVE TOGETHER. Raising the turn ceiling alone would double the worst case while still ending runs at an arbitrary place: that run spent 35,781,390 tokens reaching 320, and 480 on the same growing prompt reaches well over 100M. Profile.MaxTokens bounds spend, and only the posture sets it - every other profile stays unbounded, so nothing outside it gains a ceiling nobody chose. 50M is anchored on measurement, not preference: the largest observed legitimate run spent 35.8M and needed roughly 45M to finish, so this lets that work complete while stopping a run that has stopped making progress. TWO RUNS IS A THIN BASIS - the mechanism is what matters, and the number is the part most likely to be wrong. Deliberately NOT exported to the environment the way the turn budget is: a child inheriting the parent's ceiling would be handed the whole budget each, so a ten-task plan could spend ten times it. The parent's bound is the parent's.
The posture's evidence contract rides the turn loop's reminders, and a plan task runs as a child process that BuildArgs never hands --exec-profile - so a task starts with the posture OFF and the contract never reaches it. The tasks that fan work out are exactly the ones whose claims land in the plan's report unchecked. Most of the substance was already in this prompt: claims backed by what was read, quoted file:line, an honest "not found" preferred to a plausible guess. Two rules were not, and both are ones a task can violate straight into the report a reader will trust: A passing test is not proof that a property holds. A measured submission scored a property top marks in five places on the strength of "the test passes" - while the test consumed the lazy iterator it was meant to prove lazy, discarded the value whose absence was the bug, and reported avg x 2.5 as a p99. A number must come from a command actually run. The same submission reported a table of timings no command in the session produced, with the same test reading 0.86s in one paste and 4.20s in the next. Stated in this file rather than shared with agent.ZeromaxingEvidenceNotice because this package CANNOT import agent - agent's own test binary imports specialist, so the reference would compile and then break `go test ./internal/agent` with a cycle, the same trap PostureReasoningEffort exists to avoid. The tests pin the RULES rather than the wording, so the text stays free to change and a deletion still fails. NOT FIXED HERE, and worth naming: the measurement tripwire still has a blind spot for plan work. It runs in the parent's loop against the parent's own tool output, and a plan task's commands run in the child's session - so a number a task invents is caught by neither. Closing that means either a ledger inside each child or feeding the child's streamed tool results into the parent's, and both cross the agent/specialist boundary this comment is about.
…urvive Two things a plan run lost, both found in one measured plan. A TASK'S OWN COMMANDS ARE THE CHECK ON ITS OWN NUMBERS. The parent's measurement tripwire compares the parent's answer against the parent's tool output; a plan task's commands run in the CHILD's session, so a figure a task invents was caught by neither. The child's tool results do stream to the parent, which is what makes the check possible at all: each task now keeps a ledger of what its own commands printed, and its answer is checked against it. A conflict is REPORTED, not retried - the parent's version sends an answer back for another turn, and doing that here would spend a whole task to re-ask a question the report can simply carry the answer to. It lands above the fold because it is the one finding a reader cannot check for themselves: the command ran inside a child's session and its output is not in the report. A CUT-SHORT TASK MUST HAND SOMETHING ON. withDependencyBriefing already carries a cancelled task's output to its dependents labelled INCOMPLETE, so the machinery for a follow-up to pick the work up existed - and was fed nothing. A task killed mid-edit-loop has written no prose, so Output was empty: a measured run lost 858,231 tokens of real work with no record of what it had touched. The handoff is built from the child's own ChangedFiles tool results, not from a model call. By the time the meter trips the child's process is already gone, so a summary turn would mean resuming a task that just overspent - paying more for the report than the report is worth. The file list is free, exact, and cannot be invented: a file is on it because a tool reported changing it. Prose the task did write is kept and the list appended, never the reverse.
… wait
Two constants contradicted each other:
maxEmptyPollYield = 5 * time.Minute // a poll may wait this long
defaultCompletedRetention = 30 * time.Second // ...but we forget after this
Zero prints "Use write_stdin with session_id N ... to poll", accepts a
five-minute poll, and forgot the answer after thirty seconds. In a measured run
a 60-second test was started, polled with yield_time_ms 40000, and the id was
already gone - so the SAME 60-second test ran again to recover a result the
first had produced. The reply told the caller "do not guess or probe session
ids" about an id this manager had issued and instructed it to poll.
Retention is now DERIVED from the poll bound rather than chosen beside it, and a
test pins the relationship rather than the number, so moving the poll bound
cannot silently reopen the gap.
Past that window a finished session is still remembered - id, command, exit code
and the output TAIL, which is where a result, a failure and an exit summary
live. A late poll gets that instead of an error. An id this manager never issued
still returns ErrProcessNotFound, so a real probe is refused and
UnknownExecSessionError stays id-invariant for the repeated-failure guard that
keys on it.
Recorded when the process exits, not only on the polling paths. The common case
is that NOBODY polled while it ran - the reason to background a command is to go
and do something else - and that is the case that was measured. It is also the
case my first six tests did not cover: deleting that capture left them all
passing, which is why a real-process test for it exists now.
Each is inert until the wiring commit that follows, so this one adds no behaviour on its own. SCOPE.EXTRAROOTS. A write-capable plan runs in a worktree so it has "somewhere to write that is not the user's tree" - one branch, one diff, a discard path. Scope.Roots returns the workspace root AND anything granted beyond it, which is right for "everything this run may write" and wrong for a field documented as "the directories THIS RUN may reach outside its workspace". Wiring the first where the second was meant handed an isolated child the parent's tree straight back. ExtraRoots answers the second question. NOT the fix I first proposed: workspaceWriteAutoAllowed looked like the culprit, but an outside-workspace path is blocked earlier and that branch was never reached - the path was not outside. The sandbox was doing its job with the roots it was given. SESSIONBUDGET. Depth caps nesting, maxPlanWorkers caps concurrency, a session shows one plan at a time, and a run is bounded by spend. All bound ONE run or ONE plan; a conversation that keeps starting more work was bounded by nothing. 200, because across 75 recorded sessions the most any one started was 22. A COUNT, NOT A COST - the failure that shaped the token budget was one run spending 35.8M tokens, which a count would not have seen. Counted on admission and never decremented: the question is how much work this conversation set going, not how much runs now. REQUIRE-PLAN-KEYWORD. Once the posture is on, orchestrate exists for the rest of the session, and everything the model reads shares context with the user's instructions. An imperative sentence in a file, a PR comment or MCP output reads like an instruction to the tool that spends the most. So a session may require that a plan was asked for in the user's OWN words, checked at admission rather than requested in the prompt - a prompt rule is advice to the same model the injected text is talking to. That needed the turn's raw user text to reach a tool, which nothing carried; RunOptions.UserMessage does now. An empty message is NOT consent, or the gate would be off in exactly the headless and scheduled paths most exposed to untrusted payloads. MEMORY. A session ends and what it worked out goes with it. internal/memory reuses plan_store's answers verbatim - an allow-list name that cannot spell a traversal component, symlink refusal on directory and file, an O_EXCL temp file renamed into place - because a note store is a write primitive pointed at a path the model chooses, the same shape as "save my plan". Reading is free; writing prompts, because a note is read back and believed in every future session. Local scope by default. Plan tasks reach NEITHER tool: a stale note could steer a whole fan-out, and twenty tasks racing to describe one finding is the update_plan problem again. Both new config flags default off, which is what keeps a build carrying this byte-identical to one without it. A project may ENABLE the keyword gate - asking to be handled carefully is not a privilege escalation - but never switch it off. A project may not enable memory at all: memory_write writes into the workspace and project config is not trust-gated.
Separated from the code it wires because every capability in the previous commit is inert without this, and because app.go touches all four - so folding it in would have made three of the four commits unbuildable on their own. Bisecting a branch whose middle does not compile is worse than one commit that does two things. ExtraRoots replaces Roots for a child's write roots, so an isolated plan task stops being handed the parent's tree back. One SessionBudget for the whole session, shared by every run and plan in it - a per-run counter would reset on each message and bound nothing a conversation does over time. The keyword gate and the memory tools are read from resolved config, so both stay off unless asked for. The memory tools are registered HERE rather than in the core tool set on purpose: adding them there changes the advertised tool set for every run, which is what this branch guarantees the posture does not do when it is off. Measured by watching the posture-off request body diverge, and reverted.
anandh8x
requested changes
Aug 3, 2026
anandh8x
left a comment
anandh8x
left a comment
Collaborator
There was a problem hiding this comment.
I tested this from the TUI as well as reviewing the diff. The core path works: zeromaxing activates, a two-task read-only plan completed correctly in parallel, and the specialist/TUI/CLI tests pass.
I am requesting changes for the remaining user-facing issues:
- The bundled research plan still uses the literal "THE SUBJECT". The documented "/plans run research" path cannot provide a subject, so it can spend five child-agent runs researching a placeholder.
- The budget schema describes limits but does not declare its minimums and maximums. In the manual run the model emitted max_tokens_per_task: 5000, Zero rejected it, and the model had to retry. The schema should prevent that invalid call.
- The zeromaxing chip hit-test still searches every footer row for the label. Typing the same word in the composer can make the composer row become the hover/click target instead of the status chip.
- The completed plan UI showed both "AGENTS 2 done" and "no agents spawned", and raw child session_id values were printed inside the plan result.
The size also makes this difficult to review safely: 36,607 additions across 198 files and 96 commits. About 22k lines are tests, which is good, but there are still roughly 14.2k production additions spanning posture, scheduling, persistence/resume, background execution, write isolation, model routing, TUI, sandbox, credential-store, and worktree changes. These are independently reviewable seams and would be safer as focused PRs.
The feature has real value, but I do not think this draft is ready to merge until the concrete issues above are fixed and the scope is reduced or split into reviewable pieces.
Vasanthdev2004
requested changes
Aug 3, 2026
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
Collaborator
There was a problem hiding this comment.
Read through this properly rather than skimming the diff — 36k lines is a lot, and most of it is solid. The plan/orchestrate model hangs together, the isolation design is the right shape, and the commentary explaining why things are the way they are is genuinely better than most of what lands here.
Two things need fixing before it goes in, and one of them is the reason I'm requesting changes rather than nitting.
1. Isolated plan tasks can write to the real repo — internal/specialist/exec.go:366
The write-capable path promises isolation and doesn't deliver it. Chain:
appendExtraWriteRootArgsskips a root only whenroot == input.CwdScope.Roots()returns the parent workspace root first, then extras- for an isolated task
input.Cwdis the worktree, so the parent repo root isn't skipped — it goes out as--add-dir <parentRepo> - the child feeds
addDirsintosandbox.NewScope(workspaceRoot, extras), andAddfails withwrite root %q, so these are write roots, not read
Meanwhile planWorkspaceNote tells the user "nothing was written to the parent tree", and the tool schema says naming a write tool "runs it in an isolated worktree and asks for approval first". So a task the user approved on that promise can write_file/apply_patch/bash anywhere in their actual repo.
I don't think this is carelessness — a worktree's .git file points back at the parent, so some git operations genuinely need write access under <parentRepo>/.git. But that argues for granting that path specifically, not the whole tree. Whatever you land on, the note and the schema text need to describe what's actually enforced.
internal/specialist/child_scope_test.go:36 only covers Cwd == the parent workspace, which is why this passes today — the isolated case, the one that matters, isn't exercised.
2. /effort auto mutates a run in flight — internal/tui/session_controls.go:93
You already wrote the argument for this guard, on the branch right above it:
Same idle-session rule /profile enforces… The budget propagates to sub-agents spawned later in the same run, so changing it mid-run leaves one turn running under two different budgets.
The entering branch checks m.pending. The leaving branch calls revertExecProfile() with no check at all, and /effort auto is reachable mid-run. Sub-agents spawned later in that turn inherit the reverted budget while earlier ones kept the posture's, which is exactly the straddling state /turns and /profile refuse. It also flips the orchestrate gate, so the tool disappears from under a model that's mid-plan.
Same guard, same reason, other direction.
Worth fixing in the same pass
internal/cli/plan_model_probe.go:43—providers models <name> --verifylooks like it probes the active provider rather than the named one, because the prober routes the selected profile throughlivePlanProvider. If so,--verifyreports the wrong thing whenever the named profile isn't the active one.plan_exec.go:720/plan_runner.go:98—budget.max_tokens_per_taskis applied per attempt. With retries and fallbacks a task can spend a multiple of its declared cap, which makes the cap advisory rather than a bound.internal/agent/loop.go:595—Options.MaxTokenscounts only the parent's own provider usage, so orchestrate and sub-agent spend never lands against the posture's spend bound. For a posture whose whole point is running long, that's the number people will assume is holding.internal/cli/provider_models.go:92—--verifyfans out one goroutine per discovered model, uncapped. The specialist path next door caps at 8 for exactly this reason.plan.go:483(from my earlier round, still open) —seconds > 0means a negativemax_wall_seconds/max_stall_secondsreads as "unset" and the plan runs unbounded. A negative value should be an error, not an off switch.plan_exec.go:510— paused time is credited to the wall clock only once the pause ends, while the watchdog keeps counting through it, so a long pause can trip the budget.plan_exec.go:413—cutShortkeys on a non-nil error the production runner never returns, so a task in flight when a plan is stopped records asTaskFailedrather thanTaskCancelled.
Smaller
orchestrate_saved.go:382 — the cross-plan resume guard compares plan names, but names are optional, so two unnamed plans both compare "" and it passes. orchestrate_saved.go:237 — planTaskSummaryLine cuts only at \r\n and leaves ESC through, unlike the sibling you already fixed; same for the failed-task error text at sidebar.go:668. These strings are model-authored, so escape sequences reach the terminal. zeromaxing_glow.go:208 — chip hit-testing takes the first footer line containing "zeromaxing", which composer text can shadow. plans/research.json:8 still ships literal "THE SUBJECT" with no placeholder declared, so /plans run research dispatches five sub-agents on an unanswerable prompt.
Three tests don't test what they're named for: plan_watchdog_test.go:386 asserts parent.Err() != nil on a context.Background(), which can never be true; plan_grant_test.go:377 puts its only assertion inside if err == nil && len(models) > 0, so the error property it names is never checked; scope_temporary_test.go:19 only tries macOS paths, so all five refcount tests — the only coverage AddTemporaryWrite has — skip on Windows and Linux.
Where that leaves it
Fix 1 and 2 and I'll re-review the whole thing rather than sending you back round again — I know CodeRabbit has already bounced this five times and I'd rather this be the last round from me. The middle group I'd want addressed or argued with; the smaller ones you can take or leave, but the three dead tests I'd fix since they're currently reporting green on properties nobody is checking.
Nothing here is a design objection. The shape is right.
…ameter
research.json read "Find where THE SUBJECT is DEFINED" — prose describing a
hole rather than a parameter declaring one, so nothing refused it and nothing
filled it. Run from the documented path it dispatched five child agents to
search the repository for the literal words "THE SUBJECT".
A stand-in that is not a ${parameter} is invisible to expandPlanParams, which
is the one thing in this package that would otherwise have caught it. So the
plan now declares ${subject}, and a test asserts no builtin plan carries a
prose stand-in — the next bundled plan gets the guarantee without anyone
remembering to ask for it.
That alone would have made the plan unrunnable from the path that names it:
/plans run took a name and nothing else. It now takes the words after the
name, and fails closed in both directions — a plan declaring a parameter will
not run without one, and words handed to a plan declaring none are refused
rather than dropped. A single parameter is resolved by the command, since
everything after the name is the value; several are named to the model so it
fills those and invents none.
Refused BEFORE the turn is dispatched. expandPlanParams refuses at admission
too, but only after a model call has composed the plan and the run has begun.
For a five-task plan the difference is five sub-agent runs.
A manual run emitted budget.max_tokens_per_task: 5000. Admission refused it for sitting below minimumPlausibleTaskTokens, correctly — and the model spent a turn and a call discovering a rule that was written down only in prose, in a description it had already read. Every bound here was already enforced; none was declared. A schema minimum is checked by the provider before the request is made, so the invalid call is never emitted rather than being caught and retried. Each bound mirrors its enforcement constant rather than repeating the literal, because two spellings of one limit drift. A test pairs them and admits the declared floor while refusing one below it: a declared bound that does not match enforcement is worse than none, since it teaches the model a rule the code does not hold.
The hit test scanned every row footerView renders and took the first one containing the label. But footerView is not only chips: it renders the plan panel, the idle hints, a queued-message preview and the composer. So typing "zeromaxing" into the composer put a matching row above the chip's, and the hover target became the composer — a click meant for the chip landed on text the user was still writing, and the chip itself stopped responding. A plan task prompt carrying the word did the same through the panel. The chip renders in statusLine and nowhere else, so the search is narrowed to the rows the status line occupies: rendered separately and taken off the end of the footer, which is where statusLine writes on every branch.
A completed two-task plan drew
AGENTS 2 done
no agents spawned
sidebarSpecialists drops a finished agent once it is past its linger, so the
section had no lines to render; doneAgentCount counts exactly those agents, so
the header reported two. The header and the body were describing the same run
and disagreeing about it.
The count is the true one — two agents ran. The placeholder was reporting the
emptiness of a filtered list as the emptiness of the session, so it now says
what is actually there and names the toggle that brings them back. "no agents
spawned" survives for the case where it is true.
…ession id BuildFinalResult prefixes "session_id: <id>" so a Task caller can continue the child it started. A plan task has no such caller: the plan owns its children's lifetimes, and TaskResult.SessionID already carries the id structurally. Its output, though, is quoted into the report under "result:", pasted into the dependency briefing every downstream task reads, and rendered in the plan panel — so the line surfaced a raw child session id in the middle of a user-facing answer three times over, and told every dependent task to treat it as a finding. Removed at the plan boundary, keyed on the id we were handed rather than on the pattern: the first line goes only when it is exactly the prefix followed by that id, so a child whose own prose opens the same way comes through whole. The Task path keeps the line, which is what it exists for. Asserted at the caller as well as the helper — a real task through the real executor seam — because a helper that works while nothing calls it is the defect this repo keeps finding.
… routing Independent improvements to the plan/orchestrate subsystem, built and verified together on this branch. All are posture-gated: the posture-off cold-start request stays byte-identical. - Identity-aware resume. A plan resumes from where it stopped; a completed task's bounded output survives into the tasks that depended on it; and an edited task (plus everything downstream) re-runs instead of replaying a stale result. Bare "/plans resume" continues the last cancelled run. - Read-grant propagation to sub-agents. A plan's tasks inherit the parent's request_permissions READ grants through a new --add-read-dir flag applied with scope.AddRead, so a read-only audit of a granted external path no longer fails "outside the workspace". A read grant is never emitted on the write channel, so it can never become writable in the child. - Size-aware per-task model routing. Free providers (no prices) tier by the model size parsed from the id instead of falling through to alphabetical; the router is shown explicit size labels; a configurable plan_models.min_size floor drops models known to be too small; and `providers models` shows the classification. - Plan-run observability. Per-worker token and model display, a worker view folded from the accounting events, background-agent completion/status, and durable per-task numbers that survive resume. Every production change is covered by a regression test; mutation-checked.
Review feedback: Done() and Remaining() were only ever called by their own tests — the resume path narrows through RemainingPlan and never touched either, and the comments falsely claimed orchestrate_saved consulted Done() first. They are also unsafe to wire in as suggested: Done() is id-based, so with the identity-aware resume a plan whose ids all succeeded but whose task was edited would be reported "done" and its re-run silently skipped. Removed both; the one test that asserted "a failed task is remaining" now asserts it through the real gate (RemainingPlan) instead of the dropped helper.
Collaborator
Author
|
Thanks for the careful reviews. Important context first: the fixes for most of these were committed but unpushed when you reviewed — the fork branch was behind, so the code you saw predates them. Everything is now pushed; here is where each point stands against current HEAD. @anandh8x — all four addressed
@Vasanthdev2004 — addressed
|
…ents A bare Task delegation inherited the parent's model unconditionally, so under zeromaxing every sub-agent ran on the session model no matter what it was for. A Task that names no model is now routed to the configured role pin — the plan path's own classifier and pins, the grant outranking the prose — gated on the posture AND the autoAssign config, so a posture-off spawn stays byte-identical. Four defects found and closed while verifying the seam end to end: - A resumed task drifted back to the parent's model. runResume applied neither the call's model nor the assigned one; it now recovers the model the session actually ran on from the session store, with an explicit model on the resume call winning exactly as on a fresh launch. - A plan task could be second-guessed at dispatch. Plan tasks flow through the same executor, and a model the plan tool deliberately left empty (a reported decision, or auto_assign:false) would have been re-assigned blind — including to a pin the plan level had passed over as not served. autoTaskModel now refuses plan-authored manifests, keyed on the provenance the manifest already carries. - Naming an uncurated model killed the spawn. applyTaskModel forwarded the posture's raised reasoning effort unconditionally; the child only clamps efforts for models the registry knows, so providers that reject the parameter refused the whole request — a real orchestrator lost three children to it and retreated to inherit-everything. The effort now forwards only where the registry can vouch, the same gate the plan path already applies. - A stale pin after a provider switch killed the spawn. Pins named Ollama models while the session ran on xai, and three children died with not-found. A pin now fires only when the provider's own listing carries both the pin and the session's model (the plan path's provider-mismatch guard); anything uncertain inherits. One listing is cached for two minutes, so a five-agent fan-out costs one probe. Every load-bearing line is mutation-checked: the pin lookup, the manifest-model guard, the posture gate, the session-model recovery, the explicit-resume-model apply, the plan-provenance skip, the effort vouch gate and the served-set guards each have a test that goes red when the line is gutted.
…te-aware animation, settled cancels While the posture is on, the workspace wears it; off, every surface is byte-identical to before (each gate mutation-checked). MODELS panel: the sidebar grows a section showing the live mix of models the fleet runs on — a proportional mix bar plus a row per model with live counts, colours stable per model name. Absent until an agent runs on a known model, so a plain session's layout is untouched. The skin: an electric gradient (theme blue blended into the accent — deliberately two-tone) paints the always-on surfaces: the composer box on all four edges, the chat|sidebar divider as a full-height rail, the sidebar section headers, and the Working ripple. The gradient sweep is mirrored, so no seam ever reads as "half painted". No new timers: every walk reads the spinner clock that already runs during a turn, and reduced motion pins all of it. State-aware animation: thinking drifts, writing races (4x), and orchestrating gets a scanner — a bright band sweeping the composer bar while plan tasks are actually running. The Working ripple's wavelength follows the same states. Plan bars join the skin: done work fills with the gradient, the running head burns accent, failure stays findable in calm dimmed red, and the update_plan checklist gets a progress bar of its own (the orchestrate bar's renderer, so the two cannot drift). Click offsets for plan steps account for the inserted bar row, test-pinned against the rendered sidebar. Settled cancels: cancelling the run kills the children but the trackers were never told — agents kept their spinner, ticking clock and "live" mark in MODELS beside "Run cancelled." in the transcript. cancelRun now marks running specialists and orchestrate tasks cancelled (not failed — stopping a run is not a defect), freezing their clocks and dropping the live counts.
The stock #50fa7b green and #ff5555 red are the highest-chroma signals in the whole registry, and in a zeromaxing session — success ticks and error rows arriving constantly — they read as alarms, not status. The success and failure roles now wear Dracula's signature cyan and pink, which sit naturally beside the purple accent and stay clear of amber's permission meaning. The palette token names stay green/red (every consumer binds to them as success/failure); diff colours keep their conventional hues — a diff is a diff on every theme. The registry-wide contrast floors still hold.
The status card stacked three renderers — a labelled list, the resolved
posture line, and the delta sentence — so the profile name appeared
twice and the effort and turn budget three times each. It is now a
single sentence of key-value clauses: each fact once, the transitions
as clauses on the row they qualify ("was 80", "raised by the posture",
"NOT raised — high is unsupported on this model"), no section header
and no hint advertising the command the user just typed.
Every honesty guarantee the old card's tests enforced survives on the
one line: the refused effort level is named, the self-correct claim
tracks live state ("your /selfcorrect off overrides the posture"),
exactly one effort-transition claim exists, and the escalation row keeps
the headless-only asymmetry. /effort still carries the shared delta
sentence verbatim, so the switch notice and that surface cannot
disagree. Also learned and recorded: compactCommandOutputText collapses
every whitespace run, so padded column alignment can never survive this
renderer.
Collaborator
|
@gnanam1990 sorry for the wait. I went straight at the two blockers on current HEAD rather than re-reading all 49k lines, because you have been sitting on this since this morning. Both are fixed in one place and missed in a twin. Three one-line misses and then I think this is done. Isolation.
Negative Fix those three and I will approve. No more rounds from me after that. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
How to review this PR
It is one PR by necessity — the areas share core files (
plan_tool.go,plan_exec.go,app.goare each touched by many commits), so they cannot becleanly split into separate PRs without breaking builds. But the changes are
independent by concern. Review area by area, top to bottom; each is
self-contained and these are the seams a split would follow.
orchestratetool — the zeromaxing rung and the DAG tool, validated by one constructorplan.go,plan_tool.go,plan_gate.go,plan_keyword.goplan_schedule.go,plan_exec.go,plan_events.goplan_identity.go,plan_resume.go,orchestrate_saved.gorequest_permissionsREAD grants via a new--add-read-dirflag, applied read-only (never writable)sandbox/scope.go,cli/exec.go,cli/exec_parse.go,plan_runner.gomin_sizedecency floor; the router is shown size labels;providers modelsdisplays themplan_model_size.go,plan_model_assign.go,plan_model_router.go,config/types.gotui/orchestrate_*.go,sidebar*.go,specialist_card.go,worker_view.goEvery production change has a regression test beside it (~61% of the diff is
tests); the tree builds and
go test ./...passes; with the posture off theorchestrate tool is unadvertised and no plan machinery runs (see Verification
for the precise additivity claim and its one documented exception).
Summary
Adds zeromaxing — an explicit, opt-in posture that lets a turn spend more to
get a more exhaustive answer — and the plan orchestration it exists to drive.
The posture is a rung above
highon the existing effort ladder, reached by/effort zeromaxing,/profile zeromaxing, or--exec-profile zeromaxing. Itraises the turn budget, widens the sub-agent allowance, and advertises one new
tool:
orchestrate, which takes a declared DAG of tasks and runs them as childagents, in parallel where the dependency graph allows.
The whole feature is additive. With the posture off, nothing here executes.
That is not an aspiration — it is the property every commit on this branch was
checked against, and the check is in "Verification" below.
What landed
zeromaxingrung; lifecycle reminders injected below the cache breakpointorchestratetoolmax_workers1–16 walks the validated topological order. Measured 40.0s → 27.0s at 4 workerssmall/medium/large/unrestrictedtiers; project config may only tighten what user config sets/plansverbs: list, save, run, resume, restart, stop, pauseDesign notes worth review
No new store. Plan state is derived from five session events beside the
existing specialist ones. Resume is a reduction over them, which is why a plan
recorded by the TUI and one recorded by
zero execresume identically.One scheduler, not two. The sequential path is the concurrent one with a
single worker. Two executors would have been easier to write and impossible to
keep in step — a duplicated rule drifts, and a duplicated executor drifts faster.
Optional interfaces over name switches. Control, per-task progress, isolation
and concurrency each arrive through a type-asserted optional half, so a recorder
that only records is unaffected and no existing signature changed.
One plan per surface. The panel holds one plan and the card table is keyed by
task id — unique within a plan, not between two.
PlanSurfaceBusyenforcesthat at the tool, on the path the model drives, matching the guard
/plans restartalready had on the path a user drives.Verification
Additivity, the load-bearing claim. With the posture off, the orchestrate
tool is unadvertised and no plan machinery runs; for a write-capable run the first
HTTP request body is byte-identical to an
origin/mainbinary. One documentedexception, orthogonal to the posture: a run holding no mutating tools omits the
~5 KB confirmation-policy block (
runCanMutate), so a read-only run's prompt issmaller than a pre-feature build — deliberate, fail-closed, and independent of
whether the posture is on. Re-proven after
every commit; both binaries built fresh, baseline from a clean
origin/mainworktree:
Gauntlet.
go build ./...,go vet ./...,go test ./...all pass.gofmtclean. Concurrency-sensitive packages run under
-racewith repeat counts.Mutation checking. Every fix on this branch was verified by reverting the
production change and confirming the test fails. Several tests passed initially
for the wrong reason and were rebuilt until they bit — the misses are recorded in
the commit messages rather than quietly fixed.
Fan-out measured end to end, not asserted:
max_workers=1→ 40.0s,max_workers=4→ 27.0s on the same plan.Known gaps
is hardened; the remaining ~60 call sites are their own campaign.
Notes for the reviewer
This is a draft, and deliberately so: it is large. Per
CONTRIBUTING.mda PRneeds an approved parent issue and one PR should carry one change — this carries
a feature. Happy to split it along the seams above (posture rung / orchestrate
tool + executor / durability + resume / TUI surface), each of which builds and
tests independently, if that is the preferred shape.
UI changes are best seen running; screenshots can be added on request.
Summary by CodeRabbit