Skip to content

feat(sdk): flows deploy — publish digest to bucket + run from digest (#333) - #337

Merged
kjgbot merged 2 commits into
mainfrom
feat/spec-Adep-flows-deploy
Sep 11, 2026
Merged

kjgbot merged 2 commits into
mainfrom
feat/spec-Adep-flows-deploy

Conversation

@kjgbot

@kjgbot kjgbot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Closes #333. Deploy content-addressed bundles per SURFACE.md §4 follow-up to #298. Codex agent spec-Adep-flows-deploy on finn-mini; head 10b2e48.

🤖 Generated with Claude Code


Note

Medium Risk
New deploy/run paths verify bundles and gate execution before journal writes, but they change how flows run resolves inputs and how untrusted bundle directories are validated—worth careful review on tampering, partial deploy, and cache behavior.

Overview
Adds the first deployment slice for content-addressed bundles: flows deploy <name>@sha256:<digest> --to file://… publishes a locally built bundle from dist/flows/… into <bucket>/<name>/sha256/<digest>/ with staged copy, signature verification, and idempotent deploy_noop when the target already matches. flows run <name>@sha256:<digest> [--bucket …] fetches from the bucket (or deploy.bucket in flows.json), verifies every cache hit under $XDG_CACHE_HOME/flows/bundles/<digest>/, runs checkRunnableBundle (declarative deterministic flows only—no assets, triggers, placement, or authored TS), then submits the sealed spec.canonical.json through the existing journal path without a checkout.

bundle-transport.ts centralizes digest parsing, file-bucket layout, and copyBundle/fetchBundle. verifyBundle accepts an optional expected digest for bucket/cache directories. Run refusals gain typed kinds (bucket_unconfigured, bucket_unreachable, bundle_signature_invalid, bundle_unsupported); deploy uses exit 1 for deploy_partial after copy starts. SURFACE.md documents the slice and explicitly lists remaining #333 work (S3, triggers, full bundle kinds).

Reviewed by Cursor Bugbot for commit 5d409ee. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: df39e71f-7298-431b-a3f6-b1730de8e24f


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.

❤️ Share

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

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 5 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 10b2e48. Configure here.

Comment thread packages/sdk/src/cli/bundle-preflight.ts Outdated
Comment thread packages/sdk/src/cli/bundle-preflight.ts Outdated
Comment thread packages/sdk/src/bundle-transport.ts
Comment thread packages/sdk/src/cli/deploy.ts Outdated
Comment thread packages/sdk/src/cli/run-digest.ts
Session-Id: 01a09169-a0d5-7f72-a5fa-fd26569704aa

Session-Id: 01a091df-35ad-7771-a10f-3d4322765694
Session-Id: 01a091df-35ad-7771-a10f-3d4322765694
@miyaontherelay
miyaontherelay force-pushed the feat/spec-Adep-flows-deploy branch from 10b2e48 to 5d409ee Compare September 11, 2026 19:13
@kjgbot

kjgbot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

maintainability lens — FAIL

Maintainability review — PR #337

Blockers

verifyBundle implicit-contract change (packages/sdk/src/bundle.ts:115-151). The optional expectedDigest? parameter silently changes verification semantics: passing it bypasses the directory-name binding entirely (basename(root).endsWith(…)). Nothing at the type level distinguishes "trusted local build (must use directory-name check)" from "already-known digest from a signed manifest lookup." A future caller who copies-paste-invokes verifyBundle(dir, someDigest) in a new local-build path silently disables the directory binding this function was designed to enforce. The JSDoc "Bucket/cache readers supply the requested digest" is a policy statement, not an invariant. Recommend splitting into two functions or requiring an explicit mode enum.

Error-kind laundering in deploy.ts:47-50 and run-digest.ts:17-22. Both fallback to a specific BundleFailureKind for any non-BundleFailure exception. In runDeploy, non-BundleFailure errors before started are labeled bucket_unreachable — a compileSpec/kernelToAuthoring parse failure would surface as "bucket unreachable." In prepareDigestRun, unknown BundleFailure.kind values (including future additions to BundleFailureKind) collapse into bundle_unsupported. Future maintainers adding a new failure kind will break diagnostic accuracy without a compile error. Either use an exhaustive switch on the union or let unknown kinds propagate as an internal error.

Concerns

Test-file naming collision. tests/run-digest.test.ts (config refusals) and tests/run-from-digest.test.ts (happy-path + cache) are near-identical filenames covering different scopes. A stranger looking for a "run from digest" test will pick the wrong file. Consolidate or rename to reflect scope (run-digest-config.test.ts).

bundle-transport.ts:20-25 bucketDirectory error mapping. All URL parsing failures (bad scheme, query, hash, relative path) collapse into BundleFailure('bucket_unreachable', 'This slice requires an absolute file:// bucket URI.'). The user sees "unreachable" for what is really "malformed." Consider a distinct bucket_uri_invalid or preserve the underlying reason.

bundle-transport.ts:33-35 verifyDigest swallows structural errors as signature failures. ENOENT on manifest.json, JSON parse errors, and permission failures all become bundle_signature_invalid. Debugging is harder when the message doesn't match root cause.

cli.ts:185 spreads { bucket: parsed.bucket } unconditionally when command === 'run', unlike the !== undefined guard used one line below for reuseFromRunId. Not incorrect, but the inconsistency invites bugs when the option list grows.

bundle-preflight.ts:16-22 — the compound predicate (kind ≠ declarative OR any non-deterministic step OR any requirements OR any triggers OR any assets) emits a single generic bundle_unsupported. When a reviewer years from now adds worker or LLM support, they will not know which sub-condition their bundle tripped.

Notes

  • Evidence file documents that CI depends on Bun on PATH (evidence/spec-Adep/verification.txt:87-91), but nothing in the SDK build refuses gracefully; the failure mode leaks as bundle_invalid.
  • SURFACE.md:497-520 clearly scopes the slice — good.
  • flow.name ??= name mutation in bundle-preflight.ts:16 is subtle but tested.

REVIEW_FAILED

@kjgbot

kjgbot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

history lens — PASS

Blockers: none. I found no qualifying HISTORY violation in the supplied diff.

Notes:

  • The change extends the immutable-bundle work landed in 3a078ab9 (feat(sdk): flows build — immutable content-addressed bundle (#298) #316). In packages/sdk/src/bundle.ts:115–153, the optional expected digest accommodates bucket/cache directory names while preserving manifest hashing and the existing payload/signature checks. This does not restore a previously removed verification bypass.
  • DRIVE-LOG records the earlier mistake of accepting unresolved deterministic commands without diagnostics. packages/sdk/src/cli/bundle-preflight.ts:21–28 retains the existing preflight evaluator, and packages/sdk/tests/bundle-preflight.test.ts:50–59 asserts the unresolved-command warning. Isolating cache execution from surrounding project configuration also follows the history’s concern about ambient flows.json contamination.
  • packages/sdk/src/cli/run.ts:89–98 routes prepared bundles into the existing execution path. No kernel, journal, or review-gate implementation is changed. Unsupported bundle kinds are explicitly refused in packages/sdk/src/cli/bundle-preflight.ts:15–19.
  • The two commit subjects—“flows deploy + digest-run file bucket slice” and “isolate digest preflight and correct deploy refusals”—match their changes. Neither claims comprehensive verification or completion of the full deployment contract.

Concerns, nonblocking:

This was a static history review; I did not rerun tests.

REVIEW_PASSED

@kjgbot

kjgbot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

structure lens — MISSING

@kjgbot

kjgbot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

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

Lens transcripts posted as sibling comments above.

@kjgbot
kjgbot merged commit d44e31c into main Sep 11, 2026
8 of 10 checks passed
@kjgbot
kjgbot deleted the feat/spec-Adep-flows-deploy branch September 11, 2026 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flows: flows deploy — publish immutable bundle to a bucket + pin trigger to digest (SURFACE §4 follow-up to #298)

2 participants