Skip to content

Synchronize integration worktree edits with canonical DO workspaces - #774

Draft
deregtd wants to merge 8 commits into
ddr/split-replica-prerequisitesfrom
ddr/split-worktree-replicas
Draft

deregtd wants to merge 8 commits into
ddr/split-replica-prerequisitesfrom
ddr/split-worktree-replicas

Conversation

@deregtd

@deregtd deregtd commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

User scenario

An agent is editing a Git-backed workspace in its DO, then runs a build or test command in a Linux container. Both sides may continue editing. Container edits must become ordinary canonical workspace changes, DO edits must reach the running container, and acknowledged changes must survive a disconnected bridge or a DO reset.

When a container is restored from a snapshot, it needs to catch up from a known point rather than become the new owner of an old workspace copy. A local commit made in the DO must also appear as the same real Git commit in the container.

The existing DO worktree/file tools do not provide an external integration with that checkpoint, replay, submission, and durable-acknowledgement protocol. This PR adds that generic protocol; it does not implement the container itself.

What changes

  1. An explicitly granted integration acquires a scoped, expiring replica lease and an immutable checkpoint: selected worktrees, source/base identities, local Git heads, pending OTs, and a canonical replay cursor.
  2. Base files and local commit objects stream as raw binary. The provider bootstrap from Let integrations fetch authorized Git commits without receiving credentials #769 can fetch the upstream base efficiently; DO-only commits are exported as actual Git objects, not synthesized file replacements.
  3. While both sides operate, the integration reads ordered DO changes and submits bounded text OTs anchored to the canonical cursor and file digest. The kernel uses the existing OT engine, durably commits the accepted edits and receipt together, and returns any corrections.
  4. Identical submission retries return the same receipt; an ID reused for different content is rejected. Replay cursors, bounded pages, lease checks, and retained journal history let clients reconnect and catch up without pretending an unknown outcome succeeded.
  5. Model-step saves and concurrent replica submissions share a serialization boundary. An acknowledgement cannot lose edits because the other writer drained an in-flight buffer. Both entry paths enforce the same ordinary file-tool permissions.

The DO remains canonical throughout. Ordinary agent file operations stay ordinary DO operations; there is no exclusive container ownership mode or runtime stop-the-world synchronization policy here.

Kernel/gatekeeper boundary

The kernel owns canonical worktrees, OT ordering, authority, and durable receipts. The integration owns filesystem-change detection and OT calculation, initial hydration/catch-up policy, container images, FUSE/native engines, snapshots, commands, and network approvals. Those Linux-specific pieces remain in gadgets-internal.

This revision transfers binary base files but accepts text mutations only. Whole-file binary replica mutations remain the separate #759 follow-up; this PR does not replace text deltas with full-file changes or add binary diffs.

Dependencies and merge order

This draft targets ddr/split-replica-prerequisites, a merge-only review base, so prerequisite changes do not drown out this diff:

The prerequisite PRs target main independently. Do not merge this PR into the temporary base as a release step. Reconcile the final prerequisite heads, then rebase/retarget this PR onto main after they land. In particular, this review base pins #770's initial text-attachment contribution; #770 has since expanded to all file formats and that newer foundation still needs integration during restacking.

Verification and preservation

This reorganization leaves production source/configuration byte-identical to the previous #774 head (82186a4d). It moves the persistence queue and shared permission extraction into this PR's diff, while #776 owns the generic timing instrumentation. The timing test is relocated, not dropped or duplicated.

Full lint, generated-config checks, and workspace build/type checks passed. The initial broad backend run passed 1,550 tests but hit a 20-second transfer-test timeout; that transfer suite then passed all 3 tests in isolation. The bounded-concurrency rerun passed 1,547 tests / 81 files but exited with one workerd-pool runner startup timeout before all files ran, so neither broad run is claimed fully green. The relocated timing/validation suites also passed all 3 tests in isolation. The five previously reproduced failures in unchanged main's ai-models suite are excluded from these runs. Prior backend integration verification passed 3 tests, with 4 intentionally skipped.

