Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion REWRITE.md
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ An adversarial audit of the repo's 8 issues + 216 PRs (~90 of them bug-fix/incid
| ---------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------- | -------------- |
| **One `Model` port over `@effect/ai`** | Two parallel model stacks (Worker `modelGateway` + container `demo-agent/model.ts`) each hand-roll transport routing, structured-output emulation, and retry; provider shape-divergence leaks past transport into every decode site | One `Model` Tag over `@effect/ai` provider Layers, consumed identically by Worker + container; native `completeStructured<A>`; **retire the raw `env.AI.run` binding as a transport**; normalization + `cf-aig-authorization` inside the adapter Layer; Bedrock = a dedicated SigV4 `InvokeModel` adapter | PR5 (+PR1 `completeStructured`, PR3 governor) | 22 |
| **Auth as decode-or-fail-loud contracts** | No Principal/Credential capability — every trust-boundary crossing (HMAC, installation token, CF Access) is hand-rolled inline and fails _green_ | Map each crossing onto an existing stage (no fourth pole): Trigger decode owns inbound (HMAC verify, `installation_id`≤0→`None`, `head.sha` a decoded field never ambient `GITHUB_SHA`); Sink typed against the granted permission + single-source manifest parity; a secret-parity+sync Layer (`Config`+`Redacted`+sha256[:8] both sides); one `AccessSession` Tag (outbound, cookie-by-url) + a separate inbound viewer-gate | PR1/PR2/PR3/PR4 | 20 |
| **Byte-boundary capability (`sandbox.capture`/`FileRef`)** | No capability owns "move a container path's bytes across the sandbox boundary"; each transfer re-picks an SDK read method and re-decodes framing, and the container FS is mistaken for durable state | One `sandbox.capture(path): Effect<FileRef, CaptureFailed \| DecodeFailed \| SizeMismatch>` chokepoint (`stat` size gate → always `readFileStream` → decode-once → `putStream`); `FileRef` schema handle rides checkpoints, never inline bytes; reads are _bounded_ (never `.arrayBuffer()` the whole archive); container FS is step-scoped non-durable — externalize to a `FileRef` and re-hydrate in the consuming step (keep #206's `isWorkingDirFailure` backstop) | PR4 (+PR1 `FileRef`, PR2 workspace) | 15 |
| **Byte-boundary capability (`sandbox.capture`/`FileRef`)** | No capability owns "move a container path's bytes across the sandbox boundary"; each transfer re-picks an SDK read method and re-decodes framing, and the container FS is mistaken for durable state | One `sandbox.capture(path): Effect<FileRef, CaptureFailed \| DecodeFailed \| SizeMismatch>` chokepoint (`stat` size gate → always `readFileStream` → decode-once → `putStream`); `FileRef` schema handle rides checkpoints, never inline bytes; reads are _bounded_ (never `.arrayBuffer()` the whole archive); container FS is step-scoped non-durable — externalize to a `FileRef` and re-hydrate in the consuming step (keep #206's `isWorkingDirFailure` backstop) — **partly landed**: `workspace()` now returns a rebuildable `spec` and `execInWorkspace` re-hydrates in the consuming step (ADR-0001 rule 3). The `FileRef` capture chokepoint is still open; the landed form re-clones rather than restoring captured bytes | PR4 (+PR1 `FileRef`, PR2 workspace) | 15 |
| **Four-way `RunOutcome` verdict + `RunError` partition** | The run→dispatcher boundary reduces a rich result to a pass/fail bit, so infra faults render as code findings and causes get swallowed | `verdict = Passed \| Finding(summaryMd) \| InfraFault(cause) \| Skipped(reason)`, one `Match.exhaustive` → GitHub conclusion; partition `RunError` into infra vs finding supertypes at definition; every `TaggedError` carries `cause` (`Cause.pretty`); a branded absolute `ArtifactRef` is the Sink's only link type | PR1 | 15 |
| **Execution env as a declared typed capability** | The sandbox env is an imperative property of one hand-maintained Dockerfile + a coarse `sandboxImage` string; every drift surfaces only at `Deploy` on main | Replace the enum with a Schema `RequiredCapabilities` set the run declares; one `imagePlan` → build args + checkout closure (computed from the pnpm dep graph, #159 gone); a deploy-time preflight that **runtime-probes** each declared capability; a PR-CI `BundleManifest` assertion (one file, shebang, `--help` exit 0); content-digest image keying; DO migrations = a reviewed delta vs a persisted tier ledger; `concurrency: cancel-in-progress` (not `Effect.timeout`) fixes the #77 build-queue hang | PR4 (net-new) | 14 |
| **`SandboxPool` admission capability** | The container pool is a finite shared resource nothing models; capacity is scattered across global/per-container/per-acquisition altitudes | One `SandboxPool` Tag is the sole scoped path to a container: D1 counting semaphore (sized to `max_instances`) wrapping the per-key lease, bounded transient-aware retry, one wall-clock deadline from `limits.maxDurationSec`; the dispatcher is a capacity gate, not only a router | PR1 Tag, PR3 consumes | 11 |
Expand Down
17 changes: 13 additions & 4 deletions apps/dispatcher/src/sandbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,23 @@ import type { Env } from "./env";
* paid a 10-minute idle tail after its last command. Across a CI-shaped
* workload (hundreds of short runs a day) that tail was ~45% of total spend.
*
* `isActivityExpired()` never fires while a request is in-flight (a long quiet
* `exec` keeps the container awake regardless), and the window restarts when
* the last request completes — so this only trims the *idle* tail. The primary
* `isActivityExpired()` never fires while a request is in-flight — it renews
* and returns false while `inflightRequests > 0` (`@cloudflare/containers`
* `dist/lib/container.js`), and `containerFetch` increments that before
* proxying, so a long quiet `exec` keeps the container awake regardless. The
* window restarts when the last request completes, so this only trims the
* *idle* tail. The primary
* teardown is the explicit `destroy()` at the workflow's finalize boundary
* (workflow.ts), which fires on success/failure/defect/interrupt; this idle
* window is only the backstop for paths that die before reaching it (Worker
* eviction, deploy mid-run).
*
* What this window does NOT do is make the filesystem durable. Cloudflare gives
* a container no durable disk, no minimum runtime, and restarts one that runs
* out of memory, so the tree can vanish while the container is BUSY — which no
* idle setting reaches. `execInWorkspace` is what recovers that; this window
* only narrows the idle case below. See ADR-0001 rule 3.
*
* Why 10m, not the 2m a cost pass once set: a run's container filesystem is the
* SHARED state across its durable steps — `step("checkout")` clones into it, a
* later `step("exec")` runs in it. A checkpointed step's RESULT is memoized, but
Expand All @@ -52,7 +61,7 @@ import type { Env } from "./env";
* `offload-test`) then mis-rendered as a red lint/test verdict. Because
* `destroy()` is the real teardown, a longer idle window costs extra ONLY on the
* rare paths that skip finalize, so 10m (the SDK default, and the `LEASE_TTL_MS`
* run-scale) buys durability across normal inter-step gaps at negligible cost.
* run-scale) covers normal inter-step gaps at negligible cost.
* `sandbox-cf.ts` `isWorkingDirFailure` is the honesty backstop for the residual
* (eviction / replay beyond this window): it re-classifies a lost-workspace exec
* as a retryable `ExecFailed`, never a phantom finding.
Expand Down
1 change: 1 addition & 0 deletions packages/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ The split keeps the layer boundary visible at the top of every recipe file. The
| Primitive | Does | Built from | Used by |
| --------------------------------------------------- | -------------------------------------------------------------- | -------------------------- | -------------------------- |
| [`workspace`](src/primitives/workspace.ts) | Acquire a container + clone a repo (+ optional cached install) | `sandbox`, `installCached` | every recipe |
| [`execInWorkspace`](src/primitives/exec-in-workspace.ts) | Run a command in a workspace, rebuilding the checkout if the container lost it | `sandbox`, `workspace` | any recipe that execs against a clone |
| [`installCached`](src/primitives/install-cached.ts) | R2-backed dependency install, keyed on the lockfile hash | `cache`, `sandbox` | `workspace`, browser-tests |
| [`sharded`](src/primitives/sharded.ts) | Count-and-index parallel fan-out | `Effect.forEach` | test-matrix, browser-tests |
| [`bootApp`](src/primitives/boot-app.ts) | Start a detached process and wait for its port | `sandbox` | cdp-acceptance |
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export class CheckoutFailed extends Schema.TaggedError<CheckoutFailed>()("Checko
export class ExecFailed extends Schema.TaggedError<ExecFailed>()("ExecFailed", {
exitCode: Schema.Number,
stderrTail: Schema.String,
/** The command never ran: its working directory was gone. `execInWorkspace` repairs this. */
workspaceMissing: Schema.optional(Schema.Boolean),
}) {
// Read by `runEffect` (step.ts) and folded into the own-property message of
// the Error thrown at the Workflow boundary — a prototype getter does not
Expand Down
25 changes: 25 additions & 0 deletions packages/core/src/fakes/sandbox-fake.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,15 @@ export const makeSandboxFake = (
* exercised. Each matched key is counted down independently across calls.
*/
launchFailures: Record<string, number> = {},
/**
* Transient lost-workspace failures, keyed by command-substring → number of
* leading `exec` attempts to reject with `ExecFailed.workspaceMissing`.
* Models the container being replaced after the checkout step completed:
* Cloudflare container disk does not survive a restart, so the tree the run
* checkpointed is simply gone. Counted down per key, like `launchFailures`,
* so a caller's rebuild-and-retry can be exercised.
*/
workspaceLosses: Record<string, number> = {},
): { layer: Layer.Layer<Sandbox>; state: SandboxFakeState } => {
const state: SandboxFakeState = {
acquired: [],
Expand All @@ -114,6 +123,7 @@ export const makeSandboxFake = (
const detachedCommands = new Map<string, string>();
// Mutable per-key countdown of remaining transient launch failures.
const remainingLaunchFailures = new Map<string, number>(Object.entries(launchFailures));
const remainingWorkspaceLosses = new Map<string, number>(Object.entries(workspaceLosses));

const resolve = (command: string): CannedExec | undefined => {
const key = Object.keys(program).find((k) => command.includes(k));
Expand All @@ -136,6 +146,21 @@ export const makeSandboxFake = (

exec: (opts: ExecOpts) => {
const command = normalizeCommand(opts.command);
const lostKey = Object.keys(workspaceLosses).find((k) => command.includes(k));
if (lostKey !== undefined) {
const remaining = remainingWorkspaceLosses.get(lostKey) ?? 0;
if (remaining > 0) {
remainingWorkspaceLosses.set(lostKey, remaining - 1);
state.execs.push({ command, cwd: opts.cwd, env: opts.env, timeoutSec: opts.timeoutSec });
return Effect.fail(
new ExecFailed({
exitCode: -1,
stderrTail: `working directory '${opts.cwd ?? ""}' was missing at exec time`,
workspaceMissing: true,
}),
);
}
}
const entry: SandboxFakeState["execs"][number] = {
command,
cwd: opts.cwd,
Expand Down
103 changes: 103 additions & 0 deletions packages/core/src/primitives/exec-in-workspace.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { it } from "@effect/vitest";
import { Effect, Exit, Layer } from "effect";
import { describe, expect } from "vitest";
import { ExecFailed } from "../errors";
import { makeSandboxFake } from "../fakes/sandbox-fake";
import { CacheFake } from "../fakes/misc-fakes";
import { IOFake } from "../fakes/io-fake";
import { execInWorkspace } from "./exec-in-workspace";
import { workspace, type Workspace } from "./workspace";

const SPEC = { repo: "owner/repo", sha: "abc123", install: false } as const;

const build = (workspaceLosses: Record<string, number>) =>
makeSandboxFake({}, {}, {}, workspaceLosses);


describe("execInWorkspace", () => {
it.effect("re-clones and runs again when the container lost the workspace", () => {
const { layer, state } = build({ "cargo test": 1 });
return Effect.gen(function* () {
const ws = yield* workspace({ ...SPEC });
expect(state.clones).toHaveLength(1);

const result = yield* execInWorkspace(ws, { command: "cargo test" });

expect(result.exitCode).toBe(0);
// The rebuild is the point: a second clone, into the SAME container.
expect(state.clones).toHaveLength(2);
expect(state.clones[1]).toEqual({ repo: SPEC.repo, sha: SPEC.sha });
expect(state.acquired).toHaveLength(1);
}).pipe(Effect.provide(Layer.mergeAll(layer, CacheFake, IOFake)));
});

it.effect("gives up after one rebuild rather than looping on a dying container", () => {
const { layer, state } = build({ "cargo test": 5 });
return Effect.gen(function* () {
const ws = yield* workspace({ ...SPEC });
const exit = yield* Effect.exit(execInWorkspace(ws, { command: "cargo test" }));

expect(Exit.isFailure(exit)).toBe(true);
expect(state.clones).toHaveLength(2);
}).pipe(Effect.provide(Layer.mergeAll(layer, CacheFake, IOFake)));
});

it.effect("does not re-clone for an ordinary command failure", () => {
const { layer, state } = makeSandboxFake({
"cargo test": { fail: "ExecFailed", exitCode: 101, stderrTail: "tests failed" },
});
return Effect.gen(function* () {
const ws = yield* workspace({ ...SPEC });
const exit = yield* Effect.exit(execInWorkspace(ws, { command: "cargo test" }));

expect(Exit.isFailure(exit)).toBe(true);
expect(state.clones).toHaveLength(1);
}).pipe(Effect.provide(Layer.mergeAll(layer, CacheFake, IOFake)));
});

it.effect("a red suite is a result, never a rebuild", () => {
const { layer, state } = makeSandboxFake({ "cargo test": { exitCode: 1 } });
return Effect.gen(function* () {
const ws = yield* workspace({ ...SPEC });
const result = yield* execInWorkspace(ws, { command: "cargo test" });

expect(result.exitCode).toBe(1);
expect(state.clones).toHaveLength(1);
}).pipe(Effect.provide(Layer.mergeAll(layer, CacheFake, IOFake)));
});

it.effect("re-runs the install when the workspace spec asked for one", () => {
const { layer, state } = build({ "cargo test": 1 });
return Effect.gen(function* () {
const ws = yield* workspace({ ...SPEC, install: true });
const clonesAfterCheckout = state.clones.length;

yield* execInWorkspace(ws, { command: "cargo test" });

expect(state.clones).toHaveLength(clonesAfterCheckout + 1);
}).pipe(Effect.provide(Layer.mergeAll(layer, CacheFake, IOFake)));
});

it("carries workspaceMissing on the error the runtime raises", () => {
const err = new ExecFailed({ exitCode: -1, stderrTail: "gone", workspaceMissing: true });
expect(err.workspaceMissing).toBe(true);
expect(new ExecFailed({ exitCode: 1, stderrTail: "x" }).workspaceMissing).toBeUndefined();
});

it.effect("threads the command options through to the sandbox", () => {
const { layer, state } = build({});
return Effect.gen(function* () {
const ws: Workspace = yield* workspace({ ...SPEC });
yield* execInWorkspace(ws, {
command: "cargo test",
env: { CI: "1" },
timeoutSec: 900,
});

const exec = state.execs.find((e) => e.command === "cargo test");
expect(exec?.cwd).toBe(ws.dir);
expect(exec?.env).toEqual({ CI: "1" });
expect(exec?.timeoutSec).toBe(900);
}).pipe(Effect.provide(Layer.mergeAll(layer, CacheFake, IOFake)));
});
});
43 changes: 43 additions & 0 deletions packages/core/src/primitives/exec-in-workspace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Primitive: execInWorkspace — run a command against a workspace, rebuilding
// the checkout first if the container lost it.
//
// Container disk does not survive a restart, so a memoized checkout can point
// at a directory that is gone. specs/adr/0001-cloudflare-workflows-scope.md
// rule 3.

import { Effect } from "effect";
import type { ExecFailed, ExecTimeout } from "../errors";
import { sandbox, type ExecResult } from "../services/sandbox";
import { hydrateWorkspace, type Workspace } from "./workspace";

export type ExecInWorkspaceOpts = {
readonly command: string | readonly string[];
readonly env?: Record<string, string>;
readonly timeoutSec?: number;
readonly redactValues?: readonly string[];
};

export const execInWorkspace = (
ws: Workspace,
opts: ExecInWorkspaceOpts,
): Effect.Effect<
ExecResult,
ExecFailed | ExecTimeout | Effect.Effect.Error<ReturnType<typeof hydrateWorkspace>>,
Effect.Effect.Context<ReturnType<typeof hydrateWorkspace>>
> =>
Effect.gen(function* () {
const run = (dir: string) =>
sandbox.exec({ ...opts, cwd: dir, container: ws.container });

const first = yield* Effect.either(run(ws.dir));
if (first._tag === "Right") return first.right;
if (first.left._tag !== "ExecFailed" || first.left.workspaceMissing !== true) {
return yield* Effect.fail(first.left);
}

yield* Effect.logWarning(
`workspace ${ws.dir} was gone at exec time — rebuilding the checkout and running again`,
);
const dir = yield* hydrateWorkspace(ws.container, ws.spec);
return yield* run(dir);
});
3 changes: 2 additions & 1 deletion packages/core/src/primitives/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
//
// See specs/03-dsl.md § Primitives and ./README.md.

export { workspace, type Workspace } from "./workspace";
export { workspace, hydrateWorkspace, type Workspace, type WorkspaceSpec } from "./workspace";
export { execInWorkspace, type ExecInWorkspaceOpts } from "./exec-in-workspace";
export { installCached } from "./install-cached";
export { sharded, type Shard } from "./sharded";
export { fanOut, type FanOutShard } from "./fan-out";
Expand Down
Loading
Loading