Repository navigation
feat(kernel,sdk,surface): f.human parks on wait.human; flows answer resumes it - #466
Conversation
…answer closes it
The authored body's f.human no longer throws unsupported_verb. It reads the
root journal for a wait.completed{human_responded} under its ordinal
(human-N); when none is recorded it throws AuthoredHumanParked, the durable
root turns that into the new step.wait verb, and the kernel journals
wait.human for the running attempt, releases the lease and parks the run
(needs_human, exit 3). event.emit keyed by the wait id closes a wait.human as
human_responded — the contract DESIGN.md already documented — and the root is
dispatched again; the resumed body finds the answer and lowers it as a
memoized human-N deterministic step so the boolean the author branches on is
journaled evidence. flows answer <run> <wait> yes|no [--note] records it.
Unknown runs now surface run_not_found from every verb instead of a journal
open failure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (34)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 4 potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| ...(recorded.note === undefined ? {} : { note: recorded.note }), | ||
| ...(recorded.answeredBy === undefined ? {} : { answeredBy: recorded.answeredBy }) }; | ||
| const literal = `'${JSON.stringify(record).replaceAll("'", "'\\''")}'`; | ||
| await lowerDeterministic(id, `printf '%s' ${literal}`, false); |
There was a problem hiding this comment.
🔴 Named gates silently pass human answers
When f.human(...).gate(config) resolves, lowerDeterministic omits the operation's namedGate. The child skips the declared check, so rejected answers can continue the flow.
Learn more
Every surface Step<T> accepts a named gate, and AuthoredFlowOperation stores that gate in namedGate. The run, llm, and agent implementations pass this value into their lowering paths. The human implementation constructs its operation inline, so this call cannot access the stored value and starts the deterministic answer child without verification. The kernel then journals a successful child regardless of the declared gate.
Example: await f.human('Ship?', {to: 'owner'}).gate({type: 'regex_match', pattern: '"answer":true'}) receives false. The answer child still succeeds and the await returns false; the declared gate never produces gate_failed.
Recommended fix: Bind the human AuthoredFlowOperation to a variable, as the run, llm, and agent implementations do, and pass humanOp.namedGate to lowerDeterministic. Add tests for both passing and failing named gates on f.human.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Real bug — fixed in 527c09f. human() now hoists its operation (humanOp, the same pattern as runOp/llmOp) and passes humanOp.namedGate into lowerDeterministic, so .gate(config) on f.human lowers into the human-N step's verification and becomes the human-N.gate child exactly like every other authored step.
Test: tests/authored-human.test.ts "lowers a named gate on the answer into the human-N step, so a rejected answer can fail it" — asserts the lowered spec carries human-1.gate with the embedded pattern, and that a rejected answer under that gate fails the step (step_failed). Before the fix the gate never reached the spec.
| const recorded = await readHumanAnswer(journal, rootRunId, id); | ||
| if (recorded === undefined) throw new AuthoredHumanParked({ waitId: id, question, to }, rootRunId); |
There was a problem hiding this comment.
🟡 Human questions never reach recipients
When f.human lacks an answer, it only raises AuthoredHumanParked; no path delivers the question to to. Unattended runs remain parked until someone independently discovers and answers them.
Learn more
The repository constitution requires a declared human ask to be delivered to the recipient's channels while the run parks. This implementation reads the durable answer and emits a park signal, while driveRoot only journals wait.human. The newly added surface documentation also states that to is merely recorded and the local kit does not deliver it. A user who is not watching the originating command receives no indication that their decision is required.
Example: A scheduled flow asks f.human('Publish?', {to: 'khaliq'}) with no terminal attached. The run durably parks, but Khaliq receives no Slack, WhatsApp, Telegram, or iMessage request. The flow cannot continue until someone separately inspects the run and executes flows answer.
Recommended fix: Add a durable delivery path for newly journaled human waits. Route the prompt, evidence, run ID, and one-tap answer action to the declared recipient, with retries and deduplication keyed by the wait ID. Do not expose f.human as complete until at least one supported channel fulfills the RFC delivery contract.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correct, and deliberate for this PR: to is recorded (wait.human.requested_of), not routed. This is stated in docs/SURFACE.md §5 Human gates: "to names who is asked and is recorded with the question; the local kit does not deliver it anywhere (that is the channel-delivery work in RFC covenant 3 and ops/BACKLOG.md)". The delivery surface today is the park report + flows answer locally, and on Cloud the run record + POST /api/v1/workflows/runs/<id>/answer (AgentWorkforce/cloud#3789), which is what a channel callback would hit.
Filed #468 for actual delivery (resolve to against declared tools, journaled send under the root so a resume does not re-send, one-tap answer → the Cloud route, and timeout), referencing RFC covenant 3. Leaving this PR to the durable wait/answer mechanics.
| } else if entry.entry_type == EntryType::WaitHuman { | ||
| let wait: WaitHumanPayload = serde_json::from_value(entry.payload.clone())?; | ||
| if wait.wait_id == event_key { | ||
| open.push(( | ||
| wait.wait_id, | ||
| entry.step_id.clone(), | ||
| entry.attempt, | ||
| WaitCompletionReason::HumanResponded, | ||
| )); |
There was a problem hiding this comment.
🟥 Human gates lack recipient authorization
Any daemon client can answer any discovered wait through event.emit; the kernel never checks requested_of. Unauthorized approvals can resume sensitive flows as valid human decisions.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
The daemon socket is the trust boundary for every verb, not just this one: a client that can event.emit can also run.start, run.cancel, step.complete on any run, or journal.read everything. The kernel does not authenticate clients; whatever reached the socket is trusted, which is the existing model for the local kit (same-user filesystem socket). Checking requested_of inside the kernel would be a check against a string the same untrusted client supplied, so it would add no authority.
On Cloud the authority check lives where identity exists: the answer route (AgentWorkforce/cloud#3789) requires session / cli:auth / workflow:invoke:write plus canAccessWorkflowRun, and refuses run-bound and sandbox tokens outright (human_answer_forbidden) so a run's own credentials — i.e. an agent inside it — cannot satisfy the gate. The sandbox then relays the authenticated identity via flows answer --by.
What I did tighten in the kernel (527c09f): event.emit now refuses a human response that lacks a non-empty answeredBy, so an unattributed answer can never close a wait.human; and the journal records the attribution as what it is (see the next thread). Kernel test step_wait_parks_the_attempt_and_a_human_answer_redispatches_it covers the refusal.
| if (typeof record !== 'object' || record === null || typeof record.answer !== 'boolean' | ||
| || (record.note !== undefined && typeof record.note !== 'string') | ||
| || (record.answeredBy !== undefined && typeof record.answeredBy !== 'string') | ||
| || (record.at !== undefined && typeof record.at !== 'string')) { |
There was a problem hiding this comment.
🟨 Approval audit fields are caller-controlled
Non-CLI clients can set arbitrary answeredBy and at strings in event.emit. The journal then presents forged approval attribution as recorded evidence.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Agreed both were client-asserted with nothing saying so. Smallest honest change (527c09f), on the kernel side where the journal is written:
at: the kernel now drops any client-suppliedatand stampsat_msin thewait.completed.resultfrom its own clock — the same value as the entry'sat_ms. The journal says when; the client never does. The SDK readsat_msback and renders ISO in the loweredhuman-Nrecord.answeredBy: kept under that name (it is whatflows answer --by/ Cloud's route send and what humans read), but the kernel addsattribution: "client_asserted"to the result so the journal states that the identity was asserted by whoever held the socket, not verified by the kernel. On Cloud that holder is the resumed sandbox relaying an authenticated caller; locally it is the OS user. An empty/missingansweredByis now refused.
I chose this over renaming to claimedBy because it keeps one field name across CLI, Cloud and journal, while making the epistemic status explicit in the record itself. Contract updated in authored-human.ts doc comment, SURFACE.md §5 and kernel/DESIGN.md event.emit row; kernel + SDK tests assert at absent, at_ms == entry clock, attribution present.
Cloud's answer route runs flows answer inside the resumed sandbox; the OS user there is the sandbox, not the person who decided. --by records who. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nd timestamps answers
- A `.gate(config)` on f.human is now lowered into the human-N step's
verification like every other authored step (it was dropped, so a declared
gate was never evaluated).
- event.emit refuses a human response without a non-empty answeredBy,
journals attribution: client_asserted (the socket authenticated the
caller, not the kernel), drops any client at and stamps at_ms from the
entry's own clock. The SDK contract follows: clients send
{ answer, note?, answeredBy }; the body reads at_ms/attribution back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review swarm: maintainabilityNo fresh transcript was produced for run |
Review swarm: historyNo fresh transcript was produced for run |
Review swarm: structureNo fresh transcript was produced for run |
Review swarm: FAILED
Cloud run: |
1 participant
Closes #400.
f.human(question, { to })used to throwunsupported_verb. It now parks the run durably and resumes with the answer.What happens
Lowering
step.wait{run_id, step_id, attempt, idempotency_key, wait_id, prompt, requested_of, options?, timeout_at_ms?}. Only the lease holder may call it (same check asstep.complete). It appendswait.humanfor the running attempt — nostep.completed, no semantic iteration charged — releases the lease, and drives: the step folds toNeedsHuman, the run parks. A reusedwait_idis refused.event.emitcloseswait.humanwhenevent_keyequals thewait_id, withcompletionReason: human_respondedandresult= payload. That is the contractkernel/DESIGN.md§5 already documented forevent.emit; it was not implemented. The step folds toRunnableand is dispatched as a fresh attempt.run_not_foundnow comes back from every verb for a run with no registry row and no journal file, instead of ajournal_write_failedopen error (a corrupt-but-present journal is still a journal failure).f.humanconsumes an ordinal (human-N) like every authored operation. It reads the root journal forwait.completed{human_responded}under that id. None → throwsAuthoredHumanParked; the durable root catches it, callsstep.waitwith the body's own wait id, and the signal propagates (exit 3,humanWaitin the JSON report). Answer present → the boolean is lowered as ahuman-Ndeterministic step carrying{"human","to","answer","note?","answeredBy?"}on stdout, memoized under its admission key, so the value the author branches on is journal evidence the IPC verifier holds to the same standard as every other step.f.humanis nowStep<boolean>on the surface (a thenable like every other verb; wasPromise<boolean>).waiton the error frame, validated) and the Bun parent, which holds the lease, issuesstep.wait.flows resumeon a still-open question reports it (exit 3) instead of waiting 30s for a dispatch that cannot come.flows answer <run> <wait> yes|no [--note] [--by <identity>]— checks the wait is open (refuses unknown / already answered / non-human-Nids ashuman_wait_unknown), thenevent.emits the answer contract{ answer: boolean, note?, answeredBy?, at }(answeredBy=--by, else the OS user; Cloud's resumed sandbox passes the Cloud caller). Attaches no worker; prints theflows resumethat continues.Answer contract
event.emit(runId, waitId, { answer: boolean, note?: string, answeredBy?: string, at: ISO }). This is whatflows answersends and what Cloud's resumed sandbox sends viaflows answer --by(companion: AgentWorkforce/cloud#3789 — answer route + resume applying the recorded answer). Anything else journaled as the result is refused on resume ashuman_answer_invalid.Live vs documented
tests/human-live.test.ts); Bun 1.4.0 standalone → Node body park/answer/resume (tests/authored-node-runtime.test.ts); kernelstep.wait/event.emitcycle (server/tests.rs).tois recorded, not routed — RFC covenant 3 /ops/BACKLOG.md);timeout(kernel never readstimeout_at_ms, DESIGN §1.4); answering from Cloud (AgentWorkforce/cloud#3789, which needs this release in the runtime pin first);f.dispatchstillunsupported_verb.answeredByrecords who. Cloud enforces caller identity on its route.Docs:
docs/SURFACE.md§5 Human gates,kernel/DESIGN.mdverb table,docs/EVENT-AWAIT.mdstale facts,examples/social-post-pipelinenow runs (and itsdone("canceled")→done("declined"), sincecanceledis a kernel outcome the body cannot declare).Tests
RELAYFLOWD_BIN+FLOWS_BUILD_BUN=bun 1.4.0), new:authored-human.test.ts(11),human-live.test.ts(3),cli-answer.test.ts(13), root + standalone cases.🤖 Generated with Claude Code
Note
High Risk
Changes kernel wait/event semantics, lease lifecycle, and run open errors, plus a new human-answer trust boundary on the daemon socket—mistakes could strand runs or accept bad answers.
Overview
f.humanis wired end-to-end instead ofunsupported_verb: authored flows park on the kernel'swait.human, record answers in the journal, and resume with a memoized boolean step (human-N).The kernel adds
step.waitso the lease holder can park an attempt without completing it (journalswait.human, releases the lease, run statusparked).event.emitnow also matches openwait.humanwhenevent_keyis thewait_id, requiringansweredBy, stampingat_msfrom the server, and journalingattribution: client_asserted. Missing runs surface asrun_not_foundinstead of a journal open error.The SDK/CLI adds
flows answer <run> <wait-id> yes|no(optional--note,--by), exit 3 reports withhumanWaitand answer/resume hints, andf.humanreturnsStep<boolean>on the surface. The durable root turnsAuthoredHumanParkedintostep.wait; after an answer, the body re-executes and lowers the decision as deterministic journal evidence.f.dispatchstays unsupported.Docs and
social-post-pipelineare updated to describe the live park/answer/resume loop;timeoutand channel delivery fortoremain future work.Reviewed by Cursor Bugbot for commit 527c09f. Bugbot is set up for automated code reviews on this repo. Configure here.