Extracted from #729. No deployment, feature enablement, new container performance run, or exactly-once promise for unjournalled shell effects.

@github-actions github-actions Bot added kernel Changes to the Workshop kernel workshop/shared Changes to shared Workshop APIs labels Oct 14, 2026
@github-actions

Copy link
Copy Markdown

Preview: pr774-ddr-split-wor-e9b16081

https://pr774-ddr-split-wor-e9b16081-router.cloudflare-os-previews.workers.dev

Dashboard · deleted when this PR closes

Comment on lines +140 to +142
if (!await this.host.flush(lease)) continue;
const version = this.host.version(lease);
const remote = this.#changes(lease, batch.baseCursor, batch.files);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P1] Fence the revision actually persisted by the preliminary flush

A file tool can buffer another edit after flushReplica() drains stepBuffer, while its commitAgentStep() is awaiting prefetches. Sampling version() afterwards includes that newer in-memory revision, but #changes() includes only durable rows, so the final version check passes without reconciling the buffered edit. For example, with a.txt = abc, a buffered tool edit c → X and a replica edit a → R produce a receipt whose accepted hash is for Rbc and whose corrections are empty, while the live barrier actually persists RbX. Positional collisions also bypass the normal OT mapping. Return/fence the revision drained by the flush and retry if additional edits were buffered before sampling the canonical changes.

},
};

hooks.registerWorktreeReplicaTurn?.(chatId, worktreeTurnAccess, author, () => chatBindings);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P1] Reconcile journal writes before replacing the live pass

During an automatic-compaction reload or transient-model retry, runAgent() snapshots loadChatHistory() before the new pass awaits prepareChatBindings() and history replay. The previous pass remains in #worktreeReplicaTurns until this registration replaces it. A submitBatch() during those awaits can therefore update the previous pass and ACK a durable edit that is absent from the new pass's history snapshot. This line then publishes a live file view missing that acknowledged edit; subsequent model reads and edits use stale content and can overwrite the edit or apply an OT at the wrong positions. Fence the pass handoff and revalidate/replay journal additions before publishing the replacement access. A regression should submit a batch while the second pass's replay is held behind an await and verify the replacement file view includes it.

@ask-bonk

ask-bonk Bot commented Oct 14, 2026

Copy link
Copy Markdown

Review: 2 findings.

Posted both high-priority findings inline. Reviewed against a65e4257.

github run

@ask-bonk

ask-bonk Bot commented Oct 14, 2026

Copy link
Copy Markdown

Since last review: 0 resolved, 2 still open, 0 new.

Verified both previously posted P1 findings still apply at 82186a4d. The current tree has no net file changes from the last reviewed head, a65e4257, and both review threads remain open.

github run

@github-actions github-actions Bot deleted a comment from ask-bonk Bot Oct 14, 2026
content = applyCodeChange(content, {[workpieceId]: entries});
}
for (let {workpieceId, entries, pin} of prepared) {
appendAgentEdit(workpieceId, {[workpieceId]: entries}, pin);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P1] Fence in-flight gadget tools against replica pinning

A replica can establish and persist a gadget's first pin here while an ordinary writeFile/editFile is awaiting its own first-write readCommitFiles(head) or anchorKnowledgeToPin(). The tool retains the pin prepared when the gadget was unpinned; when it resumes, appendAgentEdit(..., pin) resets the entire gadget's session content to pin.baseFiles, discarding acknowledged replica changes to other paths. I reproduced this with the current agent functions: pause writeFile(a.txt) at its base read, accept and flush a replica write to b.txt, then resume the tool. Durable b.txt retains the replica write, but the live file view returns the old base content. Subsequent model reads and edits therefore use stale content. Revalidate the pin/revision after the tool's awaited setup, or serialize that setup with replica ingestion, so an already-pinned gadget's overlay cannot be reset by the in-flight tool.

@ask-bonk

ask-bonk Bot commented Oct 14, 2026

Copy link
Copy Markdown

Since last review: 0 resolved, 2 still open, 1 new.

Both existing P1 findings still apply. Posted one additional P1 finding inline, verified with a deterministic reproduction using the current agent functions.

