feat(sdk): flows build — immutable content-addressed bundle (#298) - #316
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review swarm: maintainabilityNo fresh transcript was produced for run |
Review swarm: historyNo fresh transcript was produced for run |
Review swarm: structureNo fresh transcript was produced for run |
|
🎯 review-swarm: FAILED (M:pass H:pass S:missing) Lens transcripts posted as sibling comments above. |
…d-agent CLIs (#316) Two HIGH Bugbot findings on flows#316 (Cursor Bugbot): 1. `packages/sdk/src/cli/build.ts:76` — build's preflight ran with no `modelRegistryPath` or `models` allowlist, so every declared model surfaced as `model_unknown` and refused the whole build. Load the nearest `flows.json` through the existing `readProjectConfig` helper and forward `models` + `modelRegistryPath` + `projectCli` + `projectSearchStart` into the preflight call. Live model probes stay deferred because build-time preflight cannot honestly claim a live model is reachable — but model existence, which is a build-provable fact, is now honored. 2. `packages/sdk/src/cli/build.ts:148` — captureFiles only walked the steps, so a flow whose CLI was inherited from `flow.cli` or from an entry in `agents:` never had that binary copied into the bundle. Add two capture passes before the step walk: `flow.cli` (top-level default) and each `flow.agents[name].cli`. Rewrite them to the `./assets/...` bundled path so the sealed spec never points outside the bundle. Step-level captures still run and still win when both are set. The 20 bundle tests still pass end-to-end (including the two-run byte-for-byte determinism, TS fixture build, and CLI-argument refusals). Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
97aed52 to
08b9e03
Compare
Session-Id: 01a08f7d-7049-7f30-ad8d-447f6cee21a2 Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82 Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
…d-agent CLIs (#316) Two HIGH Bugbot findings on flows#316 (Cursor Bugbot): 1. `packages/sdk/src/cli/build.ts:76` — build's preflight ran with no `modelRegistryPath` or `models` allowlist, so every declared model surfaced as `model_unknown` and refused the whole build. Load the nearest `flows.json` through the existing `readProjectConfig` helper and forward `models` + `modelRegistryPath` + `projectCli` + `projectSearchStart` into the preflight call. Live model probes stay deferred because build-time preflight cannot honestly claim a live model is reachable — but model existence, which is a build-provable fact, is now honored. 2. `packages/sdk/src/cli/build.ts:148` — captureFiles only walked the steps, so a flow whose CLI was inherited from `flow.cli` or from an entry in `agents:` never had that binary copied into the bundle. Add two capture passes before the step walk: `flow.cli` (top-level default) and each `flow.agents[name].cli`. Rewrite them to the `./assets/...` bundled path so the sealed spec never points outside the bundle. Step-level captures still run and still win when both are set. The 20 bundle tests still pass end-to-end (including the two-run byte-for-byte determinism, TS fixture build, and CLI-argument refusals). Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82 Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
captureFiles wrapped every named-agent CLI capture in Promise.all, but the map guard held resolved strings that were only written AFTER await. Two agents sharing the same ./cli path both passed the has() check and both pushed the same assets/... entry — then sealBundle refused the whole flow as a duplicate manifest path. Store the in-flight promise instead, so concurrent captures of the same path return the same resolved target and the file is pushed exactly once. Adds a regression test with two named agents sharing a CLI path; the manifest has one asset entry and both lowered steps reference it. Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82 Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82
5a913ac to
2e3a1bf
Compare
Review swarm: FAILED
Cloud run: |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2e3a1bf. Configure here.
| }); | ||
| const refusals = report.diagnostics.filter(d => d.severity === 'refusal' && d.kind !== 'probe_failed'); | ||
| if (refusals.length > 0) throw new Error(refusals.map(d => `[${d.kind}] ${d.message}`).join('\n')); | ||
| authoring = await captureFiles(authoring, directory, files); |
There was a problem hiding this comment.
Project CLI omitted from sealed bundle
High Severity
flows build feeds flows.json cli into preflight so those flows pass, but captureFiles never records or copies that CLI. toKernelSpec also does not lower project CLI onto steps. The sealed spec can therefore omit the CLI that preflight accepted, and a relative project CLI never enters assets/.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2e3a1bf. Configure here.
| if (!path.includes('/')) return Promise.resolve(path); | ||
| if (!path.startsWith('./') || path.split('/').includes('..') || path.includes('\\')) { | ||
| return Promise.reject(new Error(`${path}: bundle file references must start with ./ and stay inside the flow directory`)); | ||
| } |
There was a problem hiding this comment.
Unprefixed relative CLIs cannot bundle
Medium Severity
capture treats any CLI containing / that does not start with ./ as invalid. flows check already resolves those paths against the spec directory, and first-party flows such as testdata/hn-monitor.flow.yaml declare cli: preflight/analyze-story-claude-cli. Building those valid specs now fails.
Reviewed by Cursor Bugbot for commit 2e3a1bf. Configure here.
| const ref = (match[1] ?? match[2] ?? match[3])!; | ||
| const start = match.index! + match[0].indexOf(ref); | ||
| command = command.slice(0, start) + await capture(ref) + command.slice(start + ref.length); | ||
| } |
There was a problem hiding this comment.
Redirection targets captured as required assets
Medium Severity
The command scanner treats every ./ word after >, <, or whitespace as a bundle input. A deterministic step that redirects onto ./out.txt therefore lstats an output path and refuses the build when that file does not already exist.
Reviewed by Cursor Bugbot for commit 2e3a1bf. Configure here.
| command = command.slice(0, start) + await capture(ref) + command.slice(start + ref.length); | ||
| } | ||
| if (/^\s*["']?\//.test(command)) throw new Error(`${step.id}: absolute command paths cannot be bundled`); | ||
| steps.push({ ...step, command }); |
There was a problem hiding this comment.
Parent-relative commands escape the bundle
Medium Severity
Absolute command paths are refused, but ../ words never match the ./ capture pattern and are not rejected. The sealed spec can keep a parent-relative command that walks out of the digest directory at run time.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2e3a1bf. Configure here.


Closes #298.
Summary
Adds
flows build <flow>— canonical spec JSON + compiled TS with pinned deps + preflight declaration + identity signature, sealed intodist/flows/<name>@sha256:<hex>/with a manifest linking every file to its sha256. Top-level digest issha256(manifest.json), making the bundle content-addressed and tamper-evident.Design decision
TS authored flows: because
flow()bodies execute at runtime and cannot be canonicalized without side effects, this slice ships an exported-spec pattern — authors export both theflow()body and a validatedspecdeclaration.flows buildcompiles the spec, not the body. The limitation is documented in the PR body and README.Not in scope
flows deployat digestflows run flow@sha256:...from bucketWritten by codex agent
spec-A-flows-build-v4on finn-mini; head at97aed52.Test plan
🤖 Generated with Claude Code
Note
Medium Risk
Introduces bundle signing, executable/asset bundling, and strict verification logic that will underpin digest-based deployment; mistakes could weaken integrity guarantees or ship wrong binaries, though run/deploy-by-digest is not wired yet.
Overview
Adds
flows buildandflows build --verifyto seal flows into content-addressed directories underdist/flows/<name>@sha256:<digest>/, and documents the contract indocs/SURFACE.md.Build pipeline: YAML flows compile and preflight at build time (credentials/workers/MCP probes deferred); relative
./CLIs and deterministic command words are copied intoassets/and rewritten in the canonical spec. TypeScript flows require Bun, a matchingpackage-lock.json/npm citree, and either a default-exported declarative spec orflow()plus an exportedspec(body not executed at build); Bun also emits a compiledflowbinary and embeds the lockfile.Bundle format:
sealBundlewrites manifest + Ed25519identity.json(fromFLOWS_BUILD_KEYor repo.flows/build.key, else ephemeral with a stderr warning), reuses an existing valid digest directory, andverifyBundlefails closed on tampering, extras, symlinks, or digest mismatch (CLI exit 2)..flows/build.keyis gitignored.Vitest coverage exercises sealing, verification, asset capture/dedup, YAML/TS CLI paths, and invalid args.
Reviewed by Cursor Bugbot for commit 2e3a1bf. Bugbot is set up for automated code reviews on this repo. Configure here.