Session onboarding: ask what the work is, derive the role — and the role mechanism was broken - #73
Merged
Merged
Conversation
Accepted design, not yet implemented. Subsystem A of two; the orchestration harness (operator + subagents + an independent reviewer) is subsystem B and lands separately. Adds no new concepts. Every piece already exists and none of it fires: operator-helper-protocol.md already says the session ASKS before doing anything; agent-role.py already claims and materializes; preflight already has --require-role, opt-in; AGENTS.md already says .env is asked, never inferred. There is no hook of any kind. So the protocol depends on an agent reading a 328-line index and choosing to comply, which on 2026-08-04 cost a silently merged VARIANT of another session's binding numbers. Design: --require-role becomes the DEFAULT (harness-neutral, unlike a hook, and the push chain means an unclaimed session cannot land anything). A script cannot ask a question and a hook cannot converse, so the split is fixed: agent-onboard.py --probe reports state, the AGENT asks, agent-role.py makes it a fact. The canonical interview lives in workflow.md where an agent reads it. The interview asks about the WORK, not the vocabulary: a long campaign implies operator, one scoped change implies helper, just looking implies read-only. read-only is a declared ABSENCE of claim, not a third role — it exists because forcing a question-answering session to take the repo-wide lock or a throwaway worktree is how --no-require-role becomes habitual and the gate erodes. Mode is declared with the role: interactive by default, headless only when stated. Never inferred from the hour, silence, or a long task. .env is asked just in time, never as an up-front walkthrough; unanswered values stay empty, which AGENTS.md already defines as gates PENDING. FOLLOWING_AGENTS_PROTOCOL Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]
Implementation plan for subsystem A of .agents/specs/session-onboarding.md. Task 1 ships the probe WITHOUT --env-set on purpose: had it shipped both, Task 4's tests would have gone green on arrival and the env writer would never have had a real RED to go green from. Every task carries the doc-checkpoint obligation explicitly and verifies the COMMITTED form, because preflight runs that checker --staged only, which passes vacuously after committing — the trap that left 11 red commits on the last branch. FOLLOWING_AGENTS_PROTOCOL Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]
…ep 1) A script cannot run an interactive prompt in any harness-neutral way, so the probe reports and the agent asks. .env.example is the only source of legal keys, and a missing FILE is reported distinctly from missing VALUES, an unreadable one from both, and a queue that could not be computed from an empty one. resolve() already separates "never declared" from "locked out by another live operator", so the probe carries that through instead of sending a blocked session to a claim that will fail. mode has no source until step 2 and renders as a default, never as a declaration. FOLLOWING_AGENTS_PROTOCOL Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]
Task 1 found the budgets are binding: STATUS.md sits at 284,071 of a 284,081 cap and the BENCHMARKS NOT APPLICABLE paragraph at 676 of 700. Five separate additions do not fit, and compacting unrelated evidence to buy room is exactly what produced a cross-arm supersession claim on the previous branch. So every later task EDITS Task 1's line, rolling it 1/5 -> 2/5 -> all 5, true at its own commit and net-zero in size. FOLLOWING_AGENTS_PROTOCOL Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]
Task 1's review found my plan text was wrong in four ways, three of them the same class the probe exists to prevent — an absence rendered as a claim: 1. test_declared_role_renders_with_its_row put ENG-FOO in BOTH the row and the fixture queue, so the queue line satisfied the assertion and deleting row rendering entirely stayed green. 2. Nothing tested the undeclared-role interview hint, which is the probe's whole point; removing the block stayed green. 3. probe() dropped resolve()'s operator_held_by_other and reason, making the new front door LESS honest than the tool it wraps: a session locked out by another live operator rendered identically to one that never declared, and both were told to claim operator, which will fail. 4. mode was read before step 2 teaches resolve() about it, so an undeclared mode rendered as a definite "interactive" while an undeclared role correctly rendered UNDECLARED. Now rendered as a default, explicitly labelled. Counts: Task 1 now 11. FOLLOWING_AGENTS_PROTOCOL Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]
Task 1 measured it: docs/STATUS.md is at EXACTLY its 284,081 cap and the BENCHMARKS paragraph at 699 of 700. A digit roll (1/5 -> 2/5) is free; "all 5" is two characters more and would go red. So the sequence ends at step 5/5, and a task that needs more room shortens its OWN sentence. Also corrects the step-5 JSON expectation: the probe emits 9 keys now, not 6, after the review added blocked_by_other_operator, reason and queue_error. FOLLOWING_AGENTS_PROTOCOL Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]
read-only is a declared ABSENCE of claim, not a third role: no lock, no worktree, and CLAIMABLE_ROLES stays exactly two so every write path keeps keying on it. Headless is declared with the role and never inferred. FOLLOWING_AGENTS_PROTOCOL Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]
Task 2's review found both by mutation: - `ROLES = DECLARABLE` leaves the whole suite green, so "must not be widened" — the constraint that keeps read-only out of every write-gating call site — is enforced by comment only. Now asserted. - `agent-role.py show` exits 0 for read-only, so --require-role is satisfied by a declared ABSENCE of claim. Correct for a plain run, wrong for --staged, so Task 3's refusal is what actually closes the write path. Now pinned. FOLLOWING_AGENTS_PROTOCOL Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]
The obligation already existed in prose and in an opt-in flag, and neither fired. --require-role is now the default with --no-require-role to opt out, and the failure carries the interview rather than an error code, because a gate that does not say what to do next is a gate people route around. The mutation suite anchors on the default assignment itself and on the line that records the failure, so neither a silent revert of the default nor a quiet downgrade of the gate to a cosmetic print can stay green. read-only passes a plain preflight and fails --staged: reading is free, writing is a claim. Turning the flag on exposed a spec defect that made the gate unpassable rather than strict, so a role now keys on the WORKTREE and never on the session (user-directed). agent-role.py claimed its session id was "stable across tool calls within a session (measured)"; it is not, in at least one real harness -- every tool call gets a fresh shell and no environment survives it, so a role claimed in one call resolved as UNDECLARED in the next and preflight exited 1. The marker already lives in the worktree's own git dir, so one worktree is one role; the session is kept as provenance and gates nothing, and the false "measured" claim is gone. Both surviving invariants are asserted across REAL second worktrees: the lock stays in the git common dir so one operator per repo still holds, and a helper's marker cannot leak, since a helper materializes its own worktree. The accepted cost is that two sessions sharing one checkout share a role. operator-helper- protocol.md loses the two claims this falsifies, ahead of its fuller rewrite. Claiming an operator lock that is already this worktree's now rewrites the record rather than passing: it renews the heartbeat, and it stamps the worktree onto a pre-correction lock, which otherwise let two worktrees under one session id both resolve as operator. Two mutation gaps found by review close with it: the ROLES alias may not widen to include read-only, and read-only-cannot-stage is pinned, since agent-role.py show exits 0 for read-only and a declared ABSENCE of claim must not satisfy a write gate. FOLLOWING_AGENTS_PROTOCOL Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]
…havioural pin Task 3's re-review found both outside the lines Task 3 corrected, and both pass every existing gate today: NOW.md still says --require-role is opt-in, and operator-helper-protocol.md:203 still says "session-scoped role marker" — the exact claim corrected 130 lines earlier in the same file. It also found the text anchors catch a REWRITE of the default but not an OVERRIDE: keep REQUIRE_ROLE=1 and add `REQUIRE_ROLE=0 ` with a trailing space on the next line, and the suite stays green while the gate stops failing. Every assertion in PreflightWiringTests greps text; only executing the script closes the class. Task 5 now runs it. FOLLOWING_AGENTS_PROTOCOL Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]
It writes an untracked file that gates read, so an unrecognised key or a
clobbered line fails silently and surfaces later as a mysteriously PENDING
gate. Unknown keys are refused, the first write seeds from .env.example, and
repeat writes update in place without duplicating or disturbing other keys.
The value is the one field nothing else validates, and both defects review
found live there. Both were reproduced before the fix and after it.
Line separators: guarding "\n" and "\r" guarded 2 of the 10 separators
str.splitlines() breaks on, and splitlines() is what BOTH the reader and the
rewrite use. A "\v" or a U+2028 smuggled a forged pair past the key check,
the probe then reported the forged key as SET, last-wins made it effective,
and the next write materialised it as a real LF line on disk for the shell
loader too. The guard now asks the question instead of listing characters:
the value must survive a round trip through the reader that parses it back.
Duplicate keys: breaking on the first match left a trailing override -- which
a hand-maintained .env routinely carries -- still effective for both the
reader and `set -a; . ./.env`, while the exit code was 0 and the message was
reassuring. That is the silent no-op this command exists to rule out,
arriving from the other side. Every match is now collapsed onto the first
line's position, so the effective value is the written one.
Two smaller disagreements between the file's two readers close with them: an
existing-but-unreadable .env is refused the way env_state already classifies
it rather than dying in a bare traceback, and a value needing shell quoting is
quoted, since an unquoted `/pa th` made the documented loader run `th` and
leave the variable EMPTY while the probe reported it PRESENT. Empty stays
bare, because shlex.quote("") is a two-character string the probe counts as
set, and unanswered must keep reading as unanswered.
Three hardenings from the first pass stand: main() dispatches on `is not None`
so `--env-set ''` cannot fall through to the probe and exit 0 having recorded
nothing; an empty value stays legal and clearing one stays possible, because a
writer that refused empty would push its caller toward inventing a value from
a path or a username; and the success line says so rather than letting an
empty write read as a win.
The five briefed tests all exercise cmd_env_set() directly, so the flag and
its dispatch were unprotected: deleting either left all five green while the
only entry point an agent types stopped recording anything. Pinned.
Mutated 17 implementation lines one at a time, including a regression to each
of the two reviewed defects; every test named for one went red, no survivors,
and each anchor was asserted unique before the edit. The module docstring
loses its "it never writes" claim, which this change falsifies.
FOLLOWING_AGENTS_PROTOCOL
Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]
…t (A step 5)
check-protocol-consistency.py exists because an obligation was once migrated in
AGENTS.md and the checker but not in the manual, which went on instructing
agents to do the thing the migration had removed. A gate whose prose lives
nowhere is that failure with the polarity flipped, so the interview lands in
workflow.md and the checker asserts it is there: the marker, and every answer
agent-role.py accepts, bound to DECLARABLE so a fourth answer must reach the
prose. main() calling interview_errors is pinned separately against a copy of
the tree, because a checker that defines the rule and never applies it leaves
every direct-call assertion green.
Two claims that survived the machinery and passed every gate are gone. NOW.md
said `--require-role` was still opt-in -- printed at the end of every preflight,
contradicted by the run that printed it. operator-helper-protocol.md's W0 row
still said "session-scoped role marker", the exact claim corrected 130 lines
above it.
The behavioural pin the text anchors could not give: every other assertion about
this gate greps the script, which catches a REWRITE of the default and misses an
OVERRIDE -- keep REQUIRE_ROLE=1 and add `REQUIRE_ROLE=0 ` (one trailing space)
further down and the whole class stays green while the gate stops failing. The
new test EXECUTES the script with no resolvable role and asserts it exits
nonzero. Two mechanics were forced: a role keys on the WORKTREE, so the session
id cannot make one unresolvable and the run is pointed at an empty GIT_DIR
instead, asserted as a precondition rather than skipped; and a full nested run
would recurse without bound, since preflight runs the suite that calls it, so
--role-only runs the role block and stops.
test_agent_onboard.py was registered in preflight but in no CI job, so the whole
onboarding suite gated only locally; it joins the role-machinery step.
.agents/specs/orchestration-harness.md is the accepted subsystem-B design,
committed as-authored.
The final whole-branch review is folded in here rather than appended, because
every finding is this branch failing its own subject -- prose matching
behaviour -- and a branch that ships the correction as a second commit has
shipped the false claim.
"every write path refuses" was FALSE in all three places this branch committed
it (AGENTS.md, workflow.md, the spec). Grepped and reproduced: the only refusal
that exists is agent-preflight.sh --staged. `git commit`, `git push`, the
`gate && git push` chain AGENTS.md itself mandates -- which runs preflight
WITHOUT --staged -- and every record or matrix edit all proceed; a read-only
marker takes `--role-only` to exit 0. Repo-wide refusal is a much larger change
than this branch, so all three now say what holds: --staged refuses, past
staging it is the honour system. The spec's work item 3 is split into the
DELIVERED half and the DEFERRED half ("the role check runs before any record
edit"), which was dropped between phases where no per-task review could see it,
and the SDD ledger records the deferral so it is tracked rather than lost.
Two behaviour bugs, both born of Task 3's worktree-keyed lock and neither
reachable before it. resolve()'s operator-marker-without-lock branch never set
operator_held_by_other, so being locked out by a LIVE RIVAL rendered exactly
like never having declared -- no NOTE from render_probe, no note from cmd_show,
`blocked_by_other_operator: false` in the JSON, and the generic "a long campaign
-> operator" hint pointing at a claim that exits 1. That is the defect Task 1's
finding 3 was raised about, never followed to this branch. And `claim operator`
then `claim read-only` -- the exact downgrade an operator types -- left the lock
behind: the marker went read-only, heartbeat answered "not the operator; nothing
to heartbeat" so nothing renewed it, and a second worktree was refused for the
full 2h TTL by a session holding no role. cmd_claim now releases a lock it owns
whenever the claimed role is not operator, by the same ownership test release
uses, so a downgrade cannot free somebody else's.
probe() had ZERO field assertions. Five hardcodes each left 31/31 green:
blocked_by_other_operator False, reason None, mode "headless", role "operator",
env "present". Every existing probe test fed render_probe a FIXTURE, so nothing
asserted the front door workflow.md sends every session to reads resolve() at
all -- and a probe answering `role: operator` to an undeclared session is the
eleventh instance of this branch's signature class. One test class claiming
through the real CLI in a throwaway repo kills all five; env is pinned against a
substituted env_state, because a developer with a complete .env would make the
"present" hardcode true.
--role-only's own docstring claimed it "checks strictly more than
--no-require-role". It runs none of the 15 record gates or 16 mutation suites
that flag runs; neither mode is a superset of the other, and it is the ROLE it
checks more strictly.
operator-helper-protocol.md named `.agents/operator.lock`, a path that does not
exist -- the lock is <git-common-dir>/vllm-cpp-operator.lock, and state.md
already recorded the deviation. This branch rewrote the W0 row and left the
wrong path standing inside it; the two pre-existing hits are corrected too.
docs/BENCHMARKS.md lost an unrelated parenthetical (the live-state audit
follow-up: evidence-rule limit, ACTIVE precondition, count/scope fixes) to make
room for this branch's clause in a 700-char paragraph. It is restored
byte-identical to the merge base and this branch's own clause paid for it,
moving into the records-work paragraph as "session onboarding through probe 5/5".
Evidence is moved, never deleted -- least of all to fit a new claim.
Finally, AGENTS.md called the role "the FIRST question of the session" while the
marker has no TTL: it is once per WORKTREE, and a later session in a checkout
that ever claimed inherits the role silently. agent-role.py states that trade;
AGENTS.md now does too. And CLAIMABLE_ROLES' comments claimed "every 'may this
session write?' test keys on" it, which has no consumer outside agent-role.py
and its suite -- softened to what the constant actually guarantees.
FOLLOWING_AGENTS_PROTOCOL
Assisted-by: Claude Code:claude-opus-5 [ClaudeCode]
4 tasks
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
2 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.
Row
No row ID — protocol work. Subsystem A of two, per
.agents/specs/session-onboarding.md. The accepted design for subsystem B ships here as documentation only:.agents/specs/orchestration-harness.md.What changed
The rule that every session declares a role — operator, helper, or read-only — existed only as prose and an opt-in flag, and nothing triggered it. This makes it fire:
scripts/agent-onboard.py --probereports what a session has not resolved (role, mode,.env, the READY queue) and asks nothing;scripts/agent-role.pygainsread-onlyand--headless;scripts/agent-preflight.shdemands a role by default. The interview asks what the work is, not which role you want, and lands in.agents/workflow.mdwhere an agent reads it, withcheck-protocol-consistency.pyasserting it stays there.read-onlyis a declared absence of claim, not a third role — without it, a question-answering session must take the repo-wide operator lock or make a throwaway worktree, and faced with either people reach for the escape hatch until the gate means nothing.Evidence
All 12 commits pass
check-doc-checkpoint.py --commit <sha>individually — the per-commit spread that made #59 red does not exist here.scripts/agent-preflight.shpassestest_agent_role.py(38),test_agent_onboard.py(35),test_check_protocol_consistency.py(18)docs/STATUS.md,docs/BENCHMARKS.mdSpeed claims
Honest gaps
The role mechanism was broken and nobody knew.
agent-role.pyasserted the session id was "the parent process id … stable across tool calls within a session (measured)". It is not: every tool call gets a fresh shell, so a role claimed in one call was invisible in the next. Making the gate mandatory is what exposed it — under the opt-in default nothing depended on it. Roles now key on the worktree. The disproven claim is kept as an explicit retraction rather than deleted.The accepted cost of that fix: two sessions sharing one checkout now share a role. Helpers get their own worktree by construction, so this is almost always the operator's primary checkout, where one role is the right answer. Consequence worth knowing: the marker has no TTL, so a checkout that ever claimed carries that role into later sessions silently — run
agent-role.py showbefore assuming you are unclaimed.Enforcement is one point, not "every write path". Only
preflight --stagedrefuses aread-onlysession.git commit,git pushand thegate && git pushchain all proceed.AGENTS.md,workflow.mdand the spec say exactly this now — an earlier draft of all three claimed otherwise, and the final review caught it. Refusal on the other write paths is deferred and not built; the spec records both halves rather than leaving it between phases.Eleven times on this work a test passed with the thing it names deleted — including this branch's own gate default (an unrelated line satisfied the assertion) and
probe(), where five separate field hardcodes each left the suite green. All were found by reviewers mutating code and re-running; none by reading. Roughly half originated in the plan text. That evidence is what.agents/specs/orchestration-harness.mdis built on.Not run: the C++
ctestsuite. This branch changes zero C++/CMake files.🤖 Generated with Claude Code