Skip to content

feat(spike): on-call demo extensions on the durable host (P4a) - #431

Merged
bubbleptr merged 1 commit into
feat/durable-multiview-pacefrom
feat/durable-multiview-demo
Oct 2, 2026
Merged

bubbleptr merged 1 commit into
feat/durable-multiview-pacefrom
feat/durable-multiview-demo

Conversation

@bubbleptr

@bubbleptr bubbleptr commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

No linked issue: fifth stage (P4a) of the Pi Durable multi-client spike (.scratch/durable-multiview/HANDOFF.md §14), stacked on #430. P4 is split in two PRs. This one adds the host-side demo extensions and the protocol they need. The three clients' UI for approvals, the plan, forks, thinking blocks, the narrow layout, and usage comes in P4b, along with a full scenario run. Spike-only: nothing under apps/ or packages/ changes.

Summary

  • Demo extensions (spikes/durable-multiview/host/demo/, node host/main.ts --demo on the real model, --faux-demo on a scripted model). All tools are mocks; no CodingTools or bash.
    • oncall (main and its forks only): a playbook section that fixes the scenario's order, and a plan section that shows the demo.plan document to the model. Tools:
      • update_plan writes the plan.
      • subagent follows upstream example 22 (replay: "safe"). Each child is offered only its area's investigation tool.
      • rollback and schedule_check.
    • Approval hook: beforeTool holds rollback until a client decides. It reads the memo first, then the demo.approvals document, and only then waits on an in-memory ApprovalBoard. A decision is committed with decisions[id] ??= ..., so the first click wins, and is then memoized. The wait is cancelled with the hook's context (Esc).
    • Rollback task: demo.rollback creates three region child tasks and waits on them with failFast. eu-west always fails its health check, which aborts the other regions mid-switch. Each aborted region restores its traffic in its own abort handler (bottom-up). Progress goes to the demo.rollout document, which the tool streams as running output. The tool is replay-safe: it scans for a rollback it already owns before creating one.
    • Reminder: schedule_check creates a background demo.reminder task. It sleeps until an absolute time kept in its input, then posts a follow-up with requestId: reminder:<taskId>.
    • Hot reload: the investigation tools load by path. Demo.reload() re-imports the module with ?v=N and reinstalls it under the same name. With --demo, the host watches the file and reloads on save.
    • faux-script.ts: a scripted model for tests. Requests from main and the parallel subagents interleave, so each answer depends only on the request itself: the tools it offers and the messages since the newest user input.
  • Gateway and protocol:
    • New streams: approvals, and doc:<kind>:<conversationId> for extension documents (listed in hello.docs). A document that doesn't exist yet streams null until the commit that creates it.
    • New calls: approve and fork. fork takes removeTools, matched by name against the parent's offered tools.
    • The conversation list labels forks fork N.
    • DurableView gains docs and approvals. DurableController gains approve(approval, approved) and fork(entryId, prompt, removeTools?). clientName is recorded as the decider.
  • openHost gains approvals, docs, onOpen, and notify.

Test plan

  • Spike bun run typecheck
  • Spike bun run test: 35 tests. New in demo.test.ts, on the scripted model and an in-process host:
    • The plan syncs, three subagents run at once, and each child is offered only its own tool.
    • Two clients see one approval. The first decision wins, and the later click is told who decided. The rollback ends with eu-west failed and the other two regions compensated.
    • After approving and restarting the host mid-rollback, nobody is asked again and there is still one rollback.
    • A scheduled check survives Esc and a restart, and arrives once.
    • A fork at an answer drops rollback, keeps the plan as of the fork, and is listed as fork N.
    • After a hot reload, the next tool output uses the new format.
  • crash-recovery.test.ts adds a kill -9 of the host process while all three subagents run their tools. After the restart the same three child conversations finish the investigation.
  • Real-model smoke on radius/deepseek-v4.1-flash with --demo:
    • The model wrote the plan and delegated to three parallel subagents, then recommended a rollback.
    • The rollback asked for approval and ran with eu-west failed and the other two regions compensated.
    • On that run the model skipped the follow-up check after the failed rollback, so the playbook now asks for one either way.
  • Root bun run typecheck, bun run lint, bun run test (168 files, 1975 tests), and bun run build

Checklist

  • Linked issue or a short reason this has none
  • Title follows Conventional Commits
  • bun run typecheck, bun run test, and bun run build are green
  • ADR added or updated if an architectural boundary or product term changed (none; findings go to docs/research/ in P5)
  • CONTEXT.md updated if vocabulary changed (none changed)
  • New or changed shared/ui/ components are registered on /design (none)
  • Branch is feat/; this is not a push to main

Subagents, an approval hook, a failFast rollback with compensation, a
background reminder, forks, and hot-reloadable investigation tools, with
approval and document streams on the gateway.
@bubbleptr
bubbleptr added this pull request to stack #428 October 2, 2026 11:39

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 6 potential issues.

