feat(sdk): settle data and code gate contract - #139
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (38)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change defines a shared gate contract, freezes JSON-compatible inputs, bounds recursive JSON Schema validation, adds preflight gate reporting, and rejects invalid declarations before kernel storage or execution. ChangesGate validation and schema-bound verification
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Author
participant SDK
participant Preflight
participant Kernel
participant Journal
Author->>SDK: submit flow with gates
SDK->>Preflight: compile and inspect gates
Preflight-->>SDK: gate plan or invalid_spec
SDK->>Kernel: send validated specification
Kernel->>Kernel: validate JSON Schema declaration
Kernel->>Journal: create journal for valid run
Kernel-->>SDK: run result or invalid_spec
Poem
Warning Some tools did not complete. Review the errors below. 🔧 Clippy (1.97.1)Clippy execution failed Note 🎁 Summarized by CodeRabbit FreeYour 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 Essentials by visiting https://app.coderabbit.ai/settings/billing. Comment |
|
Repair evidence for exact pushed head The three reviewer-authored Red before implementation: direct lowering silently accepted callbacks and expressionsLiteral command: cd sdk
./node_modules/.bin/vitest run tests/gate-contract.test.ts --reporter=verboseCaptured output: Red before implementation: malformed JSON Schema passed both pre-run validatorsLiteral SDK command: cd sdk
./node_modules/.bin/vitest run tests/gate-contract.test.ts --reporter=verboseCaptured output: Literal kernel command: CARGO_HOME="$HOME/.cargo" RUSTUP_HOME="$HOME/.rustup" CARGO_TARGET_DIR="$HOME/.relayflows-toolchain/target/gate-contract" "$HOME/.cargo/bin/cargo" test --locked --manifest-path kernel/Cargo.toml -p relayflowd-core spec::tests::json_schema_declarations_match_the_shared_preflight_fixtures -- --exact --nocaptureCaptured output: Green boundary probeLiteral command: cd sdk
node --input-type=module <<'EOF'
import * as sdk from './dist/index.js';
const cases = {
expression: { type: 'expression', expression: 'length < 200' },
callback: (value) => value.length < 200,
};
console.log('classifyGate_exported=' + ('classifyGate' in sdk));
for (const [name, verification] of Object.entries(cases)) {
const candidate = { version: '0.1.0', steps: [{ id: name, type: 'deterministic', command: 'printf ok', verification }] };
for (const boundary of ['compileSpec', 'toKernelSpec']) {
try {
sdk[boundary](candidate);
console.log(`${boundary}_${name}=ACCEPTED`);
} catch (error) {
console.log(`${boundary}_${name}=REJECTED ${error.name}: ${error.message.replaceAll('\n', ' | ')}`);
}
}
}
EOFCaptured output: Green preflight refusal and no-journal/no-command regressionLiteral CLI command: cd sdk
set +e
node dist/cli.js check ../testdata/json-schema-invalid.flow.yaml
task_status=$?
set -e
printf 'check_exit=%s\n' "$task_status"
test "$task_status" -eq 2Captured output: Literal engine command: CARGO_HOME="$HOME/.cargo" RUSTUP_HOME="$HOME/.rustup" CARGO_TARGET_DIR="$HOME/.relayflows-toolchain/target/gate-contract" "$HOME/.cargo/bin/cargo" test --locked --manifest-path kernel/Cargo.toml -p relayflowd --test invalid_schema_preflight -- --nocaptureCaptured output: Exact-head full verificationLiteral SDK command: cd sdk
set -o pipefail
./node_modules/.bin/vitest run --exclude tests/live-kernel.test.ts --silent --reporter=dot 2>&1 | tail -n 12
task_status=${pipestatus[1]}
printf 'vitest_exit=%s\n' "$task_status"
exit "$task_status"Captured output: Literal typecheck command: cd sdk
./node_modules/.bin/tsc --noEmit
task_status=$?
printf 'tsc_exit=%s\n' "$task_status"
exit "$task_status"Captured output: Literal full Rust workspace command: set -o pipefail
CARGO_HOME="$HOME/.cargo" RUSTUP_HOME="$HOME/.rustup" CARGO_TARGET_DIR="$HOME/.relayflows-toolchain/target/gate-contract" "$HOME/.cargo/bin/cargo" test --locked --manifest-path kernel/Cargo.toml --workspace --quiet 2>&1 | tail -n 45
task_status=${pipestatus[1]}
printf 'cargo_exit=%s\n' "$task_status"
exit "$task_status"Captured output: The first default-timeout live run was not green and is not counted as a pass. Literal command: cd sdk
RELAYFLOWD_BIN="$HOME/.relayflows-toolchain/target/gate-contract/debug/relayflowd" ./node_modules/.bin/vitest run tests/live-kernel.test.ts --reporter=dotCaptured failure output: Both cases passed individually, and the full suite then passed with an explicit 60-second per-test ceiling. Literal full rerun command: cd sdk
RELAYFLOWD_BIN="$HOME/.relayflows-toolchain/target/gate-contract/debug/relayflowd" ./node_modules/.bin/vitest run tests/live-kernel.test.ts --reporter=dot --testTimeout=60000Captured output: Literal diff command: git diff --check HEAD~1..HEAD
task_status=$?
printf 'diff_check_exit=%s\n' "$task_status"
exit "$task_status"Captured output: |
|
Follow-up artifact repair at The first pushed repair was not DONE because the hosted exact-artifact smoke failed. Literal inspection command: gh run view 33662542959 --job 100356314891 --log-failedCaptured failing output: The dynamic Literal local standalone-artifact command (same Bun 1.4.0 compiler, host target substituted for Linux): cd sdk
./node_modules/.bin/tsc --noEmit
task_tmp_dir=$(mktemp -d)
bun build src/cli-executable.ts --compile --target=bun-darwin-arm64 --outfile="$task_tmp_dir/flows"
"$task_tmp_dir/flows" check --json ../testdata/hello-deterministic.flow.yaml
printf 'artifact_smoke_dir=%s\n' "$task_tmp_dir"Captured output: Hosted Linux check is pending on |
|
Hosted exact-head artifact is green after the static metaschema import repair. Literal command: gh pr checks 139 --watch --interval 10Final captured output: |
|
Repair evidence for exact head The repair closes four public-boundary holes without adding a gate language: Red before implementationFocused greenThe schema behavior test also asserts Full SDK and live protocolFull kernelBuilt and packed public boundaries
|
|
Evidence correction: the two angle-bracket labels in my preceding comment abbreviated inline scripts, so those two command lines were not literal. The literal captured commands and outputs were: |
|
Exact-head hosted artifact gate completed after the repair push: Run: https://github.com/AgentWorkforce/flows/actions/runs/33669268034 |
|
Fresh Product changes:
Literal red evidence before product changesLiteral focused green and typecheckLiteral full SDK, kernel, and live protocol resultsLiteral real-socket resultThe focused Rust test above is itself a spawned-daemon Unix-socket integration test. An independent SDK Literal installed-package and Bun runtime resultsThe installed consumer was created from
|
|
Exact-head hosted artifact result: Run: https://github.com/AgentWorkforce/flows/actions/runs/33673407803 |
|
Hostile-Proxy boundary repair evidence for exact head The exported unknown-input boundaries now snapshot/reject before reflection:
RED at Built-dist behavior before the repair: Focused GREEN: Final-tree public boundary rerun: Full SDK: Typecheck/build: Node built-dist GREEN: Bun built-dist GREEN: Packed-package Node + Bun GREEN ( Standalone Bun artifact: Real-socket no-effect proof: The test launches the real Full Rust workspace: Live SDK/kernel: Pre-push race check and push: Reviewer-owned reports under |
|
Hosted exact-head follow-up: Hosted PR remains open; no merge performed. |
kjgbot
left a comment
There was a problem hiding this comment.
SIGNOFF A — FINDINGS (not an approval)
I reviewed exact head f5b8437b41e32d7ba45bb96eecf9bf8eb2aee65a against merge base a0d42ffbdc7fb60b42c0b5bea4f58408249b08a2.
P1 / F1: the claimed behavior-free SDK data boundary is incomplete. At that reviewed head, exported validateSpec accepts a valid Proxy after three ownKeys/descriptor traps and returns {ok:true}; exported kernelToAuthoring likewise executes three traps and returns normalized output. compileSpec correctly rejects the same Proxy at zero traps, which isolates the bypass.
File evidence: sdk/src/validate.ts:81-92 reflects input through Object.keys; sdk/src/compile.ts:187-324 has public kernelToAuthoring / requireKernelObject / assertKernelKeys reflecting an unknown; sdk/src/index.ts:31-40 exports both boundaries. Reproduced in Node, Bun, and a packed-and-installed @relayflows/sdk tarball.
Required repair: snapshot/reject before any reflection in both exported unknown-input boundaries (or make the reverse converter non-public and parsed-JSON-only), with Node, Bun, and packed-install regression coverage.
The PR now reports head e6210a2fc666df6dc8c777c009712ddf99efa877, which I have not reviewed; this verdict is pinned only to f5b8437b41e32d7ba45bb96eecf9bf8eb2aee65a and needs re-signoff after a fix. Full literal evidence is committed in ops/reviews/20260902-2140-pr139-signoff-structure.md.
7702a2f to
0ea58b5
Compare
Supersedes an unmerged first pass of this report that targeted 990093b. Every command is pinned to a literal SHA rather than the origin/main ref, which moved twice during the task. This rebase produced ZERO conflicts, which is the risk rather than the result: on #139's rebase a line that reverted a lowering auto-merged silently. Every hunk was therefore audited by reading. 512723c adds #138, which touches four files this branch also edits (spec.rs, spec/tests.rs, validate.ts, validate.test.ts) and, critically, moves timeoutMs to deterministic-only in TWO independent places: the step-fields allowlist and compileStep's base spread. Getting one right and missing the other yields a spec that validates but lowers wrong, and validateSpec cannot see it. Both halves are byte-identical to 512723c and both were re-proved behaviourally through compileYaml + toKernelSpec: a deterministic step lowers to timeout_ms, llm and agent are refused at the allowlist. #136's `output` line survives in both verb lists. Artifact survival, both directions. All 15 of #138's blobs hashed before and after: 11 identical including compile.ts and step-fields.ts; the 4 that moved are the 4 this branch edits and each is a pure addition. Every line of #138 content absent afterwards was enumerated: a first pass with plain diff reported 14, of which 7 were false positives from re-indentation and one rustfmt attribute rewrap; whitespace-insensitively 7 remain, all attributed and none authored by #138. In the other direction, a whole-tree set-diff of the branch's own change set before against after reports exactly three deltas across 41 files, the same three deliberate resolutions as the first pass and nothing else. The branch's own gate is proved where it lives rather than where it is convenient: a canonical spec compiled through the SDK, its lowered kernel spec then mutated and submitted over a real socket with the SDK out of the path. The kernel refuses all five non-canonical forms across both surface kinds, and accepts the canonical control. Gates: tsc --noEmit, tsc -p tsconfig.type-tests.json (a gate #138 added that the brief's list predates), and tsc -p tsconfig.tests.json all pass; cargo test --workspace is 130 passed, 0 failed; vitest is 410 passed with one failure, the pre-existing wire-conformance one. Rust test names set-difference to exactly the union of both parents, 130 executed against 130 expected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
105411c to
40edd03
Compare
* kernel: dispatch runnable steps in parallel Session-Id: 01a062cc-f525-7d01-932e-a634815114c1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): drive complete parallel dispatch batches Session-Id: 01a062cc-f525-7d01-932e-a634815114c1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): preserve parallel assignment lifecycle Session-Id: 01a062cc-f525-7d01-932e-a634815114c1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): close parallel dispatch admission gaps Session-Id: 01a062cc-f525-7d01-932e-a634815114c1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): reject forged completion pins Session-Id: 01a062cc-f525-7d01-932e-a634815114c1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): canonicalize workspace surfaces across kernel/SDK/socket Second P1 in PR #137 review: `/mount/repo` and `/mount/./repo` were admitted concurrently. `parallel::SurfaceIdentity::Opaque("workspace:...")` compared the raw string; workspace surfaces bypassed the canonical path identity used for external surfaces, letting alias forms conflict-check as disjoint. Repair: - `SurfaceIdentity::External` → `SurfaceIdentity::Path{kind, namespace, components}` with `PathSurfaceKind::{Workspace,External}`. Workspace and external surfaces now share the canonical path identity but do not cross-collide. - Rename `spec::external_surface_identity` → `spec::path_surface_identity`; callers of the workspace surface use the same canonicalizer. - SDK `isCanonicalExternalSurface` → `isCanonicalPathSurface`; workspace entries now reject empty/./.. components with the same error class as external surfaces. - New `kernel/relayflowd/tests/crash_resume/workspace_identity.rs` covers alias refusal and canonical subtree serialization over a real socket. - 33 new SDK validate cases pin workspace canonicalization; 9 kernel parallel_tests cases (ancestor/descendant/sibling for both surface kinds) still green. Evidence: $ cargo test -p relayflowd-core --lib machine::parallel test result: ok. 9 passed; 0 failed; ... $ cargo test -p relayflowd --test crash_resume workspace_aliases test result: ok. 1 passed; 0 failed; ... finished in 0.89s $ ./node_modules/.bin/vitest run tests/validate.test.ts Test Files 1 passed (1) Tests 48 passed (48) Session-Id: 6cae47a0-1263-4c8b-bfaa-bd5ffc72e08e Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): preserve terminal-slash surface compatibility Session-Id: 01a0667b-bd7e-73c1-8e14-e3e9d13d136e Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * fix(kernel): one spelling per surface, refusing the terminal slash Reverts 83db98b's accept-and-normalize and restores 53bfee0's strict rule for BOTH workspace and external surfaces. An independent signoff at 83db98b found a P0: exactly-once effects can double-fire. 83db98b widened the *external* accept set the same way it widened workspace -- its own test diff moved "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/provider/item/" from reject to accept -- but added `workspace_surfaces_equal` only. The exactly-once ledger key is a raw SQL string: PRIMARY KEY (step_id, idempotency_key, surface_path) relayflowd-journal/src/lib.rs:48, append.rs:162 `idempotency_key = sha256(run_id || step_id)` (machine.rs:396) and `step_id` are both constant across attempts, so `surface_path` is the only variable in that key -- and it had two legal spellings. Executed against the real SqliteJournal at 83db98b: attempt1 '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/provider/item' deduped = false attempt2 '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/provider/item/' deduped = false effect_count = 2 ; confirmed_effect_count = 1 `deduped=false` means "you owe the provider call" (engine/effects.rs:17-24), so one logical effect fires twice. At the parent commit it failed closed at effects.rs:127. Accept-and-normalize only holds if EVERY identity comparison routes through the same normalization. 83db98b reached fifteen workspace comparison sites and got all fifteen right; it missed the sixteenth, which happens to be the one guarding exactly-once. Uniform reject needs no such completeness: one surface has exactly one spelling, and a non-canonical one never enters the system. Two facts make the strict rule the house rule rather than a new constraint: testdata/hello-agent.flow.yaml already authored `surface: repo`, so the ladder fixture was the outlier; and 53bfee0's own contract test `workspace_mounts_and_worktrees_must_have_one_canonical_spelling` already asserted `/mount/repo/` is refused, which accept-and-normalize contradicted. RED (before this change, with the tests restored to the strict contract): $ cargo test -p relayflowd-core --lib spec::tests::external_surface ---- spec::tests::external_surface_paths_must_have_one_canonical_spelling stdout ---- panicked at relayflowd-core/src/spec/tests.rs:146:9: accepted non-canonical surface "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/provider/item/" test result: FAILED. 0 passed; 1 failed GREEN: $ cargo test --workspace 22 + 31 + 1 + 1 + 4 + 3 + 37 + 5 + 18 passed; 0 failed $ ./node_modules/.bin/vitest run 259 passed (260); 1 pre-existing live-kernel failure, unchanged Fixtures: testdata/hello-ladder.flow.yaml authors `surface: repo`, and its canonical JSON and sha256 are regenerated through the SDK compiler rather than hand-edited. The canonical diff is one character; the hash moves ecccd7b2..de095a29 -> 57cac294..f6d57944, and spec_parity confirms kernel and SDK still agree byte-for-byte. `workspace_surfaces_equal` is kept across its 18 call sites. Under a single spelling it is equivalent to string equality for valid surfaces, but it still compares parsed identities and so fails closed when either side does not parse -- defense in depth at the pin/declaration seam for exactly the bug class above. Removing it would be an 18-site change for no safety gain. A non-canonical spelling is no longer a surface at all, so `external_surface_contains("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/provider/item/", "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/provider/item")` now fails closed rather than resolving to the canonical form, and the two ancestor/descendant conflict cases that exercised the terminal slash are dropped as unreachable states rather than restated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * test(sdk): a late completion after cancel reports run_terminal, not lease_conflict This edits a test that judges this branch's own behaviour, which AGENTS.md rails against. It is therefore its own commit, touching nothing else, and the taxonomy call was made by the lead (relayflow-lead-0903), not by this branch. Flagging it for the independent signoff to re-derive rather than inherit. The test is #142's own — "cancels over the real socket and rejects the lease holder after closure", added by main in feat(kernel): add durable run cancellation. It cancels a run over the socket, then has the lease holder complete the step, and asserted the refusal carried `lease_conflict`. What this branch changed is which of two refusals fires first, not whether the completion is refused. `step.complete` now runs the `ensure_mutable` admission gate before `completion_worker`: ensure_mutable(&engine, ¶ms.run_id)?; // -> run_terminal let worker_id = hub .completion_worker(connection_id, &key) .map_err(protocol_conflict)?; // -> lease_conflict Each code has exactly one producer in the tree (server/protocol.rs:58 and :46), so the ordering fully determines which is returned. Unchanged by this commit, and still asserted by the same test: the completion is refused, exactly one run.cancel.requested entry exists, and exactly one run.completed entry exists carrying completionReason "canceled". Only the error code moved. The lead's reasoning for preferring run_terminal: lease_conflict tells a worker "someone else holds your lease", which is false here -- nobody holds it, the run is over -- and it invites a retry that terminality does not. Checking "can this run accept mutations at all?" before "who holds this lease?" is also the correct precedence: the cheaper, more general, fail-closed question first. The rejected alternative was weakening ensure_mutable so lease_conflict still won. That trades a correct guard for a stale expectation. Mutation-verified, both directions, on the rebased tree at 512723c. RED (before this commit): $ ./node_modules/.bin/vitest run FAIL tests/live-kernel.test.ts > ... > cancels over the real socket and rejects the lease holder after closure AssertionError: expected JournalProtocolError: run_terminal: run 0... { code: '...' } to match object { code: 'lease_conflict' } - Object { - "code": "lease_conflict", + JournalProtocolError { + "code": "run_terminal", Tests 2 failed | 409 passed | 3 skipped (414) GREEN (after): $ ./node_modules/.bin/vitest run Tests 1 failed | 410 passed | 3 skipped (414) The one remaining failure is the pre-existing `JournalClient wire conformance` failure, which is independent: it survives moving ensure_mutable after completion_worker, whereas this test does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * docs(reviews): record the PR #137 rebase onto 512723c Supersedes an unmerged first pass of this report that targeted 990093b. Every command is pinned to a literal SHA rather than the origin/main ref, which moved twice during the task. This rebase produced ZERO conflicts, which is the risk rather than the result: on #139's rebase a line that reverted a lowering auto-merged silently. Every hunk was therefore audited by reading. 512723c adds #138, which touches four files this branch also edits (spec.rs, spec/tests.rs, validate.ts, validate.test.ts) and, critically, moves timeoutMs to deterministic-only in TWO independent places: the step-fields allowlist and compileStep's base spread. Getting one right and missing the other yields a spec that validates but lowers wrong, and validateSpec cannot see it. Both halves are byte-identical to 512723c and both were re-proved behaviourally through compileYaml + toKernelSpec: a deterministic step lowers to timeout_ms, llm and agent are refused at the allowlist. #136's `output` line survives in both verb lists. Artifact survival, both directions. All 15 of #138's blobs hashed before and after: 11 identical including compile.ts and step-fields.ts; the 4 that moved are the 4 this branch edits and each is a pure addition. Every line of #138 content absent afterwards was enumerated: a first pass with plain diff reported 14, of which 7 were false positives from re-indentation and one rustfmt attribute rewrap; whitespace-insensitively 7 remain, all attributed and none authored by #138. In the other direction, a whole-tree set-diff of the branch's own change set before against after reports exactly three deltas across 41 files, the same three deliberate resolutions as the first pass and nothing else. The branch's own gate is proved where it lives rather than where it is convenient: a canonical spec compiled through the SDK, its lowered kernel spec then mutated and submitted over a real socket with the SDK out of the path. The kernel refuses all five non-canonical forms across both surface kinds, and accepts the canonical control. Gates: tsc --noEmit, tsc -p tsconfig.type-tests.json (a gate #138 added that the brief's list predates), and tsc -p tsconfig.tests.json all pass; cargo test --workspace is 130 passed, 0 failed; vitest is 410 passed with one failure, the pre-existing wire-conformance one. Rust test names set-difference to exactly the union of both parents, 130 executed against 130 expected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * docs(reviews): correct the wire-conformance attribution — it is this branch's An earlier version of this report called the vitest wire-conformance failure "pre-existing". That was wrong. It was observed at 83db98b, which is #137's own head and already carries bdd598c; "pre-existing at the branch parent" is not "pre-existing on main", and I repeated the stronger claim without testing it. Determined empirically instead. A throwaway worktree at 512723c with its own kernel build and RELAYFLOWD_BIN pinned to it runs tests/live-kernel.test.ts at 21 passed (21), wire conformance included. The same file fails on this branch. The failure is the branch's. It is also not an error-code taxonomy change, which is why no assertion was touched. bdd598c gates six verbs with ensure_mutable, two of them event.emit and stream.append. The wire-conformance test starts a deterministic run, which completes immediately, then calls both on it and asserts they SUCCEED (matched === 0, offset === 0). Main accepts them; this branch refuses them. The observable protocol behaviour changed from accepted to refused. The refusal is load-bearing rather than gratuitous, and the obvious fix is the wrong one. On main, stream.append against a terminal run writes a stream.appended entry after run.completed. The branch's state.rs fold guard rejects exactly that, so a journal main produces is one this branch cannot load: the same data directory resumes clean under the main binary and fails under the branch binary with "journal entry 5 appears after terminal run.completed". Relaxing ensure_mutable to turn the test green would let the daemon write journals it then cannot resume, which is a durability bug and worse than a red test. The rebase also surfaced a direct contradiction: the branch's own protocol_admission test asserts stream.append and event.emit return run_terminal and leave the journal unchanged, while main's wire-conformance test asserts they succeed. Both are in the tree, both were written deliberately, and they cannot both pass. Because protocol_admission pins the error code, an accept-and-ignore compromise would require editing that gate too. Three options are laid out in the report. None is implemented; the branch is unchanged and the push is on hold pending the lead's call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * test(sdk): a terminal run refuses event.emit and stream.append PROTOCOL CHANGE. This edits main's test to match a deliberate behaviour change in this branch, so it is its own commit, touching nothing else. The call was the lead's (relayflow-lead-0903), not this branch's. The signoff should re-derive it rather than inherit it. This is main's test -- "JournalClient wire conformance against live relayflowd". It started a deterministic run, which completes immediately, then called event.emit and stream.append on that terminal run and asserted both SUCCEED: expect((await client.eventEmit(run_id, 'unmatched', {ok:true})).matched).toBe(0); expect((await client.streamAppend(run_id, 'results', {answer:4})).offset).toBe(0); bdd598c admits every mutating verb through `ensure_mutable`, so this branch refuses both with run_terminal. Unlike the step.complete change in 269fcc6, this is not an error-code taxonomy move: observable behaviour on a shipped verb pair goes from accepted to refused. It does not remove a working capability. It removes a way to corrupt a journal that main reports as success. On main, stream.append against a terminal run journals stream.appended AFTER run.completed, and this branch's state.rs fold guard rejects exactly that -- so main produces journals the daemon cannot fold on resume. Same data directory, both binaries: MAIN resume -> exit=0 {"status":"completed","completion_reason":"success"} BRANCH resume -> exit=1 Error: fold run journal Caused by: journal entry 5 appears after terminal run.completed The realistic shape is worse than that synthetic one, and shows main is already self-inconsistent. A worker holds an llm lease; the run is cancelled out from under it; the worker then does what a live worker does: late step.complete -> THROW [lease_conflict] <- main already refuses this late stream.append -> OK {"offset":0} <- and corrupts the journal late event.emit -> OK {"matched":0} >>> entries AFTER terminal run.completed: ["stream.appended"] Main already holds "a terminal run accepts no step completions" (engine/remote.rs:44, from #142). This branch extends the same rule to the other mutating verbs, which is what the words already meant. Checked before changing anything, rather than assuming: no product code calls either verb (only the JournalClient method definitions); no doc sanctions a post-terminal append -- kernel/DESIGN.md:384-385 says event.emit "satisfies wait.event", which a terminal run has none of, and stream.append "journals stream.appended", which is the corruption; and the one in-repo live use (crash_resume/llm.rs) is against a parked run and is unaffected. The test keeps full wire coverage of both verbs. Their success paths move to the parked llm run, which is the only state in which appending to a run's journal is meaningful, and the terminal case now asserts the refusal -- matching crash_resume/protocol_admission.rs, which is left alone because it pins the invariant correctly. $ ./node_modules/.bin/tsc -p tsconfig.tests.json (clean) $ RELAYFLOWD_BIN=<this worktree's own build> ./node_modules/.bin/vitest run Test Files 23 passed | 1 skipped (24) Tests 411 passed | 3 skipped (414) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * docs(reviews): record the option-1 resolution and the assumption test behind it The lead chose option 1 and asked that the assumption behind it be tested rather than inherited: is there any legitimate late stream.append or event.emit on a terminal run, out-of-band async completion in particular. Checked four ways and the assumption holds. No product code calls either verb — only the JournalClient definitions, a loopback mock, and tests. No doc sanctions a post-terminal append: DESIGN.md says event.emit satisfies a wait, which a terminal run has none of, and stream.append journals stream.appended, which is the corruption itself. The one live in-repo use appends against a parked run and is unaffected. The async shape was executed rather than reasoned about, and it cuts against main: a worker holding an llm lease on a run cancelled out from under it has its late step.complete refused with lease_conflict while its late stream.append is accepted in the same moment, journalling after run.completed and producing a journal the branch cannot fold. Main already holds the rule for step completions at engine/remote.rs:44; this branch extends it to the remaining mutating verbs. Adds the PR-body Protocol change section, and records a CI finding: the two contradicting tests both passed because neither runs in flows CI. cargo test never runs at all, and CI's vitest names four files, of which live-kernel.test.ts is not one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 * docs(reviews): lead the Protocol change section with main's self-inconsistency The strongest argument for the change is not the resume demonstration, it is that main already holds the rule and fails to apply it uniformly: a worker whose run was cancelled has its late step.complete refused with lease_conflict and its late stream.append accepted, in the same moment. This PR extends main's own rule rather than imposing a new one. Reordered so a reviewer meets that first, with the resume corruption as the reason it matters and DESIGN.md:384-385 as corroboration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 --------- Co-authored-by: kjgbot <kjgbot@agentrelay.dev>
40edd03 to
8b7148d
Compare
Rebased onto
|
Correction to my previous commentI wrote that "the codex agent assigned the rebase never pushed" and that the branch "sat unmoved for ~12h." That framing is wrong and I want it corrected in the record.
So the duplicated work is mine, not its. Its independently-produced result matches mine, which is a useful cross-check on both:
It also reached the same conclusion on the conflict I hit: server-side error mapping belongs in #137's extracted The one substantive difference is the base: it pinned Its full evidence is at It also independently disclosed the intermittent capacity failure — see #155. |
…gnoff commissioned Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
|
VERDICT: PASSED Independent signoff: PR #139 at
|
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
8b7148d to
1ed9023
Compare
CI at
|
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Correcting my previous comment: the hang is NOT specific to this branchI said the #158 — which is So "only this branch" was wrong. What the four runs actually show:
That is a suggestive split but it is n=2 on each side, and it is equally consistent with a Linux-only flake that the two kernel PRs happened to hit. I have re-run #158's job to tell those apart; if it hangs a second time the split is real, if it passes the hang is a flake and this PR needs no work for it. Either way the correction stands on its own: do not treat the hang as evidence against this PR's gate/termination changes until the re-run says something. I reached for the most specific explanation available and stated it more confidently than two data points support. The |
Session-Id: 01a062df-0cdf-7f23-89dd-121aa9ecf743 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: 01a062df-0cdf-7f23-89dd-121aa9ecf743 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: 01a062df-0cdf-7f23-89dd-121aa9ecf743 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: 01a062df-0cdf-7f23-89dd-121aa9ecf743 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: 01a062df-0cdf-7f23-89dd-121aa9ecf743 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: 01a062df-0cdf-7f23-89dd-121aa9ecf743 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
A `json_schema` gate whose `$ref` graph cycles through only in-place
applicators compiles cleanly and then recurses without bound the first time
it validates an output. In Rust that aborts the process, so `run.start`
accepted the spec, created the journal, ran the step's command, and then took
relayflowd down with SIGABRT — leaving a run stuck `running` that re-executed
its effect on every resume (4 executions of one logical step, no
`completionReason`, no `step.completed`). RFC-0001 covenant 2 and gate 1.
A stack overflow cannot be caught, so the bound is structural and runs before
the declaration is accepted: reject a reference cycle that re-applies to the
same instance and therefore makes no progress. Cycles through a child
applicator (`properties`, `items`, `prefixItems`, ...) consume one level of
the instance per step and stay legal, so ordinary recursive schemas are
unaffected.
`kernel/relayflowd-core/src/schema.rs` and `sdk/src/json-schema-bound.ts`
implement the same rule and are pinned to a shared corpus in
`testdata/json-schema-bound-cases.json`, so the kernel and the SDK agree on
which schemas are legal by construction rather than by coincidence of Ajv's
catchable RangeError and Rust's uncatchable abort. That also closes the
reported SDK/kernel divergence on a self-recursive `$defs`. Every corpus
refusal compiles cleanly in `jsonschema`, which is what makes the tests test
the bound and not the mechanism. `verify` now compiles through the same gate.
Also from the same review:
- `canonicalize`/`specHash` are exported unknown-input helpers, so they carry
the snapshot guard the rest of the exported surface already has, and each
key is read exactly once instead of twice (a demonstrated getter TOCTOU).
- A `json_schema` gate that accepts every output (`{}`, `true`, annotations
only) is still legal, but `flows check` marks the line and preflight emits a
`vacuous_gate` warning: a gate that judges nothing must not read like one
that judges something.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Rebase onto main brought in #133's `output:` sugar, which lowers to a `json_schema` gate at compile time but was only checked with `isObject`. So an `output` schema the kernel refuses passed `flows check` and was reported as a gate — an unbounded `$ref` cycle included. Route it through `jsonSchemaError`, the same gate a hand-written `verification: {type: json_schema}` clears. Also adapts one test to main's boundary contract rather than deleting it: `preflight` now returns a named `invalid_spec` refusal where it used to throw, so the proxy-boundary test accepts either refusal shape and treats "returned a usable result" as a failure. Trap and getter counters are untouched. Adds ops/reviews/20260903-pr139-repair-0903.md: the red four-execution ladder, the structural fix, the shared kernel/SDK corpus, the three-path `output` proof, the silently-merged-file enumeration, and every gate with literal output. It also records that origin/main moved from 3da71e2 to 990093b (#136) mid-work and that this branch is rebased onto 990093b, not the pinned SHA. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
… kernel exit-101 Two conflict resolutions on this rebase could have silently reverted freshly merged behaviour, not one. The brief named step-fields.ts; compile.ts's compileStep is the sharper one, because neither validateSpec nor `flows check` can see it — both still report a healthy gate when the `output:` lowering has been reverted to the raw authored gate. Only compileYaml + toKernelSpec, read against the kernel verification object, tells "the key was accepted" apart from "the key became a gate". Records that, and that one of the reverting lines auto-merged without git flagging a conflict. Also characterises the kernel gate's one exit-101-with-zero-failures rather than leaving it as flake: the binary is named (relayflowd-core spec_parity, and only that one), disk is ruled out at 30 GiB free, one clean reproduction attempt came back green, and concurrent load is named as the untested condition. Recorded as unexplained. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
#138 moved `timeoutMs` out of BaseStepSpec/STEP_COMMON_FIELDS into DeterministicStepSpec/STEP_FIELDS_BY_TYPE.deterministic, and out of compileStep's shared `base` into the deterministic branch. Both conflicts this rebase produced landed on that hunk and on the `output` lowering beside it. compile.ts's deterministic branch now reads the SHARED `verification` binding and carries #138's timeoutMs spread. The two sides of that conflict are equal today — typedOutputVerification returns step.verification unchanged for a verb that cannot declare `output` — so either would have passed every test; the shared binding is kept because it is what holds the invariant that every branch of the switch reads the lowered gate, not the raw authored one. Adapts one assertion in #138's new dependency-validation suite: an exact toEqual on PreflightResult, which this PR widens with `gates`. A refused spec compiled nothing, so its gate plan is empty. No assertion weakened, no test added or removed. Report updates: the base moved twice and this rebase pinned the SHA; the verification standard the traps expose — validateSpec, preflight and `flows check` all answer "was the key accepted?", and only compileYaml + toKernelSpec answers "did it reach the kernel?", so that path is the primary assertion and the others corroborate; a four-path timeoutMs proof; and #138's own blob-comparison method applied to all 15 files it touched, with every deleted line attributed (two were widenings reading as deletions, the same false-alarm shape #138's signoff found). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Signoff 4 reproduced the original P0 verbatim on the repaired head: one logical step executed four times, daemon SIGABRT on run.start and every resume, run stuck "running". The route was a $ref written as a URI naming an $id declared inside the same document -- the standard 2020-12 compound-schema-document form that every bundler emits. Both resolvers keyed on a leading "#", so no edge was added, no cycle was found, and jsonschema then resolved it from the document's own resource map and overflowed. The load-bearing error was the comment justifying that: "an unresolvable reference is left opaque, validator_for refuses it outright". True for remote resources, false for an in-document $id. The premise held for one case and was generalised to both. Reference resolution is now URI-aware and mirrored function for function across schema.rs and json-schema-bound.ts: collect_scopes builds a resource map keyed by resolved base URI AND by the raw $id (consistency between registration and lookup matters more than exact RFC 3986 normalization, and a bundled document writes the same literal in both places); anchors are keyed (base URI, name) instead of document-wide first-match-wins, which closes the duplicate-anchor crash; resolve splits <uri>#<fragment>, resolves the URI part against the base in effect at that node, and applies the fragment inside that resource. The checker stays iterative. Also settles the divergence in the other direction: a RangeError out of Ajv's compile is caught and discarded rather than reported as "invalid JSON Schema: Maximum call stack size exceeded". The rule decides legality, the engine decides only well-formedness, and a stack overflow is neither verdict -- by the time Ajv runs the bound has already proved the declaration terminates and the kernel accepts it. Narrow by construction: a schema the bound refuses never reaches Ajv. The corpus is extended by derivation from the specification's reference forms rather than from the file: F1-F12, each with a refused instance and, where the form can express one, an accepted instance. 12 -> 20 refused, 14 -> 22 accepted. F12 gets its own engineRefused bucket that pins BOTH halves of the narrowed premise -- the bound must not claim these, the engine must refuse them -- so a future engine that accepts an unresolvable reference fails a test instead of silently reopening the hole. Also corrects three things signoff 4 caught in the report: a STEP_FIELDS_BY_TYPE evidence block quoted from the pre-#138 base, an undisclosed fourth test adaptation of the gates:[] class in cli.test.ts, and the anchor-scoping item in "what I did not verify" -- which I had guessed would be a false refusal rather than a crash, and the guess was wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
origin/main moved to 16860d2 (#151, trigger-key lowering) right after the last push. Rebased onto the pinned SHA; two conflicts, and the first is trap 2's shape a fourth time in the same function. #151 added `triggers: flow.triggers.map(toKernelTrigger)` to toKernelSpec -- authoring keys lowered into the kernel's snake_case dialect, which is authoring sugar becoming a different object at the boundary, exactly like `output:`. This branch had changed the same lines from `flow.*` to `compiled.*` for the snapshot guard. Taking either side wholesale reverts the other; the resolution is `compiled.triggers.map(toKernelTrigger)`. The prediction from section 10 held: validateSpec returns ok=true and `flows check` returns CHECK PASSED exit=0 whether or not the lowering happened. Only compileYaml + toKernelSpec, read against the kernel object, shows eventType -> event_type. Unlike the first three traps this one also has committed fixtures behind it -- #151 pinned a canonical form and a spec hash -- so a reverted lowering would go red in the suite too. Blob-compared all 14 files #151 touched: 8 byte-identical including both pinned fixtures, 6 changed by me with every deletion attributed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
1ed9023 to
4da825b
Compare
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Merging: green CI, independent signoff carried, zero unresolved threadsCI green at Getting there took three runs, and the two reds in between were not this PR:
I re-ran rather than attributing, which is the rule I wrote up on #156 after getting it wrong twice. The signoff carries, proven rather than assumed. It returned PASSED at Only
Zero unresolved review threads. For the record on the rebase itself: the conflict was trap 1 — 🤖 Generated with Claude Code |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* feat(sdk): settle data and code gate contract
Session-Id: 01a062df-0cdf-7f23-89dd-121aa9ecf743
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* fix(sdk): fail closed on invalid gates
Session-Id: 01a062df-0cdf-7f23-89dd-121aa9ecf743
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* fix(sdk): bundle JSON Schema draft metadata
Session-Id: 01a062df-0cdf-7f23-89dd-121aa9ecf743
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* fix(sdk): fail closed at gate boundaries
Session-Id: 01a062df-0cdf-7f23-89dd-121aa9ecf743
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* fix(runtime): close gate boundary execution holes
Session-Id: 01a062df-0cdf-7f23-89dd-121aa9ecf743
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* fix(sdk): reject proxies at exported boundaries
Session-Id: 01a062df-0cdf-7f23-89dd-121aa9ecf743
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* fix(gates): bound JSON Schema declarations so validation terminates
A `json_schema` gate whose `$ref` graph cycles through only in-place
applicators compiles cleanly and then recurses without bound the first time
it validates an output. In Rust that aborts the process, so `run.start`
accepted the spec, created the journal, ran the step's command, and then took
relayflowd down with SIGABRT — leaving a run stuck `running` that re-executed
its effect on every resume (4 executions of one logical step, no
`completionReason`, no `step.completed`). RFC-0001 covenant 2 and gate 1.
A stack overflow cannot be caught, so the bound is structural and runs before
the declaration is accepted: reject a reference cycle that re-applies to the
same instance and therefore makes no progress. Cycles through a child
applicator (`properties`, `items`, `prefixItems`, ...) consume one level of
the instance per step and stay legal, so ordinary recursive schemas are
unaffected.
`kernel/relayflowd-core/src/schema.rs` and `sdk/src/json-schema-bound.ts`
implement the same rule and are pinned to a shared corpus in
`testdata/json-schema-bound-cases.json`, so the kernel and the SDK agree on
which schemas are legal by construction rather than by coincidence of Ajv's
catchable RangeError and Rust's uncatchable abort. That also closes the
reported SDK/kernel divergence on a self-recursive `$defs`. Every corpus
refusal compiles cleanly in `jsonschema`, which is what makes the tests test
the bound and not the mechanism. `verify` now compiles through the same gate.
Also from the same review:
- `canonicalize`/`specHash` are exported unknown-input helpers, so they carry
the snapshot guard the rest of the exported surface already has, and each
key is read exactly once instead of twice (a demonstrated getter TOCTOU).
- A `json_schema` gate that accepts every output (`{}`, `true`, annotations
only) is still legal, but `flows check` marks the line and preflight emits a
`vacuous_gate` warning: a gate that judges nothing must not read like one
that judges something.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* fix(sdk): gate output declarations through the same schema bound
Rebase onto main brought in #133's `output:` sugar, which lowers to a
`json_schema` gate at compile time but was only checked with `isObject`. So an
`output` schema the kernel refuses passed `flows check` and was reported as a
gate — an unbounded `$ref` cycle included. Route it through `jsonSchemaError`,
the same gate a hand-written `verification: {type: json_schema}` clears.
Also adapts one test to main's boundary contract rather than deleting it:
`preflight` now returns a named `invalid_spec` refusal where it used to throw,
so the proxy-boundary test accepts either refusal shape and treats "returned a
usable result" as a failure. Trap and getter counters are untouched.
Adds ops/reviews/20260903-pr139-repair-0903.md: the red four-execution ladder,
the structural fix, the shared kernel/SDK corpus, the three-path `output`
proof, the silently-merged-file enumeration, and every gate with literal
output. It also records that origin/main moved from 3da71e2 to 990093b (#136)
mid-work and that this branch is rebased onto 990093b, not the pinned SHA.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* docs(review): name the second silent-revert trap and characterise the kernel exit-101
Two conflict resolutions on this rebase could have silently reverted freshly
merged behaviour, not one. The brief named step-fields.ts; compile.ts's
compileStep is the sharper one, because neither validateSpec nor `flows check`
can see it — both still report a healthy gate when the `output:` lowering has
been reverted to the raw authored gate. Only compileYaml + toKernelSpec, read
against the kernel verification object, tells "the key was accepted" apart from
"the key became a gate". Records that, and that one of the reverting lines
auto-merged without git flagging a conflict.
Also characterises the kernel gate's one exit-101-with-zero-failures rather
than leaving it as flake: the binary is named (relayflowd-core spec_parity, and
only that one), disk is ruled out at 30 GiB free, one clean reproduction
attempt came back green, and concurrent load is named as the untested
condition. Recorded as unexplained.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* fix(sdk): keep timeoutMs deterministic-only through the rebase onto #138
#138 moved `timeoutMs` out of BaseStepSpec/STEP_COMMON_FIELDS into
DeterministicStepSpec/STEP_FIELDS_BY_TYPE.deterministic, and out of
compileStep's shared `base` into the deterministic branch. Both conflicts this
rebase produced landed on that hunk and on the `output` lowering beside it.
compile.ts's deterministic branch now reads the SHARED `verification` binding
and carries #138's timeoutMs spread. The two sides of that conflict are equal
today — typedOutputVerification returns step.verification unchanged for a verb
that cannot declare `output` — so either would have passed every test; the
shared binding is kept because it is what holds the invariant that every branch
of the switch reads the lowered gate, not the raw authored one.
Adapts one assertion in #138's new dependency-validation suite: an exact
toEqual on PreflightResult, which this PR widens with `gates`. A refused spec
compiled nothing, so its gate plan is empty. No assertion weakened, no test
added or removed.
Report updates: the base moved twice and this rebase pinned the SHA; the
verification standard the traps expose — validateSpec, preflight and
`flows check` all answer "was the key accepted?", and only compileYaml +
toKernelSpec answers "did it reach the kernel?", so that path is the primary
assertion and the others corroborate; a four-path timeoutMs proof; and #138's
own blob-comparison method applied to all 15 files it touched, with every
deleted line attributed (two were widenings reading as deletions, the same
false-alarm shape #138's signoff found).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* fix(gates): resolve $ref as a URI, closing the compound-document bypass
Signoff 4 reproduced the original P0 verbatim on the repaired head: one logical
step executed four times, daemon SIGABRT on run.start and every resume, run
stuck "running". The route was a $ref written as a URI naming an $id declared
inside the same document -- the standard 2020-12 compound-schema-document form
that every bundler emits. Both resolvers keyed on a leading "#", so no edge was
added, no cycle was found, and jsonschema then resolved it from the document's
own resource map and overflowed.
The load-bearing error was the comment justifying that: "an unresolvable
reference is left opaque, validator_for refuses it outright". True for remote
resources, false for an in-document $id. The premise held for one case and was
generalised to both.
Reference resolution is now URI-aware and mirrored function for function across
schema.rs and json-schema-bound.ts: collect_scopes builds a resource map keyed
by resolved base URI AND by the raw $id (consistency between registration and
lookup matters more than exact RFC 3986 normalization, and a bundled document
writes the same literal in both places); anchors are keyed (base URI, name)
instead of document-wide first-match-wins, which closes the duplicate-anchor
crash; resolve splits <uri>#<fragment>, resolves the URI part against the base
in effect at that node, and applies the fragment inside that resource. The
checker stays iterative.
Also settles the divergence in the other direction: a RangeError out of Ajv's
compile is caught and discarded rather than reported as "invalid JSON Schema:
Maximum call stack size exceeded". The rule decides legality, the engine
decides only well-formedness, and a stack overflow is neither verdict -- by the
time Ajv runs the bound has already proved the declaration terminates and the
kernel accepts it. Narrow by construction: a schema the bound refuses never
reaches Ajv.
The corpus is extended by derivation from the specification's reference forms
rather than from the file: F1-F12, each with a refused instance and, where the
form can express one, an accepted instance. 12 -> 20 refused, 14 -> 22 accepted.
F12 gets its own engineRefused bucket that pins BOTH halves of the narrowed
premise -- the bound must not claim these, the engine must refuse them -- so a
future engine that accepts an unresolvable reference fails a test instead of
silently reopening the hole.
Also corrects three things signoff 4 caught in the report: a STEP_FIELDS_BY_TYPE
evidence block quoted from the pre-#138 base, an undisclosed fourth test
adaptation of the gates:[] class in cli.test.ts, and the anchor-scoping item in
"what I did not verify" -- which I had guessed would be a false refusal rather
than a crash, and the guess was wrong.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
* docs(review): record the rebase onto #151 and the fourth trap
origin/main moved to 16860d2 (#151, trigger-key lowering) right after the last
push. Rebased onto the pinned SHA; two conflicts, and the first is trap 2's
shape a fourth time in the same function.
#151 added `triggers: flow.triggers.map(toKernelTrigger)` to toKernelSpec --
authoring keys lowered into the kernel's snake_case dialect, which is authoring
sugar becoming a different object at the boundary, exactly like `output:`. This
branch had changed the same lines from `flow.*` to `compiled.*` for the
snapshot guard. Taking either side wholesale reverts the other; the resolution
is `compiled.triggers.map(toKernelTrigger)`.
The prediction from section 10 held: validateSpec returns ok=true and
`flows check` returns CHECK PASSED exit=0 whether or not the lowering happened.
Only compileYaml + toKernelSpec, read against the kernel object, shows
eventType -> event_type. Unlike the first three traps this one also has
committed fixtures behind it -- #151 pinned a canonical form and a spec hash --
so a reverted lowering would go red in the suite too.
Blob-compared all 14 files #151 touched: 8 byte-identical including both pinned
fixtures, 6 changed by me with every deletion attributed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
---------
Co-authored-by: kjgbot <kjgbot@agentrelay.dev>
Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
…aked test daemons Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
…o merged Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
…s stale #134 and #139 both merged four days ago and the allSettled fix is on main in refactored form — two of my checks gave false negatives (stale path, grep for the old branch's identifiers) before I confirmed the behaviour. Verified the docs disclosure the code comment claims rather than trusting it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FtQSAcGDta5VH9xiZFT4sR
Summary
Closes issue #132 item 6 without adding an expression language or changing kernel vocabulary.
flows checkreportsverification:lowering byte-for-byte unchangedRed-first evidence
Before
sdk/src/gate-contract.tsexisted:Verification
Honest aggregate-run note
The first all-files SDK run had 242 passing tests and one 5-second timeout in the first live-kernel case while files ran in parallel. The complete live-kernel file then passed 17/17 alone as captured above; the remaining files passed 227/227 together. No mutation-verification claim is made.