Repository navigation
CLI: surface needs_human from Cloud (--wait) and add flows answer --cloud (answer + resume in one command) - #624
agent-relay-code[bot] wants to merge 2 commits into
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Relayflow: the adversarial review did not pass. This branch is not approved: the flow stopped here and did not mark it ready to merge. PR #624 review — changes requestedReviewed head FindingsF1 — P1: Resume sends the stored attestation in place of submission authorityLocation:
Consequently the new command sends none of the required top-level submission fields. Under the existing submission contract, resume is rejected after the human answer has already been recorded. The new happy-path test asserts the incompatible object and accepts every request body, so it does not detect this mismatch. The review probe below captures the exact difference. Build the submission authority from the verified original bytes and persisted source authority, preserving its Surface identity without executing source. Alternatively, demonstrate and test a server contract that explicitly accepts the stored attestation for resume. Keep F2 — P2: An unknown answer response is accepted as a confirmed answerLocation: The acknowledgement guard only rejects non-objects, Validate an explicit acknowledgement contract (at least Coverage and limitationsThe focused suites exercise parked exit 3, displayed question/recipient, both command arities, source digest refusal, omitted inputs, retries, and watch/follow behavior. Their captured output and both type-check outputs are below. Two additional review probes fail on this head; they are review artifacts, not changes to the implementation or its judging gates. Reviewed the PR description, its recorded verification limitations, and all current comments. The issue-comment collection contains one CodeRabbit notice that review was skipped; the reviews and inline-comment collections are empty. Snapshots are in The original park may remain unchanged when Cloud creates a successor; the extra GET is not sufficient evidence against duplicate resume in that case. This is already disclosed in the PR, but needs a server contract before a stronger guarantee can be made. Reported synced trees and extensions intentionally refuse in this implementation, so that portion of the requested recovery path remains limited.
Captured verificationCommands below ran from npm run typechecknpm run typecheck:testsnpx vitest run tests/cloud-run.test.ts tests/cloud-answer.test.ts tests/cli-answer.test.ts tests/relay-cli-surface.test.ts tests/cloud-live.test.tsnpx vitest run tests/review-cloud-human.test.tsTo reproduce the review probes, from the repository root: cp evidence/cloud-human-review/review-cloud-human.test.ts.txt packages/sdk/tests/review-cloud-human.test.ts
cd packages/sdk
npx vitest run tests/review-cloud-human.test.tsThe temporary test was removed after capture; its exact source is retained in the evidence file above. Both assertions intentionally describe the required behavior and currently fail. PR check snapshot (not a claim of full CI success): gh pr checks 624 |
Cloud human gates now surface as
needs_humanwith exit 3, andflows answer --cloud <run-id> yes|norecords the decision and resumes with the original source/authority and noinputs.run --wait,status --watch, andlogs --follow. The waiter displays the scrubbed question, recipient and Cloud answer command; watch renders the park.--sourceaccepts original local/synced bytes without executing them. Reported synced trees and extensions refuse until their full restore contract is available.answerRecordedand a retry command; POSTs are never automatically retried.CloudRunStategains a distinctneeds_humanvariant; kernel completion vocabulary is unchanged. New SDK answer/receipt types are exported. Existing per-verb JSONokmeanings are preserved.<run-id> <wait-id-or-answer> [answer]in the shared command declaration. This avoids an optional middle positional. Existing local usage remains unchanged; command conformance and both arities are covered.Limits: no hosted run was executed. The answer-route shapes, full stored source availability, authority acceptance, workspace requirements, and successor-run behavior remain unconfirmed against production. The GET guard is not an atomic claim and cannot prevent a concurrent resume or detect a successor if Cloud leaves the original record parked; server-side deduplication is required for that guarantee. Unsupported response shapes fail closed. The CLI is the scriptable fallback for delivered in-channel answering.
Verification commands below ran from
packages/sdk. Dependencies were installed withnpm install --ignore-scripts; the full gate runs its build prerequisites itself. Complete captured outputs are committed underevidence/cloud-human-cli/.SDK types:
Configured test types:
Focused regressions:
Mutation verification of the reported waiter defect: saved the changed
cloud-run-record.tsbytes, replaced that file withgit show HEAD:packages/sdk/src/cloud-run-record.ts, ran the command below, restored the saved bytes with an equality assertion, then ran the same command again. Only the validator was reverted; the regression remained in place. Subsequent changes to that validator were comments only.npx vitest run tests/cloud-run.test.ts -t 'waiter returns needs_human instead of invalid_response'Old validator (exit 1):
Restored validator (exit 0):
Full gate: incomplete, not passing. Ran
npm test, then stopped Vitest withkill -INT 12018after it reported live-kernel assertion failures, an unavailable analyzer, missing bubblewrap, and missing local Surface build files. The process exited 130. No baseline comparison was run, so these are not asserted to be pre-existing failures. The focused Cloud suites and configured type checks above are the passing evidence for this change; this is not full-gate signoff.npm testCaptured full-gate output through interruption
Checks
The checks fail on the base commit too, so these failures were not introduced by this change: they come from the repository itself or from the environment the checks ran in. This pull request is a draft until someone looks.
What ran (.relayflow/check.sh)
Output on this branch (last 80 lines)
Output on the base commit (last 80 lines)
What the repair agent found
Repair notes — branch
relayflow/flows-software-garden-9cd92714.relayflow/check.shfailed for four reasons. Three are setup deltas betweenthis machine and CI, all fixed in
check.sh(not committed, per the task).The fourth is a sandbox capability this machine does not have; it is recorded
here and left alone. No code on this branch was at fault, and the
working tree is unchanged.
Where the whole check stands after the fixes (
sh .relayflow/check.sh, fulllog kept at
/tmp/check-final.log):All 24 failures and the one unhandled rejection are the bubblewrap cluster
below -- every failure line carries the same
bwrap/Hosted extension sandboxmessage. Before the fixes the same run reported 31 failures across 6 files;
cargo test --workspaceis green (33 test binaries,0 failedeach).Because
set -estopscheck.shat the failing vitest, the steps after it hadnever run. They were run by hand afterwards and all pass:
Fixed in
.relayflow/check.sh: sevenlive-kernel.test.tsfailuresThe extensionless wrapper fixtures in
testdata/preflightare ESM — each doesimport { receiveWrapperRequest } from './wrapper-session.mjs'. Node decides amodule's type from the nearest
package.jsonwalking up from the file. CIchecks out under
/home/runner/workwith no manifest above the repo, so Node'smodule-syntax detection applies and the fixtures load as ESM. Here the checkout
sits under
$HOME, and/home/daytona/package.jsondeclares"type": "commonjs"; the fixtures inherit it, detection is off, their bodynever runs, and they exit 0 having written nothing:
The worker then journals, with
output: null(verification.detailfrom thefailing
step.completed, captured off the live kernel):which is the seven failures in the first run: the two hn-monitor
analyze-storycases, the CliResult text fallback, the twoRELAYFLOW_WAKE_CONTEXTpins,RELAYFLOW_MODEL UNSET, and the tickSCHEDULED instantcase. Every one of them drives a fixture from thatdirectory; no repository code is involved.
check.shnow shadows the inherited"type"by writing a type-lesspackage.jsoninto the directory above the checkout(
/home/daytona/.relayflow-v2-supervisor/durable/package.json) when, and onlywhen, an ancestor declares
commonjsand that path is free. Node stops itslookup at the first manifest it finds, and one with no
"type"re-enablesdetection — CI's resolution exactly. It is outside the repository on purpose:
the tree stays clean and neither the repo nor CI sees a new root manifest.
With the shim, all 32 cases in that file pass (one skip is the pre-existing
LIVE_ANALYZER_UNAVAILABLEdiagnostic skip, which needs a realclaude):Also fixed in
.relayflow/check.sh:authored-node-runtime.test.tsTwo setup deltas, both hidden behind the first one.
bun version. Every CI workflow pins
oven-sh/setup-bun@v2tobun-version: 1.4.0, and that suite'sbeforeAllasserts the exact version(
packages/sdk/tests/authored-node-runtime.test.ts:18) because the standalonebuild it exercises is pinned to the compiler.
check.shonly installed bunwhen none was found, and this machine already had 1.3.6 on PATH, so the whole
file failed at
beforeAll:check.shnow installs and prefers bun 1.4.0 whenever the version on PATH isnot 1.4.0, matching the CI pin rather than mere presence.
Disk. With bun fixed, the suite got far enough to run, and five cases then
failed on a full filesystem:
This machine has a 10 GB overlay.
cargo test --workspaceleaves ~3.6 GB inkernel/target/debug, which leaves ~1.8 GB free -- not enough for a suite thatcompiles standalone bun binaries and stages a fixture tree per case. Nothing
after the kernel tests needs those artifacts (the SDK suite reaches the kernel
through the release binary named in
RELAYFLOWD_BIN), socheck.shnow dropskernel/target/debugoncecargo testhas finished. With ~5.3 GB free thefile passes whole:
Not fixable here: the bubblewrap-backed hosted-extension tests
24 cases across
tests/hosted-extension-isolation.test.ts(13),tests/hosted-extension-protocol.test.ts(8),tests/software-garden-babysitter-composition.test.ts(2) andtests/babysitter-native-extension.test.ts(1) fail with:/usr/bin/bwrapis installed (0.12.0,check.shinstalls it), but thiscontainer cannot create the namespaces
hosted-extension-sandbox.tsasks forwith
--unshare-all(packages/sdk/src/hosted-extension-sandbox.ts:371):CI's remedy —
sysctl kernel.apparmor_restrict_unprivileged_userns=0, whichcheck.shdocuments as left to the operator — cannot be applied: the key isread-only here even with passwordless
sudo.Bubblewrap's other mode for hosts without unprivileged user namespaces --
running setuid root -- is compiled out of this build, so that route is closed
too (the setuid bit was set only for this probe and immediately reverted):
So unprivileged user namespaces are unavailable to this sandbox and the real
isolation these tests exercise cannot run. Nothing on this branch touches the
sandbox, and the only ways to make the suite green would be to uninstall
bwrapso the tests self-skip, or to relax the sandbox's flags — hiding thetests or weakening what they pin. Left failing and reported instead.
Fixes #475
Summary by cubic
Surfaces Cloud human gates as
needs_humanwith exit 3 and adds a one-commandflows answer --cloudthat records the decision and resumes the run.Behavior
run --wait,status --watch, andlogs --followexit 3 on aneeds_humanpark, printing the scrubbed question, recipient, and answer command.flows answer --cloud <run-id> yes|nodiscovers the open wait, verifies source SHA-256, records the answer, and resumes with the original source/authority and noinputs; the resume POST is suppressed when Cloud already resumed.flows answersyntax is unchanged;CloudRunStategains a distinctneeds_humanvariant.Checks
Written for commit 43c97f4. Summary will update on new commits.