drive: cloud run 3fbcbcfb

[kjgbot]

Automated drive work from cloud run 3fbcbcfb-7216-4a77-9e9d-0951486d2957.

The sandbox cannot open PRs (no remote, no GitHub token), so this was delivered
from a host that can. Verification and adversarial review ran in-run — see
ops/reviews/ in the diff. A human merges.

[coderabbitai]

Warning

Review limit reached

Next included review available in 20 minutes.

View limit details

Limit details: You’ve used the included review currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 198e175e-71b8-4f73-a36e-e830d3070b1a

📥 Commits

Reviewing files that changed from the base of the PR and between 9681f11 and 04e122c.

📒 Files selected for processing (4)

• ops/NEXT.md
• sdk/src/index.ts
• sdk/src/worker.ts
• sdk/tests/live-kernel.test.ts

---

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/settings/billing.

_{Comment @coderabbitai help to get the list of available commands.}

[kjgbot]

maintainability lens — FAIL

Maintainability review — PR #56 (SDK agent worker)

Blockers

1. The CLI invocation contract is implicit and undocumented. worker.ts:112–139 spawns cli with an empty argv and pipes the instruction into stdin (child.stdin.end(${instruction}\n)). Nothing in the class docstring, AgentWorkerOptions, or AgentDispatchSpec tells a stranger that agent CLIs must read instructions from stdin. A future author writing a step with cli: /usr/bin/echo or a Node script that reads argv will get a silent hang with no clue what the contract is. RFC-0001 §3 makes the CLI declaration first-class — that means the calling convention has to be pinned somewhere the author will actually see it (docstring on AgentDispatchSpec plus a test that would fail if the invocation shape drifts).

2. Heartbeat failure never stops the child. worker.ts:66–75 starts a 5s setInterval heartbeat and hands failures to onError?.(...) — the CLI keeps running. If the lease expires (network glitch, daemon restart, missed heartbeat), the kernel is free to re-dispatch while the orphaned child is still working, then stepComplete on the dead attempt fails, and the second attempt performs the same effect. That directly contradicts the Appendix A rule 5 "exactly-once effects" invariant this worker is the reference implementation of. The heartbeat interval is also hardcoded (5s) with no relation to the lease_deadline_ms on the dispatch — no way to tell in six months why 5s or how to change it safely.

3. Default pins lie about the workspace revision. worker.ts:29–37 falls back to {workspace: [{surface: 'repo', revision_id: 'unversioned'}], streams: []} when the caller omits pins. Appendix A rule 2 requires each attempt to journal real revision ids — the string 'unversioned' gets written into started_pins/end_pins on stepComplete and becomes an audit-trail lie no reader can distinguish from a real pin. Better to require pins in the type (already Pins | undefined — flip it) than to synthesize a wrong one.

Concerns

• Stderr masquerading as stdout. runCli (worker.ts:127–138) resolves {stdout: stderrText} when the exit is nonzero and stderr is non-empty. The output field of step.complete then silently carries stderr under some conditions — a reader diagnosing a failure from the journal has no way to know which stream they are seeing. Return them as separate fields (as the on-disk version already does).
• DoD flow/de vendor wrapper e715601 #3 isn't tested here. ops/NEXT.md promises the "attach BEFORE run starts" contract is pinned in the live-kernel suite, but the new test (live-kernel.test.ts:205–241) only exercises the happy attach-before path — nothing would fail if a future change made attach-after silently work-or-break.
• Recovery mode ignored without comment. StepDispatchEvent.recovery (protocol.ts:151–156) can arrive as inspect/manual, and the worker just runs the CLI fresh. Fine for MVP, but a // TODO: honor dispatch.recovery — reset-only today comment would save the next reader from thinking it is handled.

Notes

• agentSpec (worker.ts:151–158) accepts empty-string cli/instruction — spawn will just fail, but a narrower guard is cheap.
• ops/NEXT.md's own DoD regressions: relaycast workspace-key repair answers an untyped 500 #6 requires the literal failing output of every new test to be quoted in the file — the section is missing.

REVIEW_FAILED

[kjgbot]

structure lens — FAIL

�[0m

build · deepseek-v4-pro
�[0m
�[0m→ �[0mRead docs/RFC-0001-everything-is-a-relayflow.md
�[0m→ �[0mRead sdk/src/worker.ts
�[0m→ �[0mRead sdk/src/protocol.ts
I reviewed AGENTS.md, RFC-0001 (§1 covenants, §4 language boundary, Appendix A starting-state contract), and the diff. Structure-lens findings:

Blockers

1. Silent default pins contradict the protocol and fail-closed (AGENTS.md rule 4). worker.ts:29-42 (start()) does this.options.pins ?? { workspace: [{ surface: 'repo', revision_id: 'unversioned' }], streams: [] }. The protocol explicitly documents the opposite at protocol.ts:132-135: "an agent worker with no pins is refused at attach rather than failing in the middle of a run." Fabricating a magic 'unversioned' revision is a silent fallback that invents a pin the worker doesn't actually hold — the exact "no silent fallbacks" shape AGENTS.md forbids and that Appendix A rule 2 ("pin on start… the relayfile revision id of every declared mount surface") is written to prevent. A caller who omits pins gets a real-but-false pin instead of a refusal. The worker should require pins, not default them.

Concerns

2. end_pins = started_pins always (worker.ts:118-133). complete() hardcodes both pins to dispatch.pins, asserting the agent changed no surface. For an agent step whose job is to edit the workspace (the test's "Produce the artifact"), this silently drops the agent's edits from the journal's pin chain, violating Appendix A rule 6 ("completion pins the end state… ending revision ids, stream offsets"). True only because the test's CLI reads stdin and prints; structurally the worker can never journal a modified workspace.

3. Reaching into spec.type instead of dispatch.step_type. agentSpec() (worker.ts:148-156) re-derives the step type from spec['type'] === 'agent' even though StepDispatchEvent.step_type already carries the kernel's closed vocabulary (protocol.ts:146). Filtering on the top-level field is the shape that matches RFC decision #13 (closed kernel vocabulary); introspecting spec couples the worker to the authoring spec's internals.

Notes

• agentSpec fails open: spec.cli typed string in the interface but runCli(cli, instruction) spawns cli verbatim — no shell/preflight. Out of scope per NEXT.md but worth watching.
• Coarse completionReason: every non-zero exit and every thrown error collapses to 'worker_error' (worker.ts:120-129); no 'timeout'/'verification_failed' distinction. Acceptable for this gate, but it flattens the RFC's typed-failure ladder.
• Coupling: socketPath-as-option constructs its own JournalClient rather than accepting an injected one (the working tree already moved to injection) — mildly harder to test.
• File size (158 lines) and single purpose are fine; no new kernel verb added (correct — helpers over primitives).

REVIEW_FAILED

[kjgbot]

🎯 review-swarm: FAILED (M:fail H:pass S:fail)

Lens transcripts posted as sibling comments above.

[kjgbot]

Auto-closed: conflicts with main + swarm review returned FAILED. Drive loop will produce a fresh attempt against current base.