Gate routing on a clarification phase so agents stop jumping the gun - #584
Open
QuanCheng-QC wants to merge 8 commits into
Open
Gate routing on a clarification phase so agents stop jumping the gun#584QuanCheng-QC wants to merge 8 commits into
QuanCheng-QC wants to merge 8 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Routing was a stateless per-message decision biased towards "somebody must
answer now": the router picks whoever's description best matches the topic,
and for a human message it is forbidden to answer "nobody" (there is even a
fallback that re-routes a `stop` verdict to the master). So a half-specified
request went straight to the agent that builds things, and it started
building while the requirement was still being clarified.
Add a per-channel phase — open (default, unchanged behaviour), clarifying,
building — that every orchestration mode passes through last:
• a gatekeeper target (phase owner, else the channel master) is untouched;
• a non-gatekeeper the sender explicitly @mentioned is kept but forced into
PLAN mode, so it can answer "is this feasible?" without implementing;
• any other non-gatekeeper target is dropped and the turn goes to the
owner — the case that produced the bug. When everything is dropped and
the owner is the sender, the turn ends rather than self-looping.
The gate is authoritative in the backend rather than advice in a prompt, so
it holds regardless of which mode the thread uses or what the router model
decides; the router prompt is told about the phase only to keep its verdicts
consistent with what the gate will allow.
Connector side, the routed message carries `phase`, `phase_owner` and
`target_modes`. BaseAdapter appends a role-specific directive (append, not
prepend, so channel auto-titling still sees the user's words) and sets a
per-channel mode override for exactly that message; the Claude adapter reads
it where it reads its own mode, so a gated wake-up genuinely runs with plan
permissions and read-only tools, not just an instruction it might ignore.
Mirrored in the Python adapter stack so the two ports don't diverge on this.
Release is the user's call: a "Requirement confirmed" button in the thread
header, plus a `workspace_set_phase` tool the owner can call once the user
confirms — never on its own judgement.
The gate shipped in a state where the two paths that matter most could silently do nothing. An ownerless gate was persistable, and it was the default case. Threads made from the agent picker deliberately have no master, so the one-click "Clarify first" stored phase='clarifying' with owner=None; _phase_gatekeepers then returned nothing and routing passed through untouched while the header said "Clarifying". A gate that looks enforced but isn't is worse than none. Phase and owner are now validated together before either is written: an unknown or removed owner is a 400, an owner not yet in the channel is joined to it rather than rejected, and clarifying without any resolvable owner is refused. The control sends both fields in one request instead of hoping the backend can derive one. Gatekeepers are also re-validated at routing time against live membership, because a write-time check does not survive the agent later being removed or leaving the channel. A gate nobody can hold now falls open with a warning rather than redirecting to a name no connector answers to — an unenforced gate is visible and recoverable, a stranded conversation is neither. Removal and channel-leave hand the gate to the master, or open the thread when there is no one left; picking an arbitrary survivor is not ownership. Fixing that repair path surfaced a pre-existing crash next to it: promoting the next master used scalar_one_or_none() over a multi-row query, so removing the master with two or more agents left raised MultipleResultsFound and aborted the whole removal. Python's Claude adapter accepted the per-message plan override into BaseAdapter and then ignored it, building its system prompt and permission flags from self._mode — a gated agent still launched with --dangerously-skip-permissions and Write/Edit/Bash. Both now read _mode_for(channel). The tests that were supposed to cover this only asserted mode bookkeeping, which cannot distinguish enforcement from a no-op. Both ports now assert on the argv the adapter would spawn; both suites were mutation-checked by reverting the override and confirming the new assertions fail.
… write paths Fail-open was the wrong call. When a clarifying channel has no gatekeeper left, returning the targets untouched wakes the builder with execute permissions — precisely the behaviour this feature exists to prevent, now reachable through an anomalous state rather than the normal one. Failing closed instead would strand the human with no reply. Neither is necessary: the per-target mode mechanism was already there, so an unenforceable gate now keeps whoever was targeted but downgrades every one of them to plan. The thread stays responsive and nobody builds against a requirement that is still officially unsettled. `phase` is stamped even without an owner so the woken agent is told why it is constrained. Channel creation checked only that the owner appeared in the caller-supplied participants list, which proves nothing — participants is untrusted payload. A create could therefore persist phase=clarifying with an owner that is not a workspace member at all, or one already removed. It now runs the same membership validation PATCH does. An agent that leaves a channel while being its master stayed recorded as master, so both master-mode and fallback routing kept aiming every later message at someone who had walked out. The gate repair sat directly on top of that, and reset the phase while leaving the stale master behind, so opening the gate handed the thread straight back to the departed agent. Leaving now clears the channel master first, then repairs the phase owner. The stale master is pre-existing on this path (verified against develop) but is repaired here because the phase repair depends on it; the test asserts the next human message actually reaches a live agent. The new Node argv test isolated the working directory but not the home directory, while the MCP command builder writes its config under os.homedir() — leaking files into the developer's home and failing outright where HOME is read-only. It now stubs os.homedir for the duration. The plan directive no longer claims the agent was @mentioned, since it now also covers being pulled in without one.
… a thread
DELETE /v1/workspaces/{id}/members/{name} hard-deleted the row instead of
going through network.agent.remove, so it skipped everything removal owes the
rest of the system: the status='removed' tombstone that stops a still-running
daemon from re-joining (issue #347), workspace and per-channel master
reassignment, and the clarification-gate repair added last round. Deleting an
owner through this endpoint left channels pointing at an agent that no longer
existed — the one path around the lifecycle fixes. It now emits the same
event as POST /v1/remove, keeping its own existence check so the documented
404 still holds.
Owner validation ran on every phase PATCH, including the ones leaving the
gate. A thread whose owner had been deleted therefore answered 400 to both
"Turn the gate off" and "Requirement confirmed": the only way out was to
appoint a new owner first, on a thread the user was trying to abandon. A
valid owner is now required only for a resulting clarifying phase, and an
explicitly supplied bad name is still an error; a stale inherited one is
cleared on the way out.
The control resolved the owner by string truthiness, so a stale name rendered
a confident "Clarifying · @ghost" while routing had already fallen back to
the master or to the plan-safe path where nobody holds the floor. Owner and
master are now resolved against the thread's actual agents. The control also
stayed hidden below two agents, which is precisely the degraded state a human
has to repair; it now shows whenever the thread is gated.
The gate existed but nothing turned it on, so the reported behaviour was still the default: create a thread from the picker, ask for something underspecified, and the router hands it to whichever agent matches the topic. Multi-agent threads now offer "Clarify requirements before execution", checked by default, with an owner picker. The owner preselects the workspace master when it is among the participants and otherwise demands an explicit choice — guessing would often land on the agent that builds things, and a gate owned by the builder is not a gate. The option only appears once two agents are selected, since a single-agent thread has nobody to hold the floor against. Phase and owner ride along with the channel.create event rather than a PATCH afterwards. A thread that exists ungated for even a moment can have its first message routed to a builder, which is precisely the window this is meant to close; a test asserts the opening message is already gated, with the builder listed first so an ungated fallback would have picked it. Unconditional default-on was the alternative and was rejected: picker-created threads have no master, so it would have had to invent an owner, and a requirement that is already settled would pay a confirmation round every time. Also: DELETE members now treats a tombstone as absent. Removal became a soft delete last round, so the second DELETE found the row left by the first, emitted another removal event and answered 200 — quietly dropping the 404 this endpoint returned back when it hard-deleted.
An explicit request for a gate could still produce an ungated thread. If the owner went stale between the client listing agents and channel.create running — removed, taken offline — creation quietly fell back to open, while the client painted the thread "clarifying" from its own request. The first message then routed to a builder in execute mode, unprotected, until discovery corrected the view seconds later. That is the exact failure this feature exists to remove, reintroduced through the anomalous path. An explicitly requested gate is now never dropped. When the owner cannot be resolved the channel is created gated but unowned: routing already degrades that to "everyone answers in plan mode", so the thread stays responsive and nothing can be built, and it renders as "Clarifying · needs an owner" until a human names one. Rejecting the event would have been the other way to avoid fail-open, but it throws away the thread the user just set up. The same reasoning applies to the repair path, which until now reset the phase to open when an owner was removed with no master to inherit it. Losing an agent is not the user deciding the requirement is settled. It now clears the owner and keeps the gate, so only a human ever removes one. channel.create returns the phase and owner it actually persisted, and the client renders from those instead of from what it sent. The dialog derived the gate from `selected`, which can name an agent that discovery has since reported offline, while it submitted participants filtered against the current online list — so it could offer a gate and then submit an owner that was not among the participants. Both now come from the same live set. UI copy softened from "cannot start building" to "kept in planning mode": outside the Claude adapters that constraint is prompt-level, and the wording promised more than every runtime enforces.
Two ways the gate could still lapse without anyone being told. A crashed daemon leaves status='online' in the database indefinitely, and gatekeeper validation only excluded 'removed'. An owner whose connector had died therefore kept the floor: routing dropped or redirected everyone else to it, the plan-only fallback never engaged because a "gatekeeper" existed, and the thread sat silent while looking perfectly healthy. Gatekeepers are now filtered by _member_is_online, the same liveness rule the rest of routing uses. phase_owner stays in the database — it records who owns the requirement, not who is connected this second — so ownership resumes by itself once the daemon heartbeats again, and until then the thread answers in plan mode instead of going quiet. Round two chose membership over liveness here on the grounds that silence beats wrong work. That trade no longer exists: an unowned gate is now plan-only rather than open, so there is nothing to gain by routing into a dead mailbox. The fixtures had modelled agents as status='online' with no heartbeat ever recorded, a state no real agent is in; they now carry one. In the new-thread dialog, the gate's visibility was derived from who is online rather than from what the user selected. A selected agent going offline mid-dialog made the whole option vanish, and Create — still enabled — produced a smaller, ungated thread with no indication anything had changed. Intent and applicability are now separate: the option stays visible from the selection, and when too few of those agents are still online the dialog says which ones dropped and blocks Create until the user picks someone else or unchecks the gate deliberately. Create is also blocked outright when every selected agent has gone offline, which used to produce an empty thread. Also corrects the _phase_gatekeepers docstring, which still described the old "gate inert / deliberate fail-open" behaviour — the exact opposite of what the code now does, and a trap for whoever edits it next.
develop localised the workspace into English and Simplified Chinese while
this branch was open, and typed zh-CN against the English catalogue so a
missing key is a build error rather than a blank string. The gate landed
before that and carried its own hardcoded English, which would have been
the only untranslated corner of the chat header and the new-thread dialog.
Move every user-facing string in the phase control and the dialog's
clarify block into the catalogues, and take the two other conventions the
rebuilt UI settled on: the Select component instead of a bare <select>,
and plural forms via the `{one, other}` objects rather than a ternary on
the count.
QuanCheng-QC
force-pushed
the
feature/clarify-phase-gate
branch
from
August 5, 2026 08:40
cb039a4 to
5d571b6
Compare
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
1 participant
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.
Gate routing on a clarification phase so agents stop jumping the gun
The problem, and where it came from
Reported from workspace usage: with no orchestration configured, agents jump
the gun — the requirement is still being clarified and the RD agent has
already started writing code.
It is not a bug in one place. Routing is a stateless per-message decision
biased towards "somebody must answer now":
and there is a fallback that re-routes a
stopverdict to the master);So a half-specified request goes straight to the agent that builds things. No
prompt can fix this reliably — the agent that gets woken has already won the
turn, and telling it "please don't build yet" is a request, not a constraint.
How to reproduce (on
develop)Three agents in one thread,
dynamicmode, no leader. Descriptions matter —the router picks on them:
pmdev-bedev-feThen send a deliberately underspecified request and answer only part of the
follow-up:
Requires
ROUTER_LLM_API_KEY(orANTHROPIC_API_KEY) — without it the LLMrouter is off and routing falls back to a deterministic mention/master path
where this symptom does not appear.
Before → after, same prompts, same three agents
Both runs were done manually against a local stack.
developdev-be/dev-fe, immediatelyConnector log from the gated run — four wake-ups during clarification, all of
them the phase owner, and the two dev connectors never received the messages
at all:
The captcha row is the point of the whole PR: the same requirement costs one
question up front, or two rework rounds after the fact.
What was added
A per-channel phase —
open(unchanged behaviour) /clarifying/building— applied last, after whichever orchestration mode has pickedits targets, so it constrains every mode uniformly:
clarifyingEnforced in the backend, not requested in a prompt. The router prompt is told
about the phase only so its verdicts stay consistent with what the gate will
allow, instead of being overridden after the fact every turn.
Being woken is not permission to build. The routed message carries
phase,phase_ownerandtarget_modes.BaseAdapterappends a role-specificdirective and sets a per-channel mode override for exactly that message; the
Claude adapter reads it wherever it reads its own mode, so a gated wake-up
genuinely launches with
--permission-mode planand a read-only tool set.Mirrored in the Python adapter stack.
Release is the user's call — a "Requirement confirmed" button in the thread
header, plus a
workspace_set_phasetool the owner may call after the userconfirms, never on its own judgement.
On by default for new multi-agent threads. The new-thread dialog offers
"Clarify requirements before execution", checked, with an owner picker. Phase
and owner ride along with
channel.createrather than a PATCH afterwards — athread that is ungated for even a moment can have its first message routed to a
builder. Existing threads are untouched (
phasedefaults toopen).Blast radius
A. Only affects threads that are actually gated
_apply_phase_gatereturns immediately unlessphase == 'clarifying';_phase_router_blockreturns""; the adapter directive is skipped when themessage metadata has no phase. A thread at
openroutes exactly as before.B. Affects every thread — this is where review time belongs
These are not phase-gate features; they are shared paths this PR had to change,
and they change behaviour for gated and ungated threads alike.
DELETE /v1/workspaces/{id}/members/{name}now emitsnetwork.agent.removeinstead of deleting the rowstatus='removed'tombstone, reassigns workspace + per-channel master, and a second DELETE now returns 404 instead of 200.scalars().first()instead of.scalar_one_or_none()MultipleResultsFoundand aborted the whole removalchannel.leavenow clearsmaster_agentwhen the master leaves_composeFinalResponsekeys offpp.spawnModeinstead ofthis._mode_dispatchMessagecalls_applyPhaseGateon every messageC. Schema
Migration
028adds two nullable-safe columns tochannels(
phase TEXT NOT NULL DEFAULT 'open',phase_owner TEXT NULL). Idempotent(guarded by an inspector check), reversible, no backfill, no data rewrite.
D. Not touched
Message delivery, event persistence, DM routing, the
master/workfloworchestration modes themselves (the gate wraps them, it does not modify them),
and single-agent threads.
If you only have 15 minutes
_apply_phase_gate()inworkspace_mod.py— the whole routing rule, ~60 lines.DELETE /members/{name}rewrite inrouters/workspaces.py— the largestbehaviour change for existing users, and the one least related to the feature.
_runMessage/_modeForinadapters/base.js— the per-channel override isset and cleared around exactly one message; a leak here would silently pin an
agent into plan mode.
phase+phase_ownervalidation block inupdate_channel()— this iswhere "never persist a gate nobody can hold" is enforced.
The invariant everything degrades to
Covered cases: an owner that was never valid; one removed from the workspace;
one that left the channel; one whose daemon crashed (liveness, not just
membership — a dead daemon leaves
status='online'forever); a stale ownerblocking the user from leaving the gate; both member-deletion endpoints; and a
dialog race where a selected agent goes offline mid-dialog.
How it was verified
test_llm_routerand
test_workspacesgreen (124 in the touched suites).tscclean.the override / removing the gate call makes those tests fail, restoring makes
them pass. Mode bookkeeping alone could not tell enforcement apart from a no-op.
summarised in the before/after table above.
develop's (environment:/tmp/openagents_filesownership, no local Postgres, pre-existinginit.sqldrift in
test_migration_schema).Known issues and follow-ups
owner announced "switched to building" but the channel ended up at
open.Both release the thread, so nothing broke, but
openmeans "this thread isnever gated again" and the header falls back to the "Clarify first" entry
point, which reads as though the gate had never been on. The
workspace_set_phasetool description offers all three values to the model;the enum exposed to agents should be narrowed to
clarifying/building,leaving
opento the human-only menu item. Not fixed in this PR.Python) receive the plan directive as prompt text with no read-only runtime
constraint. UI copy says "kept in planning mode" rather than "cannot build"
for exactly this reason. Follow-up order by usage: codex, opencode, cline.
building. The separate failuremode where agents echo "done / delivered / 👌" at each other after delivery
is unaffected by this PR and needs its own fix in the router's
stoplogic.workspace/frontendhas no test runner, so the dialog's default-checkedbehaviour is covered by type checking and the backend contract tests, not by
an automated UI test.
test_migration_schemaalready fails ondevelop— the insforge0001_initial_schema.sqlis stamped at 015 and 13 migrations behind. Nottouched here.