Reviewed against d5c877a8 and the stacked review base.

github run

@ask-bonk

ask-bonk Bot commented Oct 14, 2026

Copy link
Copy Markdown

Since last review: 0 resolved, 3 still open, 0 new.

Verified all three previously posted P1 findings still apply at d5c877a8, and their review threads remain open. The head SHA is unchanged from the latest review.

github run

@github-actions github-actions Bot deleted a comment from ask-bonk Bot Oct 14, 2026
@deregtd deregtd changed the title Add canonical worktree replicas and accepted OT journals Synchronize integration worktree edits with canonical DO workspaces Oct 14, 2026

@deregtd deregtd left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed the full 30-file diff at this revision. One additional reproducible storage regression is inline; the three existing concurrency findings remain reproducible.

} else {
const combined = composeCodeChange({[id]: [[path, parts.at(-1)!]]}, incoming);
if (codeChangeSerializedSize(combined) <= MAX_CODE_CHANGE_SIZE) {
parts[parts.length - 1] = combined[id][0][1];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[P2] Split accepted prefixes at a storage-safe record size

The 8_000_000 change budget lets this compose a prefix larger than SQLite KV can store. In an ordinary worktree with no replica grant, I accepted a new file containing 600,000 界 characters, then appended and accepted another 600,000 characters per epoch. Each individual edit is about 1.2 MB serialized, but the fourth accept tries to store the cumulative 2.4-million-code-unit prefix here and fails with SQLITE_TOOBIG, leaving the proposed changes unmergeable. The identical eight-accept Workers probe passes on base 21984f5; this head fails on accept four, and both new cumulative-prefix regression cases also fail. Please split prefix records using a storage-safe byte bound, including record framing, rather than the larger CodeChange budget.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Verified fixed at ef76e2dbc16908ae14ad209be3aa53b2dd74cf6b. I reran both cumulative-prefix regression cases against the current source in workerd after generating the browser runtime/bundled blueprints and rebuilding typed-storage. Both pass: eight ordinary accepts and the single-epoch variant retain the full file after DO eviction, with the prefix split into two bounded records.

My previous re-review metadata had carried forward an older failing result; this fresh run supersedes it for this finding. I’m clearing the storage-size issue. The separate replica concurrency and early-flush replay findings still need resolution. This used local test storage and installed dependencies, with no live provider/container/deployment test.

Comment on lines +3042 to +3045
if (await hooks.commitAgentStep(chatId, author, [], {
changes, createdGadgets, createdWorktrees, addedBindings, worktreeCommits,
blueprintMerges, replicaCommit,
})) ++nextChangeId;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P2] Record which tool edits an early replica flush has already covered

This saves ordinary buffered file-tool edits with msgs = [], so their changes message can precede the assistant message containing those tool calls. On history replay, the editFile branch then adds the already-applied edit to pendingReplayEdits; if the normal step barrier has no remaining writes, it emits no trailing changes message to discharge it. A subsequent session-served readFile therefore applies that edit again: starting with abc, editing a → aX, flushing through the replica, and then reading produces aXbc live but aXXbc on replay (or a replay error when the old match no longer exists). Preserve the association between an early flush and its tool effects so replay does not apply them twice, and cover replay after a mid-step flush in a regression.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed on ef76e2dbc16908ae14ad209be3aa53b2dd74cf6b with an independent workerd probe using the real agent file tools and replica checkpoint path. Within one assistant step: read abc, edit a → aX, let a replica checkpoint() run during the next asynchronous source read, then read the edited file. The live result is aXbc; a fresh runAgent() reconstructs that historical read as aXXbc, while the persisted file remains aXbc.

A checkpoint alone can therefore trigger this, without a replica write or a DO reset. Please include this mid-step checkpoint/replay sequence in the regression for the fix. The probe passed its assertions demonstrating the discrepancy; it used a scripted model and no live provider or container.

@ask-bonk

ask-bonk Bot commented Oct 15, 2026

Copy link
Copy Markdown

Since last review: 0 resolved, 3 still open, 1 new.

The three existing P1 findings still apply. Posted one additional P2 finding inline. The separately reported accepted-prefix storage issue also remains present.

Reviewed ef76e2db against the stacked base.

github run

@ask-bonk

ask-bonk Bot commented Oct 15, 2026

Copy link
Copy Markdown

Since last review: 0 resolved, 5 still open, 0 new.

Reviewed ef76e2db against the stacked base. The three existing P1 concurrency findings and both P2 storage/replay findings still apply. No additional actionable issues found; no duplicate inline comments posted.

Static review only; tests were not run because dependencies are not installed.

github run

@github-actions

Copy link
Copy Markdown

Eval results

Verdict: ⚪ Unchanged. No task moved beyond what 10 runs can tell apart from noise.

Task Score Δ score Fisher test Cache hits Avg min Avg steps
change-calendar 90% → 100% +10 pp p = 1.00 94% → 95%
+1 pp
4.5 → 4.7 24.6 → 28.7
chess 80% → 70% −10 pp p = 1.00 98%
0 pp
10.3 → 8.2 53.6 → 53.3
incident-desk 90% 0 pp p = 1.00 96%
0 pp
5.2 → 5.1 30.2 → 34.2
worker-logs 100% → 80% −20 pp p = 0.47 95%
0 pp
5.1 → 3.7 23.8 → 21.5
Failed checks
Task Check Failed
change-calendar t3 existing-windows-survive-the-rule-change 1/10 → 0/10
change-calendar t3 same-service-windows-must-be-a-day-apart 1/10 → 0/10
change-calendar t3 billing-is-capped-at-four-hours-others-are-not 1/10 → 0/10
chess t1 agrees-with-the-oracle-on-the-hard-positions 1/10
chess t1 agrees-with-the-oracle-on-perft-positions 1/10
chess t1 agrees-with-the-oracle-through-random-games 1/10 → 2/10
chess t1 starts-from-the-standard-position 0/10 → 1/10
chess t1 the-game-is-shared-across-connections 0/10 → 1/10
chess t2 imports-known-games-to-the-right-positions 1/9 → 1/8
chess t2 refuses-invalid-pgn-without-changing-the-game 1/9 → 0/8
chess t2 exports-pgn-the-oracle-replays-to-the-same-position 1/9 → 1/8
incident-desk t1 opens-acknowledges-and-resolves-in-order 1/10
incident-desk t1 simultaneous-acknowledges-yield-exactly-one-owner 1/10
incident-desk t1 simultaneous-opens-of-one-id-admit-exactly-one 1/10
incident-desk t1 board-lists-by-severity-then-age 1/10 → 0/10
worker-logs t1 ingests-resets-and-summarises-per-worker 0/10 → 2/10
worker-logs t1 hourly-buckets-cover-every-hour-including-empty-ones 0/10 → 2/10
worker-logs t1 ranges-are-half-open-and-filter-by-worker 0/10 → 2/10

Run · trajectories and raw results

@github-actions github-actions Bot deleted a comment from ask-bonk Bot Oct 15, 2026
@ask-bonk

ask-bonk Bot commented Oct 15, 2026

Copy link
Copy Markdown

🔬 Eval runs review

Performance

With 10 comparable runs per task, main → PR pass rates were change-calendar 90% → 100%, chess 80% → 70%, incident-desk 90% → 90%, and worker-logs 100% → 80%; none moved beyond noise. Mean costs were mixed: calendar $0.0203 → $0.0229, incidents $0.0228 → $0.0246, chess $0.0506 → $0.0455, and logs $0.0202 → $0.0167; shorter chess/logs times partly reflect early failures, while calendar’s significant cache-hit gain accompanied more steps rather than a changed prompt. Chess lost the most candidate runs, 3/10, and produced the largest detour: trial 4 passed after 120 steps, 14 tool errors and $0.0971, versus a baseline maximum of 81 steps and $0.0735, with repeated unmatched regex edits consuming much of the extra work.

⚪ VERDICT: NO REGRESSION FROM THIS PR

The diff adds replica synchronization and serialized persistence barriers, but the observed failures arise in agent-written gadget code under unchanged prompts and tool contracts, with no attributable performance improvement or regression established.

Triage

Failure modes

  • Oversized SQL inserts · worker-logs 2/10 · system prompt · this PR: no — Turn 1 writeFile(server.js) in trials 4 and 6 batches 100 × 7 and 500 × 6 bindings into individual statements, exceeding the runtime’s SQL-variable limit and leaving later summaries empty; the unchanged system prompt advertises regular storage APIs without explaining this limit. Trial 6’s executeCode smoke test used only four events, whereas passing main and candidate implementations used per-row inserts.
  • Broken chess position handling · chess 2/10 · model error · this PR: no — Turn 1 writeFile(server.js) in trial 6 uses incorrect castling-rook colors and mixes numeric move coordinates with algebraic-string parsing; trial 8 checks the en-passant origin at an incorrect ±16 offset, making subsequently saved positions unparsable. The resulting null/undefined accesses stop play; main also had a rules-engine failure, although its defect was incorrect sliding-piece attack detection.
  • PGN result tokens corrupted · chess 1/10 · model error · this PR: no — Trial 9’s turn 2 editFile adds pgnTokens(), which replaces every 0 with O before loadPgn() recognizes result tokens, corrupting 1-0 and 0-1. Valid imports are refused and exports retain the previous game; repeated failed exact-text repairs add detours, and main also failed one run on PGN handling.
  • Successful incident inserts reported as duplicates · incident-desk 1/10 · model error · this PR: no — Trial 8’s turn 1 writeFile(server.js) makes open() return success only when rowsWritten === 1; the checks show newly inserted incidents exist despite DUPLICATE_ID replies. This is not leftover test data: the transcript contains no test execution, and the final reply explicitly says none was created.

Tool errors

  • editFile: No matching text was found in the file. · change-calendar 1 → 0, chess 8 → 26, incident-desk 3 → 0, worker-logs 2 → 3 · model error — Agents supplied incorrect whitespace or escaping; candidate chess trials 4 and 9 repeatedly retried the same nonexistent regex line, while main also needed escaping repairs.
  • editFile: Validation failed for tool "editFile": · change-calendar 1 → 3, chess 2 → 2, incident-desk 5 → 5, worker-logs 3 → 2 · model error — Calls omitted required fields such as filename or replacement, or used a nested tool-call object instead of the declared arguments; corrected calls generally recovered.
  • editFile: Multiple matches were found. The text to match must be unique. · incident-desk 4 → 2 · model error — Agents targeted repeated fragments such as this.broadcast(); or shared SQL select lists; adding surrounding method/query context resolved the ambiguity.
  • editFile: CHESS has no file named "CHESS". · chess 0 → 2 · model error — Candidate trials 1 and 4 supplied the gadget binding as filename instead of server.js; the binding and filename are separate tool parameters.
  • executeCode: Failed to start Worker: · chess 1 → 0, worker-logs 0 → 1 · model error — Main chess trial 3 submitted .2, lacking the required default export; candidate logs trial 3 submitted ., which is invalid JavaScript. Both subsequently retried with complete modules.

Prompt cache

  • change-calendar · cache hits 94.31% → 95.30% · breaks 0.0104% → 0.0099% · this PR: no — Hit-rate p = 0.00049, but break-rate p = 0.123. The first model requests in baseline trials 1–2 read zero cached tokens and wrote about 7,880; candidate first requests read 7,259 and wrote about 620, before any gadget existed. No prompt-content change explains that initial warmth, and subsequent prefixes remain cached on both sides; the increase from 24.6 to 28.7 mean steps also increases the cached share.

What to do

  • Optional: in packages/workshop-backend/src/agent.ts, add storage guidance recommending parameter-count-bounded SQL batches and an INSERT ... RETURNING example for detecting successful insertion, then smoke-test representative batch sizes.
  • Optional: in the same file’s EDIT_FILE_TOOL_DESCRIPTION, add: “After a no-match error, re-read or grep the current text and correct the match; do not repeat an unchanged failed edit.” These evals do not indicate a required replica change.

github run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kernel Changes to the Workshop kernel workshop/shared Changes to shared Workshop APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant