fix(sdk): refuse fields outside each step verb schema - #138
Merged
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (13)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughCompilation and validation now enforce verb-specific fields, deterministic-only timeouts, structured output schemas, named-agent resolution, and iterative dependency-cycle checks. SDK and kernel tests cover deep graphs, malformed specifications, output verification, CLI checks, and type-level field consistency. ChangesFlowSpec contracts and compilation
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR adds rejection of unknown per-step fields while preserving valid fields, with focused and full SDK checks passing; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant FlowSpec
participant toKernelSpec
participant validate
participant NamedAgents
participant Kernel
FlowSpec->>toKernelSpec: submit FlowSpec
toKernelSpec->>validate: validate fields and dependencies
validate-->>toKernelSpec: validation result
toKernelSpec->>NamedAgents: resolve agent selector
NamedAgents-->>toKernelSpec: CLI and model
toKernelSpec->>Kernel: send lowered spec and JSON-schema gate
Kernel-->>toKernelSpec: compiled kernel specification
toKernelSpec-->>FlowSpec: return KernelSpec
Poem
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 |
Contributor
Author
Review repair evidence — exact head
|
Contributor
Author
P1 repair evidence — exact head
|
kjgbot
commented
Sep 3, 2026
kjgbot
left a comment
kjgbot
left a comment
Contributor
Author
There was a problem hiding this comment.
Structure signoff verdict: COMPREHENSIVELY_SATISFIED for reviewed head 0c859270801e058bd59b76a72848b6480a740de6 only (base a0d42ffbdc7fb60b42c0b5bea4f58408249b08a2). I did not review the current PR head e164e4239b0aa9e8b2dd58ed126263d206ad29d0; this is not an approval.
No blocking SDK/kernel structure or parity finding at the reviewed head. Evidence: timeoutMs is deterministic-only in the public type (sdk/src/spec.ts:111), closed descriptor (sdk/src/step-fields.ts:17), validate-before-lowering boundary (sdk/src/compile.ts:152), deterministic-only lowering (sdk/src/compile.ts:344), and compiled-dialect CLI marker (sdk/src/cli/check.ts:233). The Rust mirror owns timeout_ms only on StepKind::Deterministic (kernel/relayflowd-core/src/spec.rs:180, kernel/relayflowd-core/src/spec.rs:249). Iterative graph validation is an explicit frame stack with typed SpecError::DependencyCycle (kernel/relayflowd-core/src/spec/dependencies.rs:5, kernel/relayflowd-core/src/spec.rs:500).
Verification at the reviewed head: focused SDK type/runtime suite passed 65/65; relayflowd-core spec::tests passed 10/10, including valid and cyclic 10,000-step reverse graphs. The full evidence report is committed separately from this review comment.
Residual evidence limit: global npm on this host timed out before output, so I do not claim a literal npm test pass; the package manifest does invoke typecheck, and the exact typecheck script passed in an isolated dependency-linked copy.
kjgbot
pushed a commit
that referenced
this pull request
Sep 3, 2026
Records the resolution chosen for each conflict (including the three files git auto-merged without a marker), the reproduction and fix of the `output` regression the rebase introduced, the full one-sided-file audit, the generated-case hole and its closure, every gate with literal output and per-file count accounting, and the re-verification of the pre-rebase signoff's positive results. 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
kjgbot
force-pushed
the
feat/v2-field-lint
branch
from
September 3, 2026 11:30
e164e42 to
441c679
Compare
added 5 commits
September 3, 2026 13:35
Session-Id: 01a062df-b92c-7a13-815f-147a82a4fc51 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: 01a062df-b92c-7a13-815f-147a82a4fc51 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
Session-Id: 01a062df-b92c-7a13-815f-147a82a4fc51 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1 Session-Id: c228933d-4f94-4d83-9a9a-daf3c83b94f1
`STEP_FIELDS_BY_TYPE` is the only allowlist `validateSpec` consults, but it is a plain string table. `satisfies Record<StepType, readonly string[]>` checks its *shape* — three keys, each an array of strings — and nothing more. It does not check that the table agrees with `DeterministicStepSpec`, `LlmStepSpec` or `AgentStepSpec`. A field added to one of those interfaces but not to the table is therefore refused at runtime as an unknown key, with every compile gate green. That is not hypothetical. It happened twice in one day. #133 added `output` to the interfaces and to the then-current allowlist; when this branch moved the allowlist into `step-fields.ts` the new file merged without a conflict and without `output`, and only a test caught it. #136 then added `agent` to `AgentStepSpec` on the same descriptor. So `type-tests/step-fields.ts` now asserts the correspondence in both directions — every authoring field an interface declares is described, and nothing is described that the interface does not declare. Dropping `output`, `agent` or `timeoutMs` from the table is now a `tsc` error (TS2344) instead of a silent runtime refusal. This is why a type-test file earns its place here: it is the only gate that can see this class of drift, because the drift is between a type and a value and no runtime test exercises the pairing directly. Also asserts the llm/agent `output` sugar end to end. The three paths diverge — `validateSpec` reads the in-memory object, `compileYaml` goes through a YAML round-trip that silently drops an `undefined` value, and `flows check` adds preflight and a process exit code — so each is asserted separately rather than assuming the first covers the others. 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
Replaces the previous rebase report. #136 landed between the two rebases and carries much of what #138 built, so two of the branch's five commits were dropped as empty and the diff against main fell from 8,463 deletions to 68. Records: which commits survived and why; the resolution of every conflict and every audited auto-merge, including the duplicate `foreignFieldValue` the commit-6 auto-merge introduced; three independent proofs that #136 was not reverted; the re-derived hazard enumeration now that the shared files exist on both sides; three-path proofs for `output` and `timeoutMs` across all three verbs; every gate with literal output and per-file count accounting against a measured 990093b baseline; and the mutation verification of the type binding, including the harness fault that first reported two false "guard missed it" results. 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
kjgbot
force-pushed
the
feat/v2-field-lint
branch
from
September 3, 2026 11:52
441c679 to
4d85603
Compare
Contributor
Author
Merging — signoff basis at
|
kjgbot
pushed a commit
that referenced
this pull request
Sep 3, 2026
#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
kjgbot
pushed a commit
that referenced
this pull request
Sep 3, 2026
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
kjgbot
pushed a commit
that referenced
this pull request
Sep 3, 2026
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
kjgbot
pushed a commit
that referenced
this pull request
Sep 3, 2026
#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
kjgbot
pushed a commit
that referenced
this pull request
Sep 3, 2026
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
kjgbot
added a commit
that referenced
this pull request
Sep 3, 2026
* 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 "/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 '/provider/item' deduped = false attempt2 '/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 "/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("/provider/item/", "/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>
kjgbot
pushed a commit
that referenced
this pull request
Sep 3, 2026
#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
kjgbot
pushed a commit
that referenced
this pull request
Sep 3, 2026
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
kjgbot
pushed a commit
that referenced
this pull request
Sep 4, 2026
#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
kjgbot
pushed a commit
that referenced
this pull request
Sep 4, 2026
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
kjgbot
pushed a commit
that referenced
this pull request
Sep 4, 2026
#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
kjgbot
pushed a commit
that referenced
this pull request
Sep 4, 2026
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
kjgbot
added a commit
that referenced
this pull request
Sep 4, 2026
* 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>
kjgbot
added a commit
that referenced
this pull request
Sep 4, 2026
* 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
This was referenced Sep 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #132 (item 7).
Outcome
deterministic,llm, andagentobject is checked against that verb's closed authoring schema.validateSpec,compileSpec,compileYaml, the exportedtoKernelSpec, directpreflight, andflows check.invalid_specrefusal with author-facing errors before it runs any CLI, command, or executor probe.Literal RED captured before implementation
Command:
Captured output:
The six public-compiler failures were all at the direct
toKernelSpecassertion. The six direct-preflight failures showed the malformed objects proceeding as valid and, for deterministic steps, producing a warning instead of a refusal.Literal focused GREEN evidence
Command:
Captured output:
tsc --noEmitandgit diff --checkproduced no output before the captured exit 0.Literal full SDK suite evidence
Command:
Captured output:
Kernel build disclosure
A fresh kernel build was attempted and did not run because the machine's
cargocommand is a stale mise shim:Therefore the full SDK suite above is evidence against the explicitly printed existing local relayflowd binary, not a claim that this branch freshly built the kernel. This PR changes SDK code only.
PR #136 composition audit
Command:
Captured output:
The combined tree is conflict-free. I also inspected its combined failure taxonomy, preflight validation ordering, and refusal-kind reachability test. I did not execute tests from that synthetic merge tree, so no combined-tree test claim is made.
No merge or auto-merge was performed.