Devin Review

Comment on lines +274 to +279
const existing = await harness.watchDoc(token, id, context);
if (existing !== undefined) return this.#forward(stream, existing);
await this.send({ type: "snapshot", stream, value: null });
let inner: Subscription | undefined;
let stopped = false;
const unsubscribe = harness.subscribeCommits((publication) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 New documents remain permanently invisible

When a document appears between watchDoc and subscribeCommits, the gateway misses its creation and keeps streaming null. Clients never see that document until they resubscribe.

Learn more

A document stream first tries to watch an existing document. If that returns undefined, the gateway sends null and only then registers for commit publications. A concurrent update_plan commit in that interval creates the document before the listener exists. The stream never installs a watch, so later updates cannot reach the client either.

Example: A client subscribes to doc:demo.plan:1 while the first update_plan runs. The document is absent when watchDoc checks; the plan is committed before subscribeCommits starts; the client stays on null even after the plan is updated again.

Recommended fix: Register the commit listener before checking for an existing document, then recheck and attach exactly one watch. Coordinate the listener and check so a commit during either step cannot be lost or attach duplicate watches.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +81 to +82
if (tools.includes("update_plan") && !messages.some((message) => message.role === "toolResult" && message.toolName === "update_plan")) {
return calls(call("update_plan", { items: PLAN.map((text) => ({ text, status: "todo" })) }));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Later investigations skip the demo playbook

After one investigation, oncallScript finds its old update_plan result and skips planning for later requests. A second investigation gets only Noted instead of running the investigation.

Learn more

The scripted model selects the latest user message at userAt, and last already considers only messages after it. The plan guard instead searches all messages, including earlier turns. Once an update_plan result exists, an unrelated new user prompt cannot start another scripted investigation.

Example: The user asks to investigate v2.3, receives a recommendation, then asks to investigate the release again. The second request contains the old update_plan result, so the faux model responds Noted: without delegating.

Recommended fix: Scope the update_plan result check to messages.slice(userAt + 1), as the other per-turn decisions are scoped.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

this.#waiters.delete(id);
for (const waiter of waiters) waiter.resolve(decision);
if (waiters.length > 0) this.#changed();
return { decision, first: decision.by === candidate.by && decision.at === candidate.at };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Losing approval reports a winning decision

When the same client name votes twice within one millisecond, first can be true for both calls. The later vote is ignored, but its caller sees a success notice.

Learn more

The transaction persists only the first decision, but the returned first flag is inferred from by and at. These are not unique: client names default to client, and two calls can share a millisecond. The second request can therefore report that it won when the persisted decision was different.

Example: Two controls named client approve and deny the same call at timestamp 1000. If approval commits first, the denial receives {decision: approved, first: true} and reports You approved instead of Already approved by client.

Recommended fix: Compute a boolean inside the transaction when the decision is inserted, and return that boolean alongside the stored decision.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

if (demo !== undefined) {
// Editors save in bursts; reload once the burst settles.
let timer: NodeJS.Timeout | undefined;
const watcher = watch(demo.investigationModule, () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Atomic saves stop investigation hot reload

When an editor saves by replacing the file, watch remains attached to the old inode on Linux. Later edits no longer reload the investigation tools.

Learn more

The demo watches the investigation module as an individual file. On Linux, an atomic-save editor writes a replacement and renames it over the watched path; the watch remains on the replaced inode. The first edit can trigger a reload, but subsequent edits to the new file no longer trigger the watcher.

Example: An editor atomically changes FORMAT to table, then atomically changes it back to plain. The first save can reload, but the second save is invisible to this watcher, leaving the host with table output.

Recommended fix: Watch the containing directory and filter events by the module's basename, or re-establish the file watch after a rename. Debounce the filtered events as now.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +67 to +70
const decision = await harness.commit(async (tx) => {
const doc = await tx.doc(ApprovalsDoc, conversationId);
if (doc.decisions[id] === undefined && !this.#waiters.has(id)) throw new Error(`No pending approval ${id}`);
doc.decisions[id] ??= { ...candidate };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟥 Approval decision bypasses conversation binding

A client can send an approval ID with a different conversation ID. ApprovalBoard checks pending IDs globally, then records the decision in the supplied conversation while resolving the original waiter. The decision does not survive replay of the original conversation.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +377 to +379
const { approvalId, approved, by } = args as CallMethods["approve"]["args"];
if (this.#options.approvals === undefined) throw new Error("This host has no approvals");
return this.#options.approvals.decide(conversation.id, approvalId, approved, by);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟨 Approval audit identity is client-controlled

An authenticated client can send any by value with an approval. The server persists that value as the decision maker, so approval records can misattribute who acted.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

@bubbleptr
bubbleptr merged commit 7bfcc16 into main Oct 2, 2026
2 checks passed
@bubbleptr
bubbleptr deleted the feat/durable-multiview-demo branch October 2, 2026 14:22
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.

1 participant