Skip to content

feat: webmux oneshot + Linear round-trip, server-driven watcher - #238

Merged
centdix merged 36 commits into
mainfrom
feat/oneshot-and-on-merge-action
May 18, 2026
Merged

feat: webmux oneshot + Linear round-trip, server-driven watcher#238
centdix merged 36 commits into
mainfrom
feat/oneshot-and-on-merge-action

Conversation

@hugocasa

Copy link
Copy Markdown
Contributor

Summary

This branch lands the webmux oneshot command and a server-driven oneshot watcher that auto-closes the session and posts the agent's conversation back to the linked Linear issue when the run completes. It also adds a Linear _oneshot label trigger so a labeled Todo issue spins up an autonomous run end-to-end.

Re-opening after closing #230 for a critical self-review + dedup + one more thorough review round (see commit history for the receipts).

Headline behaviors

  • webmux oneshot — autonomous agent run on a fresh worktree, exits when terminal (PR opened = success, idle without PR = stuck). One --linear flag with auto-detect.
  • Server-driven end-of-run — auto-close + Linear post-back live in a oneshot-watcher-service on the backend, not in the CLI. The CLI is a thin streamer that exits cleanly when the server disarms.
  • Disarm on browser interaction — any user action in the dashboard (chat, terminal input, send-prompt, upload, open/close/archive/merge) clears the armed meta so the human takeover always wins over the watcher.
  • _oneshot label — Linear issues tagged _oneshot trigger an autonomous run; _oneshot wins over webmux when both are present. Documented in the README.

Architecture notes

  • New OneshotConfig wire shape on CreateWorktree + OpenWorktree arms the watcher. WorktreeMeta.oneshot persists on disk; ManagedWorktreeRuntimeState.oneshot + WorktreeSnapshot.oneshot mirror it so the CLI sees disarm without a reconcile round-trip.
  • Watcher rechecks meta immediately before closeWorktree to close a race where the user disarms during the Linear post-back HTTP call.
  • Terminal WebSocket disarms on the first input/sendKeys per connection (avoids per-keystroke disk reads).
  • agentLifecycle === "closed" is terminal, so a crash or server restart can't leave meta armed forever.

Test plan

  • bun run --cwd backend test — 303 pass / 0 fail
  • bun test bin/src — 107 pass / 0 fail
  • bun test packages/api-contract/src — 4 pass / 0 fail
  • bun run --cwd frontend test — 73 pass / 0 fail (vitest)
  • bun run --cwd backend check (tsc) — clean
  • bun run --cwd frontend check (svelte-check) — clean
  • Manual: arm a oneshot run from CLI, type into the terminal pane before idle-grace fires — confirm watcher disarms and the run is not auto-closed.
  • Manual: arm a oneshot run with --linear and let it go idle — confirm the conversation is posted to the Linear issue and the session auto-closes.
  • Manual: tag a Linear Todo with _oneshot — confirm the auto-create poller spins up an autonomous run.

Review trail

This branch went through eight rounds of review on #230 before being closed for a critical self-review + dedup + one final review round (see the refactor: address PR review #N commits). The reviewers ran by ultrareview caught: a HIGH-severity watcher gating bug (silently disabled by an isActive predicate, fixed in self-review), missing terminal-input disarm path (M1), a postToLinear/close race (M2), undocumented _oneshot label (M3), and 8 LOW findings — all addressed in the final commit.

🤖 Generated with Claude Code

hugocasa and others added 30 commits May 8, 2026 18:41
Adds `webmux oneshot` for start-to-finish runs that stream the agent
conversation to stdout without changing tmux focus, plus per-worktree
`onMergeAction` ("close" | "remove" | null) configurable via CLI flags
(`--close-on-merge` / `--remove-on-merge`) and a frontend toggle in the
worktree row menu. `oneshot --resume <branch>` re-attaches to a stuck
session and can send a follow-up prompt.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a Linear-backed handoff path so a webmux session can be picked up
on another machine (or by someone else):

- oneshot --post-to-linear <issue-id|team-key>: when the run ends,
  upload the conversation as a JSON attachment + summary comment. Team
  keys auto-create a new issue first.
- oneshot/add --resume-from-linear <issue-id>: resolve a branch and
  conversation context from the issue's webmux attachment (preferred)
  or its linked GitHub PR, then create the worktree. --branch overrides
  the resolved branch.
- New CLI: webmux linear post <branch> <issue-or-team>.
- Frontend: "Post conversation to Linear…" worktree action, plus
  "Resume from Linear issue" field in the create dialog.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ssue body

The flag handles both fresh-issue kickoffs and resumes, so the "resume"
prefix was misleading. Renames everywhere and updates the seed builder
to always pull the issue title + description into the conversation
context, regardless of whether a webmux attachment or linked PR exists.

Also:
- New --linear <id> shorthand for --from-linear ID --post-to-linear ID
  (round-trip on the same issue, the common pattern)
- Conversation context now always includes a Linear hint reminding the
  agent to reference "Fixes ENG-X" in PR title/body so Linear auto-links
  the PR if the branch doesn't match issue.branchName
- Contract field resumeFromLinear → fromLinear

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Now that --linear handles the "round-trip on an existing issue" case,
posting an arbitrary session into a pre-existing issue is redundant and
muddles the mental model. Tighten the surface so:

- --post-to-linear TEAM — creates a new Linear issue in TEAM, posts there
- --linear ENG-X — loads the issue body as context AND posts back to it
- --from-linear ENG-X — loads only, no post

`webmux linear post <branch> <team-key>` and the dashboard Post-to-Linear
dialog get the same restriction. Issue-id input produces a helpful error
that points at --linear.

Also simplifies the --linear conflict logic — mixing --linear with
--from-linear or --post-to-linear is now always rejected (no more "same
id is ok" exception).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapses --from-linear and --post-to-linear on oneshot into a single
--linear <ID|TEAM> that picks the right behavior from the input shape:

- ENG-123 (issue id) → load context from the issue + post results back
  (the round-trip case)
- ENG (team key) → no seed, create a new issue in the team when done

--from-linear stays on `webmux add` (no post lifecycle there), and the
manual `webmux linear post <branch> <team-key>` primitive is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The /ws/agents/worktrees/:name handler short-circuits for non-Codex
providers (server.ts:733), so Claude oneshots got the initial snapshot
and then sat silent while the tmux agent was happily running. The
dashboard works around this with history polling alongside the WS; do
the same in oneshot so the conversation actually streams to stdout.

Dedup in printNewMessages makes this a safe additive change for Codex
too (acts as a fallback for missed deltas).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The parse-fail warning fired on every history read for any line that
survived trim() but wasn't valid JSON — and with the new oneshot history
polling that meant ~30 warnings per minute. The line content was
typically empty-looking control characters that render blank.

- Add startsWith('{') as a cheap pre-filter so obvious non-JSON garbage
  doesn't reach JSON.parse.
- Downgrade the remaining log to debug so legitimate parse failures
  (e.g. mid-write partial JSON) don't spam stderr in normal use.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…exit, linear upload

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
So the agent's PR has the issue id in its branch name and Linear's GitHub
integration auto-links it. The team-key path now collapses into the same
round-trip flow as `--linear ENG-123`. Also makes oneshot tool-call output
compact (Claude-Code-style `● Tool(arg)` / `  ⎿ result`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`--keep-open` now means "leave the worktree session running after oneshot
exits" — oneshot always exits on idle in both modes. Drops the implicit
`--close-on-merge` default so on-merge behavior matches `webmux add` (no
default; respects global autoRemoveOnMerge).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes the auto-close service, the per-worktree `onMergeAction` override,
its CLI flags (`--close-on-merge`/`--remove-on-merge` on add+oneshot), the
UI dropdown, and the related API endpoint. The pre-existing global
`autoRemoveOnMerge` still gates auto-removal after PR sync.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the redundant "From Linear issue (optional)" text input on the
create dialog. The bottom-left LinearPanel was already the canonical
"start from Linear" entry point; now its click path sends
`fromLinear: { issueId }` so the backend injects the issue header and
any prior webmux attachment as context — same plumbing as the CLI's
`--linear ENG-123`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…erge-action

# Conflicts:
#	backend/src/server.ts
#	bin/src/completions.ts
#	bin/src/webmux.ts
#	frontend/src/App.svelte
#	frontend/src/lib/WorktreeList.svelte
#	packages/api-contract/src/contract.ts
CI typecheck caught these after the revert + main-merge: test object
literals were still spreading `onMergeAction: null` / passing
`onsetonmergeaction` props, and the merge accidentally introduced a
duplicate `writeWorktreeMeta` import in lifecycle-service.ts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ser narrowing

- Derive WebmuxConversationAttachmentPayload from the Zod schema so the
  validated boundary and the type can't drift; remove the duplicate interface.
- Gate the linked-worktree Post-to-Linear menu action behind a ConfirmDialog
  ("Post conversation to ENG-N?") — the endpoint is non-idempotent and a
  misclick previously created a duplicate attachment + comment.
- Update LinearPostDialog's inline "looks like an issue id" hint to point at
  the new worktree-menu shortcut instead of the CLI fallback.
- Add a second WS reconnect warning at attempt 15 so the user has signal
  during the silent window between the first warn (~6s) and the fatal (~60s).
- Reject `--branch` without `--linear` in `parseOneshotArgs` (matches the help
  text); the positional branch covers the non-linear path.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…er polish

- Track an in-flight `postingLinearBranches` set so the row menu's
  "Post conversation to ENG-N" item disables and shows "Posting to
  Linear…" while a request is pending; prevents a second click from
  firing a duplicate non-idempotent post before the first toast lands.
- Snapshot the linked issue id at click time (`postToLinkedConfirm =
  { branch, issueId }`) so the ConfirmDialog can't render "Post
  conversation to ?" if the worktree refreshes away between click
  and confirm.
- Drop the unnecessary `async` from `handlePostToLinear` — it only
  sets state synchronously now.
- Reorder `parseOneshotArgs` checks: `--branch` with `--resume` now
  surfaces a precise message ("Cannot use --branch with --resume")
  instead of the misleading "--branch only applies with --linear".
- Tighten `RECONNECT_WARN_AT` to a `readonly [number, number]` tuple
  for honest intent.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…erge-action

# Conflicts:
#	frontend/src/lib/WorktreeList.svelte
…st.ts

Two cases protecting `handlePostToLinear` / `confirmPostToLinkedIssue`:
- Linked worktree → menu item click opens the ConfirmDialog and "Post"
  fires `postWorktreeToLinear` exactly once with
  `{ kind: "issue", issueId: <identifier> }`. No `LinearPostDialog`
  rendered for linked worktrees.
- In-flight guard → while the post promise is unresolved, the row
  menu item disables to "Posting to Linear…" and a second click is
  a no-op (`postWorktreeToLinear` call count stays at 1).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`openWorktree`'s contract body was switched from `c.noBody()` to
`OpenWorktreeRequestSchema` (with `prompt?: string`) earlier in this
branch, but `App.svelte` still called `api.openWorktree({ params })`
with no `body`. ts-rest sent no JSON, the server's `req.json()`
threw, and the user got `Invalid JSON` on every reopen.

Pass `body: {}` to match the CLI call site at oneshot.ts:733-736.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds a second variant to the Linear label-driven auto-handler. Same
poller and master toggle as the existing `webmux` flow; the label
chooses the action:

- `webmux` (existing) → create a worktree via lifecycleService.
- `_oneshot` (new) → spawn `webmux oneshot --linear ENG-X` as a
  detached child process. Output is dropped — oneshot posts results
  back to Linear on exit, so the user sees them there.

An issue tagged with both labels routes to oneshot (more autonomous
of the two). The dispatch is gated on a new `WEBMUX_CLI_ENTRY` env
var set by `webmux serve` — if the backend is launched standalone
without it, the `_oneshot` path is silently disabled.

- backend/src/services/linear-auto-create-service.ts: split filters
  into webmux-create vs oneshot, accept an optional
  `runOneshotForIssue` dependency, dispatch both per poll cycle,
  reuse the single `processedIssueIds` dedupe set.
- backend/src/server.ts: build `runOneshotForIssue` from
  `WEBMUX_CLI_ENTRY`, spawning a detached process pinned to the
  server's port. `proc.unref()` so it doesn't keep the server alive.
- bin/src/webmux.ts: export `WEBMUX_CLI_ENTRY` to the backend's env.
- backend/src/__tests__/linear-auto-create-service.test.ts: cover
  Todo-state + label gating, both-label routing, existing-worktree
  exclusion, case-insensitive matching.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Moves the "1, 2, 5" responsibilities of oneshot mode (system prompt
injection — already server-side — plus auto-close on agent-done and
Linear post-back) out of the CLI driver and into a server-side
watcher. The CLI now just arms the watcher on create/open and owns
streaming + exit polling for the human watching the terminal.

Effect: a oneshot run can finish safely even if the CLI is killed,
the network drops, or the run was launched by the `_oneshot` Linear
label (no CLI to begin with). Any browser-originated interaction
with the session disarms the watcher so the human can take over.

- contract: `OneshotConfigSchema` carrying `autoCloseOnDone` and
  optional `postToLinearOnDone`. Plumbed through CreateWorktree and
  OpenWorktree request bodies.
- persisted meta: `WorktreeMeta.oneshot` lives on disk; presence is
  the "armed" signal.
- watcher: new `oneshot-watcher-service.ts` polls `source: "oneshot"`
  worktrees every 3s; after a 15s idle-grace (or immediately on
  stopped/error), fires `postToLinear` then `closeWorktree` and
  disarms via `lifecycleService.disarmOneshot`.
- disarm hooks: `apiSendPrompt`, `apiSendAgentsWorktreeMessage`,
  `apiInterruptAgentsWorktree`, and `apiUploadFiles` all clear the
  oneshot meta. First browser action wins — no race, no duplicate
  Linear post.
- CLI trim: drop the in-CLI close + post-back loops from
  `runOneshot`. Send `oneshot: {...}` in the create/open body so the
  server takes over. Resume path threads the same config through
  `openWorktree`.
- label trigger: `runOneshotForIssue` now calls
  `lifecycleService.createWorktree({ source: "oneshot", oneshot: {
  autoCloseOnDone: true, postToLinearOnDone: ... } })` directly
  instead of spawning the CLI. `WEBMUX_CLI_ENTRY` plumbing is gone.
- shared helper: `postWorktreeConversationToLinear` lifted out of
  `apiPostWorktreeToLinear` so the watcher can call it without HTTP.
- tests: 8 watcher cases covering idle-grace, immediate terminal,
  post-then-close ordering, disarmed-meta skip, autoClose=false,
  isActive gate, non-oneshot source skip.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…erge-action

# Conflicts:
#	backend/src/__tests__/linear-auto-create-service.test.ts
#	backend/src/server.ts
#	backend/src/services/linear-auto-create-service.ts
Fixes the high-severity items the post-merge review surfaced:

- **Watcher silently disabled in most cases.** `startOneshotWatcher`
  was gated on `hasRecentDashboardActivity`, which is zero for a
  CLI-only or label-triggered run. Drop the gate; iterating the
  worktree list every 3s is cheap when no oneshot worktrees exist.
- **CLI misclassified user-takeover as failure.** Expose `oneshot`
  on `ProjectWorktreeSnapshot` (server-side path: meta → reconciled
  state → runtime → snapshot) so the CLI's polling FSM can detect
  the watcher's disarm transition. New `onUserTookOver` callback in
  `pollProjectState` exits cleanly with code 0 and a "user took
  over" message instead of "agent idle without opening a PR" (1).
- **`disarmOneshotIfArmed` now updates runtime state** via the new
  `ProjectRuntime.setOneshot`, so the next snapshot reflects the
  disarm without waiting for a reconcile pass. Log level bumped from
  debug to warn on failure.

Plus the easy mediums:

- Extract `normalizeOneshotConfig(input)` helper (was duplicated in
  `apiCreateWorktree` and `apiOpenWorktree`).
- Gate `pollConversationHistory` on Claude — Codex has live deltas
  via WS, polling there was wasted load.
- `OneshotPostTarget` now re-exports `PostWorktreeToLinearTarget`
  from the contract instead of declaring a near-identical type.
- Update `getOneshotUsage` to reflect server-driven close + post
  and the disarm-on-interaction semantics.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- linear-auto-create-service: collapse `filterAutoCreateIssues` and
  `filterAutoOneshotIssues` onto a shared `filterTriggerableIssues`
  helper that owns the Todo + processedIssueIds + existing-branch
  logic. Each public filter now only specifies its label predicate.
- conversation-export-service: export `defaultSeedFromLinearDeps`
  bundling `fetchIssueWithAttachments` + `downloadWebmuxAttachment`.
  Replaces the inline 3-line record at all three call sites
  (server.runOneshotForIssue, server.apiCreateWorktree.fromLinear,
  CLI's --linear path). Test stubs keep using their own deps object.

No behavior change. 396 backend + bin tests still pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…close

M1: terminal WebSocket input/sendKeys now disarms the oneshot watcher on the
first message per connection so the human taking over via the terminal pane
doesn't get steamrolled by the auto-close.

M2: re-read meta immediately before closeWorktree so a browser-side disarm
that lands during the postToLinear HTTP call aborts the close.

M3: README documents the `_oneshot` label trigger and its precedence over the
`webmux` label.

L1: switch the last inline seed-from-Linear deps call site to the shared
defaultSeedFromLinearDeps helper.

L2: distinguish watcher-driven disarm from a real user takeover in CLI logs
by checking whether mux is still alive at the disarm transition.

L3: lifecycle endpoints (open without re-arm, close, archive, merge) call
disarmOneshotIfArmed so end-of-session actions also clear armed meta.

L4: treat agentLifecycle === "closed" as terminal in the watcher so an
externally killed agent doesn't leave meta armed indefinitely.

L6: expose `oneshot` on the frontend WorktreeInfo and `mapWorktree` so the
UI has access to the armed state.

L7: oneshot-watcher-service tests now cover postToLinear failure, disarm
during postToLinear, closeWorktree throwing, and the `closed` lifecycle path.

L8: source-level regression test asserts every disarm reason string the
watcher relies on is actually wired in server.ts.

L9: tighten WebmuxConversationAttachmentPayloadSchema.agent to AgentIdSchema.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@hugocasa
hugocasa marked this pull request as draft May 13, 2026 13:33
@hugocasa
hugocasa marked this pull request as ready for review May 13, 2026 13:33
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented May 13, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
webmux 9523e81 Commit Preview URL

Branch Preview URL
May 13 2026, 04:00 PM

@claude

claude Bot commented May 13, 2026

Copy link
Copy Markdown

Claude finished @hugocasa's task in 6m 45s —— View job


Code Review for PR #238: webmux oneshot + Linear round-trip

Thorough review of the 59 changed files. Overall this is a solid, well-tested branch — clean separation between the server-side watcher and the CLI streamer, careful disarm wiring, and excellent test coverage (especially oneshot-watcher-service.test.ts). A few concerns worth addressing before merge.

  • Read CLAUDE.md/AGENTS.md and project conventions
  • Survey the diff against origin/main
  • Review new backend services
  • Review server.ts disarm wiring and new endpoints
  • Review CLI oneshot.ts, linear-commands.ts
  • Review frontend changes
  • Review API contract + shared types

🟠 Concern: fresh-worktree race — watcher could fire on lifecycle: "closed" before first agent event

backend/src/services/oneshot-watcher-service.ts:67-70 treats closed as a terminal lifecycle and fires without the 15s idle grace:

const isTerminal =
  agentLifecycle === "stopped" ||
  agentLifecycle === "error" ||
  agentLifecycle === "closed";

But lifecycle: "closed" is also the default for a freshly upserted worktree (backend/src/services/project-runtime.ts:56). For Claude oneshots, lifecycle only transitions to "running" when the UserPromptSubmit hook fires (backend/src/adapters/agent-runtime.ts:219-222) — Claude has no SessionStart hook. If the agent process is slow to invoke that hook (cold start, large prompt, container boot), the watcher's first 3 s poll can see closed on an armed oneshot worktree and immediately fire closeWorktree + Linear post-back against an empty conversation.

The PR description references a previous "HIGH-severity watcher gating bug silently disabled by an isActive predicate". The current closed-as-terminal fix solves one legitimate case (server restart mid-run) but reintroduces a window where a brand-new run can be killed before it starts.

Suggested fixes (one of):

  1. Apply the idle grace to closed too — wait 15 s of continuous closed before firing. A real terminal close stays closed; a startup window resolves to running first.
  2. Gate closed on a "we ever saw this run alive" flag — track per-branch whether we've observed running/idle at least once, and only treat closed as terminal after that.
  3. Emit an immediate starting lifecycle event in materializeRuntimeSession (server-side, before the agent reports for itself) so the watcher's first reading is never closed.

Option 1 is the smallest change and would match the existing idle-grace pattern. Fix this →


🟡 Minor — disarmOneshot uses void oneshot to satisfy the unused-var lint

backend/src/services/lifecycle-service.ts:306-309:

const { oneshot, ...rest } = resolved.meta;
void oneshot;
await writeWorktreeMeta(resolved.gitDir, rest);

The void oneshot; reads as code smell. Cleaner alternatives:

const nextMeta = { ...resolved.meta };
delete nextMeta.oneshot;
await writeWorktreeMeta(resolved.gitDir, nextMeta);

…matches the same pattern already used by withUpdatedLabel two methods down (lifecycle-service.ts:663-671).


🟡 Minor — forcePrSync swallows errors silently in the CLI

bin/src/oneshot.ts:481-488 swallows sync failures. If the agent opened a PR but the post-sync GitHub call failed (rate limit, transient network), the CLI exits with code 1 and tells the user "agent stuck without opening a PR" — a misleading failure mode. A stderr(...) log on the catch would surface the actual cause without breaking flow.


🟡 Minor — webmuxVersion never wired through the server path

buildLinearSummaryMarkdown accepts webmuxVersion?: string (linear-service.ts:811) but the server-side caller postWorktreeConversationToLinear doesn't pass it (server.ts:1278-1286). Either drop the field or wire it through package.json's version. Currently always omitted in production.


🟡 Minor — processedIssueIds is process-global, not persisted

backend/src/services/linear-auto-create-service.ts:31. On server restart, the set is cleared. The existingBranches check makes this safe for webmux-label issues (the worktree exists, so the branch is taken). For _oneshot-label issues that failed to launch (e.g., runOneshotForIssue threw before createWorktree succeeded), the same issue will retry on every restart. Probably the right behavior, but worth a one-line comment so a future reader doesn't worry.


🟡 Minor — disarmAttempted is sticky per-connection but doesn't survive a re-attach

backend/src/server.ts:1801-1816 (terminal WS) sets data.disarmAttempted = true on the first input/sendKeys and never resets. A user who opens the worktree in the browser, types one char (disarming), then closes the tab and a oneshot run gets re-armed via CLI relaunch: the new WS connection gets a fresh disarmAttempted = false, so the next input correctly disarms again. ✅ Just verifying the design intent — this looks correct.


🟡 Minor — apiSyncWorktreePrs does a full readProjectSnapshot() per call

backend/src/server.ts:1318-1323. Each syncWorktreePrs call triggers a full reconcile + snapshot just to slice out one worktree. The CLI's forcePrSync only fires once when the agent is stable, so it's not in a hot loop. Fine for now, but consider returning the synced single-worktree state directly if this becomes a hot path.


🟢 Praise

  • oneshot-watcher-service.test.ts is thorough — the disarm-during-postToLinear race, postToLinear-failure-but-still-close, and closed-as-terminal cases are all covered with sharp assertions.
  • The OneshotConfig wire shape on CreateWorktree + OpenWorktree cleanly threads disarm-aware state without an out-of-band channel.
  • disarmOneshotIfArmed is applied to every user-interaction endpoint with a clear reason parameter that lands in logs — easy to debug "why did the watcher disarm at 14:32".
  • parseLinearTarget as a single discriminated-union helper used by both CLI and server keeps the issue-vs-team logic in one place.
  • The escapeFence hack (zero-width separator inside ```) is a nice touch for embedding agent transcripts in markdown without breaking fences.
  • branchMatchesIssue handles the Linear branch-prefix variants well — and the tests cover the slash cases.

Summary

Severity Count
🟠 Medium 1 (fresh-worktree closed race)
🟡 Low 6 (style + observability)
🟢 Praise 6

The medium-severity concern (closed lifecycle handling) is the only one that could affect runtime correctness. Everything else is polish. Recommend addressing the watcher race before merging since it's a small change in oneshot-watcher-service.ts with the existing test scaffolding ready to cover the fix.
· Branch

@claude

claude Bot commented May 13, 2026

Copy link
Copy Markdown

Claude finished @hugocasa's task in 4m 57s —— View job


Code Review — PR #238

This is a strong PR. The architectural pivot (server-driven oneshot watcher instead of CLI-driven close/post) is the right move — the CLI is now a thin streamer, and the auto-close + Linear post-back survive Ctrl-C, network blips, and CLI crashes. The disarm-on-interaction story is well thought through, and the test coverage on the watcher itself is excellent. Most of my notes are around docs, edge cases, and small polish.


Strengths

  • Server-driven watcher is well-designed. backend/src/services/oneshot-watcher-service.ts:43-123 correctly handles the close/post race: meta is re-read immediately before closeWorktree to detect a user disarm during the Linear post-back HTTP call (lines 99-106). Treating agentLifecycle === "closed" as terminal (lines 67-70) is the right defense against server crashes leaving meta armed forever.
  • Disarm wiring is exhaustive and self-checking: every browser-interaction endpoint calls disarmOneshotIfArmed, and backend/src/__tests__/server-disarm-wiring.test.ts is a clever source-level regression test that future refactors can't silently break.
  • LinearTarget discriminated union in packages/api-contract/src/schemas.ts:30-46 is clean — parseLinearTarget is shared between CLI, backend, and frontend, so the issue/team/invalid distinction stays consistent across surfaces.
  • Test coverage on the watcher (backend/src/__tests__/oneshot-watcher-service.test.ts) hits all the edge cases: idle-grace, terminal-without-grace, post-failure-still-closes, close-failure-still-disarms, race-on-meta-disarmed-during-post, and the closed terminal lifecycle.
  • CLI's user-took-over exit path (bin/src/oneshot.ts:510-515) — watching the snapshot.oneshot armed→null transition while mux is still alive is the clean primitive. Avoids the CLI needing to know about every disarm trigger.

Issues

MEDIUM — README claim about _oneshot retriggering is incorrect

README.md:51:

Each issue is processed once per server lifetime; remove the label and re-add it to retrigger.

But processedIssueIds is keyed by issue.id (not label state) — see backend/src/services/linear-auto-create-service.ts:31,50,112. Removing + re-adding the _oneshot label does not clear the entry. Only a server restart (or a worktree existing for that branch, which short-circuits via filterTriggerableIssues) retriggers it. Either update the README to say "restart the server to retrigger", or implement label-based dedup (e.g. track (issueId, sawLabelAt) and clear when label disappears).

Fix this →

MEDIUM — disarmAttempted is per-WS-connection, so a re-arm on the same WS won't disarm

backend/src/server.ts:1804-1816 sets data.disarmAttempted = true on the first input/sendKeys. Sequence that breaks it:

  1. User opens dashboard, types into terminal → disarmAttempted = true, meta cleared.
  2. User runs webmux oneshot --resume <branch> from a separate shell → apiOpenWorktree re-arms meta because oneshot is supplied (see backend/src/server.ts:1031if (!oneshot) await disarmOneshotIfArmed(...)).
  3. User types into the same still-open WSdisarmAttempted is already true, so the keystroke is forwarded but meta stays armed. The watcher will fire close + post-back even though the human is actively typing.

Unusual sequence, but plausible if the user keeps the dashboard tab open between CLI runs. Cheap fix: reset data.disarmAttempted = false when apiOpenWorktree arms a new oneshot config (would need a way to signal the WS, or just drop the cache and pay the disk read on every keystroke — input events aren't that hot in practice).

LOW — LifecycleService.disarmOneshot has no direct unit test

backend/src/services/lifecycle-service.ts:299-311 reads meta from disk, deletes the oneshot field, writes it back. The watcher tests mock disarmOneshot and never exercise the real implementation. Worth a small lifecycle-service test that:

  1. Arms a worktree (writes meta with oneshot).
  2. Calls disarmOneshot.
  3. Asserts the on-disk meta no longer contains oneshot but preserves the other fields.

LOW — "oneshot" badge tooltip is misleading

frontend/src/lib/WorktreeList.svelte:124-130 titles the badge as "Created by the oneshot CLI", but _oneshot-labeled Linear issues now also produce oneshot-source worktrees via runOneshotForIssue in backend/src/server.ts:135-159. Suggested tooltip: "Autonomous run (oneshot CLI or Linear _oneshot label)" or just "Oneshot session — auto-closes when done".

LOW — disarmOneshot's destructure-and-void pattern is awkward

backend/src/services/lifecycle-service.ts:307-309:

const { oneshot, ...rest } = resolved.meta;
void oneshot;
await writeWorktreeMeta(resolved.gitDir, rest);

void oneshot is a workaround for the unused-binding lint. Cleaner:

const nextMeta: WorktreeMeta = { ...resolved.meta };
delete nextMeta.oneshot;
await writeWorktreeMeta(resolved.gitDir, nextMeta);

Same behavior, no void ceremony. (Same pattern would help the withUpdatedLabel helper a few lines up which already uses delete.)

LOW — Watcher tests cast through unknown to LifecycleService

backend/src/__tests__/oneshot-watcher-service.test.ts:81 does as unknown as LifecycleService. The watcher only needs closeWorktree and disarmOneshot. Defining OneshotWatcherDependencies['lifecycleService'] as Pick<LifecycleService, "closeWorktree" | "disarmOneshot"> (currently it's the full LifecycleService) would let the test type itself precisely and catch any future watcher additions that aren't satisfied by the mock.

LOW — Duplicate buildSeedFromLinear work between CLI and server

bin/src/oneshot.ts:709 resolves the seed in-process, then apiCreateWorktree in backend/src/server.ts:918-944 may call buildSeedFromLinear again if conversationContext is not supplied. The CLI only sets conversationContext when seed.conversationMarkdown is truthy — but buildSeedFromLinear always returns at least an issueHeader (see conversation-export-service.ts:319), so this branch only fires if the CLI couldn't get the markdown. The current code is correct, but the dual-fetch path is subtle. A short comment in apiCreateWorktree's fallback explaining "this only fires when the CLI couldn't resolve the seed itself" would help future readers.

NIT — runOneshotForIssue lives inline in server.ts and isn't unit-tested

backend/src/server.ts:135-159 does seed → mode → branch → createWorktree logic that's worth a small oneshot-trigger.ts or similar — currently you can only exercise it through the linear-auto-create service. Not blocking, but extracting would mirror the pattern used for postWorktreeConversationToLinear (also inlined and shared between two callers).

NIT — parseClaudeSessionRecords log-level changed from warn to debug

backend/src/adapters/claude-cli.ts:99-102: previously a malformed JSONL line was a warn, now it's debug and only lines that don't start with { are skipped silently. The intent (filtering out non-JSON noise like fenced code blocks in resumed-with-stdin sessions) is reasonable, but a genuinely corrupt JSON line is now invisible in default-level logs. Consider keeping warn for the JSON.parse catch and debug for the prefix filter.


Verdict

Solid work — the watcher design, race-safe close, and the source-level wiring test are highlights. The MED-1 README/code mismatch and MED-2 disarm-cache corner case are the two I'd want fixed before merge; everything else is polish. Nice that the eight rounds of self-review on #230 caught the high-severity gating bug before reopening.
· feat/oneshot-and-on-merge-action

…e disarm

M1: auto-create service evicts processedIssueIds entries whose issue is no
longer Todo+labeled, so removing and re-adding the label actually retriggers
(matching the README). Tested with the new "re-creates after the label is
removed and re-added" case.

M2: terminal WS input/sendKeys check projectRuntime.getWorktreeByBranch().oneshot
(in-memory, cheap) instead of a per-connection cache. A re-arm on the same WS
is now detected without a stale-cache miss.

L1: new lifecycle-service test exercises disarmOneshot end-to-end on real
on-disk meta — arms, disarms, asserts the oneshot block is gone but profile,
allocatedPorts, startupEnvValues, branch are preserved. Also asserts idempotency.

L2: oneshot badge tooltip is now "Autonomous run — auto-closes when done"
to cover both the `oneshot` CLI and the `_oneshot` Linear label trigger.

L3: lifecycle-service.disarmOneshot uses a plain `delete` on a shallow copy
instead of the awkward destructure-and-`void` pattern.

L4: OneshotWatcherDependencies.lifecycleService narrowed to
`Pick<LifecycleService, "closeWorktree" | "disarmOneshot">` so watcher tests
can mock without `as unknown as LifecycleService`.

L5: comment in apiCreateWorktree's fromLinear fallback clarifying it only
fires for dashboard/REST callers (CLI resolves the seed in-process first).

NIT: JSON.parse failure in parseClaudeSessionRecords logs at `warn` again so
a corrupt session file is visible — the prefix filter at `debug` still drops
non-JSON noise silently.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@hugocasa
hugocasa marked this pull request as draft May 13, 2026 13:57
@hugocasa
hugocasa marked this pull request as ready for review May 13, 2026 13:57
@claude

claude Bot commented May 13, 2026

Copy link
Copy Markdown

Claude finished @hugocasa's task in 7m 38s —— View job


Code Review — PR #238: webmux oneshot + Linear round-trip

Solid work overall — the server-driven watcher with race-safe close is the right architecture, and the disarm wiring is well-thought-out. The server-disarm-wiring.test.ts source-level guard is a clever addition. One concern that's been raised twice now is still unaddressed and could affect runtime correctness for cold-start oneshots; everything else is polish.

Inline comments cover the specific files/lines. Summary below.


🟠 MEDIUM — Fresh-worktree race on lifecycle: "closed" (still unaddressed)

backend/src/services/oneshot-watcher-service.ts:67-72 treats closed as immediately terminal (no idle grace). But closed is also the default for a freshly upserted worktree (backend/src/services/project-runtime.ts:56 makeDefaultState), and Claude only flips to "running" when its UserPromptSubmit hook fires (backend/src/adapters/agent-runtime.ts:219-222). Combined with bin/src/oneshot.ts:574 returning ready as soon as mux=true && status !== "creating" — so it accepts status="closed" — the 3s watcher cadence can fire close + post-back on an empty conversation during the agent's cold-start window.

Both prior reviewers raised this in round 1 (MED-1). The latest commit (refactor: address PR #238 review — label dedup eviction, runtime-state disarm) encoded the current behavior in oneshot-watcher-service.test.ts:264-277 rather than guarding the startup window.

Fix: apply the 15s idle grace to closed too — a real terminal close stays closed, but a startup window resolves to running/idle first. See the inline comment on oneshot-watcher-service.ts:66-80 for a concrete patch + test note.

🟡 LOW

  1. Watcher's disarmOneshot doesn't update ProjectRuntime stateoneshot-watcher-service.ts:121 writes meta to disk only. When autoCloseOnDone=true, the subsequent closeWorktree triggers reconcile that picks up the change. When autoCloseOnDone=false, no reconcile fires, so snapshot.oneshot stays armed in memory until the next regular reconcile pass — and the CLI's "user took over" detector (bin/src/oneshot.ts:510-515) can miss the disarm. Mirror the disarm to runtime state like server.ts:195-196 does.

  2. webmuxVersion plumbed but never wiredbuildLinearSummaryMarkdown accepts it (linear-service.ts:810), but postWorktreeConversationToLinear doesn't pass it (server.ts:1278-1286). Production summary comments always omit version. Same observation as prior round.

  3. forcePrSync silently swallows errorsbin/src/oneshot.ts:481-488. If the post-stable PR sync fails (rate limit/network), seenPrUrls stays empty and the CLI mis-reports "agent stuck without opening a PR" and exits 1. A stderr log surfaces the real cause. Same as prior round.

  4. CLI starts polling before agent is aliveensureWorktreeReady returns at status="closed". See inline on bin/src/oneshot.ts:570-584 for the related cold-start window and a UX nit (silent 1-3s gap).

  5. apiSyncWorktreePrs does a full project reconcile per callserver.ts:1316-1321. Not in the hot path today (CLI fires once when stable). Same as prior round; worth flagging for future hot paths.

NIT

  • pollProjectState's consecutiveClosedReadings >= 2 introduces a ~6s lag between watcher-closes-session and CLI-detects-close (bin/src/oneshot.ts:518-523). Minor UX delay; comment could explain the 2-tick threshold.
  • runOneshotForIssue (server.ts:135-159) is inlined and shared with the auto-create service. Worth extracting to a small oneshot-trigger.ts so it can be unit-tested independently of the polling layer (mirrors the postWorktreeConversationToLinear extraction pattern).

🟢 PRAISE

  • backend/src/__tests__/server-disarm-wiring.test.ts — source-level regression test that future refactors can't silently break the disarm UX. Clever and cheap.
  • oneshot-watcher-service.test.ts covers the watcher's tricky paths: idle-grace, terminal-without-grace, post-failure-still-closes, close-failure-still-disarms, and the disarm-during-postToLinear race (lines 224-248). Sharp asserts.
  • Label dedup eviction in linear-auto-create-service.ts:90-105 now correctly matches the README's "remove + re-add the label to retrigger" promise — fixes the prior round's MED-1 from review Add test.txt #2.
  • LifecycleService.disarmOneshot cleanly uses delete nextMeta.oneshot (lifecycle-service.ts:307-309) — much nicer than the previous void oneshot pattern.
  • OneshotConfig wire shape on both CreateWorktree and OpenWorktree cleanly threads disarm-aware state without an out-of-band channel.
  • parseLinearTarget shared across CLI/backend/frontend keeps the issue/team/invalid distinction consistent in one place.
  • New escapeFence helper (conversation-export-service.ts:122-127) is a nice defense against accidental fence-closing in embedded transcripts.

Verdict

Severity Count
🟠 Medium 1 (fresh-worktree closed race, still open)
🟡 Low 5
🟢 Praise 6

The MED-1 race is the only one I'd insist on before merge — it's a small change in oneshot-watcher-service.ts with the existing test scaffolding ready to cover the fix. Everything else is polish or carry-over from the prior round.
· feat/oneshot-and-on-merge-action

Comment on lines +66 to +80
// `closed` is terminal-without-completion: the agent process is gone (e.g. server
// restart mid-run, manual exit). Without this, armed meta would persist forever
// and re-fire as soon as the agent comes back to life.
const isTerminal =
agentLifecycle === "stopped" ||
agentLifecycle === "error" ||
agentLifecycle === "closed";
const isIdle = agentLifecycle === "idle";
if (!isTerminal && !isIdle) {
state.idleSinceMs = null;
return;
}
if (state.idleSinceMs === null) state.idleSinceMs = now();
const stable = isTerminal || now() - state.idleSinceMs >= idleGrace;
if (!stable) return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Fresh-worktree race — treating closed as terminal without idle grace can fire close + Linear post-back on an empty conversation for a brand-new oneshot.

lifecycle: "closed" is the default for a freshly upserted worktree (see project-runtime.ts:56 makeDefaultState). Claude only transitions out of closed when the UserPromptSubmit hook fires (agent-runtime.ts:219-222), and the CLI's ensureWorktreeReady returns as soon as mux=true even with status="closed". With the 3s watcher cadence, a slow agent start can hit the first poll while still closed and immediately fire close + post-back on an empty transcript.

The watcher test on L264 (treats closed agent lifecycle as terminal so meta does not stay armed) currently encodes this behavior — which is the right defense against server-restart-mid-run, but it's also the bug for a fresh oneshot. Both prior reviewers raised this; the latest commit only added the closed branch without guarding the startup window.

Suggested fix (smallest change): apply the same idle grace to closed as to idle. A real terminal close stays closed; a startup window resolves to running first.

Suggested change
// `closed` is terminal-without-completion: the agent process is gone (e.g. server
// restart mid-run, manual exit). Without this, armed meta would persist forever
// and re-fire as soon as the agent comes back to life.
const isTerminal =
agentLifecycle === "stopped" ||
agentLifecycle === "error" ||
agentLifecycle === "closed";
const isIdle = agentLifecycle === "idle";
if (!isTerminal && !isIdle) {
state.idleSinceMs = null;
return;
}
if (state.idleSinceMs === null) state.idleSinceMs = now();
const stable = isTerminal || now() - state.idleSinceMs >= idleGrace;
if (!stable) return;
// `closed` is terminal-without-completion: the agent process is gone (e.g. server
// restart mid-run, manual exit). Without this, armed meta would persist forever
// and re-fire as soon as the agent comes back to life. BUT `closed` is also the
// default for a freshly upserted worktree (project-runtime.ts:56), and Claude
// only transitions to "running" via UserPromptSubmit — so we apply idle grace
// to `closed` too. A real terminal close stays `closed`; a startup window
// resolves to "running"/"idle" first.
const isTerminalImmediate =
agentLifecycle === "stopped" ||
agentLifecycle === "error";
const needsGrace =
agentLifecycle === "idle" ||
agentLifecycle === "closed";
if (!isTerminalImmediate && !needsGrace) {
state.idleSinceMs = null;
return;
}
if (state.idleSinceMs === null) state.idleSinceMs = now();
const stable = isTerminalImmediate || now() - state.idleSinceMs >= idleGrace;
if (!stable) return;

Then add a test asserting closed lifecycle waits the idle grace (analog of the existing waits the idle grace before firing on idle test).

Fix this →

// Disarm so the watcher doesn't re-trigger on the next poll, even if close
// didn't fully succeed (e.g. user reopens manually — that interaction would
// disarm anyway, but the explicit clear here removes the race).
await deps.lifecycleService.disarmOneshot(branch);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Runtime-state mirror missing when autoCloseOnDone === false

lifecycleService.disarmOneshot(branch) only writes meta to disk — it doesn't update ProjectRuntime.oneshot. When autoCloseOnDone === true, the subsequent closeWorktree call triggers reconciliationService.reconcile(..., { force: true }) which re-reads meta from disk and clears the runtime state. But when autoCloseOnDone === false, no reconcile fires, so ManagedWorktreeRuntimeState.oneshot (and thus WorktreeSnapshot.oneshot) stays non-null until the next regular reconcile pass.

The CLI's pollProjectState watches snapshot.oneshot going armed → null as the "user took over" signal (bin/src/oneshot.ts:510-515). For a --keep-open run that posted to Linear and disarmed, the CLI keeps polling against a stale oneshot: { ... } snapshot until the next reconcile.

Fix: mirror the disarm to runtime state right after the disk write, same way disarmOneshotIfArmed does in server.ts:195-196:

await deps.lifecycleService.disarmOneshot(branch);
const state = deps.projectRuntime.getWorktreeByBranch(branch);
if (state) deps.projectRuntime.setOneshot(state.worktreeId, null);

(That needs projectRuntime to add a getWorktreeByBranch to the watcher dep type, or a new disarmRuntimeOneshot(branch) method.)

Comment thread backend/src/server.ts
baseBranch: state.baseBranch ?? null,
agent: resolved.worktree.agentName ?? null,
prUrl,
conversation: conversationResult.data.conversation,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 webmuxVersion is accepted by buildLinearSummaryMarkdown but never passed here.

linear-service.ts:810 accepts webmuxVersion?: string, but exportInput here doesn't supply it — production runs always omit the version from the summary comment. Either drop the field from LinearSummaryInput and ExportConversationInput, or read it from the bin/package.json and thread it through.

Same observation in the prior review round — still unaddressed.

Comment thread bin/src/oneshot.ts
Comment on lines +481 to +488
const forcePrSync = async (): Promise<void> => {
try {
const refreshed = await api.syncWorktreePrs({ params: { name: branch } });
recordPrEvents(state, refreshed, callbacks.onPrEvent);
} catch {
// Sync may transiently fail (network/server) — fall back to whatever we already saw.
}
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 forcePrSync silently swallows errors — misleading "agent stuck" exit.

If the agent opened a PR but this one-shot sync HTTP call fails (rate limit, network blip), state.seenPrUrls stays empty and the CLI prints "agent stuck without opening a PR" and exits 1. A stderr log on the catch would surface the real cause without breaking the fall-back flow:

Suggested change
const forcePrSync = async (): Promise<void> => {
try {
const refreshed = await api.syncWorktreePrs({ params: { name: branch } });
recordPrEvents(state, refreshed, callbacks.onPrEvent);
} catch {
// Sync may transiently fail (network/server) — fall back to whatever we already saw.
}
};
const forcePrSync = async (): Promise<void> => {
try {
const refreshed = await api.syncWorktreePrs({ params: { name: branch } });
recordPrEvents(state, refreshed, callbacks.onPrEvent);
} catch (err) {
const msg = err instanceof Error ? err.message : String(err);
callbacks.onPrEvent(`PR sync failed (will rely on prior snapshot): ${msg}`);
}
};

Same observation as the prior review round.

Comment thread bin/src/oneshot.ts
Comment on lines +570 to +584
while (Date.now() < deadline) {
try {
const response = await api.fetchWorktrees();
const worktree = response.worktrees.find((w: ProjectWorktreeSnapshot) => w.branch === branch);
if (worktree && worktree.mux && worktree.status !== "creating") {
return { ready: true, worktree };
}
} catch {
// ignore
}
await new Promise((resolve) => setTimeout(resolve, 500));
}
stderr(`[${timestamp()}] [error] timed out waiting for ${branch} session to start`);
return { ready: false };
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 ensureWorktreeReady returns at status="closed" — the CLI starts polling before the agent is alive.

The loop exits as soon as mux=true && status !== "creating". For a fresh oneshot, status is the runtime lifecycle, which defaults to "closed" (per project-runtime.ts:56) until Claude's UserPromptSubmit hook flips it to "running". Two implications:

  1. Combined with the watcher's "closed-is-terminal" handling (oneshot-watcher-service.ts:67-72), there's a window where the watcher can fire close + post-back on an empty conversation between create and the first hook event. See the related inline on the watcher.
  2. Even with the watcher fixed, the CLI shows nothing in that 1-3s window. A "waiting for agent to start" stderr line would close the silence gap.

Consider also waiting for status to be one of "running" | "idle" (not "closed") before returning ready, so the CLI's poll state and the watcher's poll state line up on the same "agent alive" signal.

M1: the previous "closed = terminal" patch was wrong — `closed` is also the
default lifecycle for a freshly upserted worktree before the agent's first
event arrives. Treat it like `idle` (needs the 15s idle grace) so a cold-start
session that resolves to running/idle within the grace window survives, while
a genuine post-run close still fires after the grace expires. Replaces the
"closed = immediate fire" test with two cases that cover both behaviors.

L1: mirror the watcher's `disarmOneshot` to `ProjectRuntime` so the snapshot
reflects the cleared armed state immediately. Without this, when
`autoCloseOnDone=false` no close-driven reconcile fires and
`snapshot.oneshot` stays armed in memory, defeating the CLI's
"user took over" detector.

L2: `postWorktreeConversationToLinear` now passes `pkg.version` as
`webmuxVersion` so Linear summary comments include the producing CLI version.

L3: `forcePrSync` in the CLI poller now logs a warning to stderr on failure
instead of swallowing — a network blip during the post-stable sync was
indistinguishable from "agent stuck without opening a PR".

L4: `ensureWorktreeReady` now waits for status to leave `closed`, not just
`creating`. Pairs with M1 — the CLI no longer starts polling during the
cold-start window the watcher is now correctly ignoring.

NIT: comment on `consecutiveClosedReadings >= 2` explains the ~3-6s lag is a
deliberate trade-off against false-positive close events on reconcile gaps.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@hugocasa
hugocasa marked this pull request as draft May 13, 2026 14:21
@hugocasa
hugocasa marked this pull request as ready for review May 13, 2026 14:21
@claude

claude Bot commented May 13, 2026

Copy link
Copy Markdown

Claude finished @hugocasa's task in 6m 49s —— View job


Code Review — PR #238

Verdict: The MED-1 fresh-worktree race and all four LOW items from the prior round are properly addressed in efed8b8. The fix is small, the new tests cover both the cold-start and genuine-close cases, and the runtime-state mirror is wired correctly. Nothing in this round rises to merge-blocker. Two minor concerns below, both edge-case behavior changes worth a note before merge.

  • Read AGENTS.md / CLAUDE.md
  • Survey the diff and the latest commit (efed8b8)
  • Verify MED-1 (fresh-worktree closed race) is actually fixed
  • Verify the LOW concerns from prior rounds
  • Look for any new bugs introduced by the fix commits
  • Post review

Verification of prior-round findings

Concern Status
MED-1: fresh-worktree race on lifecycle: "closed" ✅ Fixed at backend/src/services/oneshot-watcher-service.ts:66-80. Now treated as needsGrace, with two test cases (oneshot-watcher-service.test.ts:275-309) covering both the cold-start guard and the post-grace fire.
L1: runtime-state mirror on watcher disarm ✅ Fixed at oneshot-watcher-service.ts:121-130. setOneshot(worktreeId, null) is called right after disarmOneshot, matching the pattern in server.ts:196-197.
L2: webmuxVersion plumbed but not wired ✅ Fixed at server.ts:1288. pkg.version now flows through ExportConversationInputbuildLinearSummaryMarkdown.
L3: forcePrSync silently swallows errors ✅ Fixed at bin/src/oneshot.ts:486-492. Failures now surface as [warn] failed to sync PRs from server: … on stderr.
L4: ensureWorktreeReady returns at status="closed" ✅ Fixed at bin/src/oneshot.ts:586-591. But see new LOW-1 below — there's a subtle edge case introduced by this change.
Label-dedup eviction (round-2 MED) ✅ Fixed in linear-auto-create-service.ts:90-105; covered by the new re-creates after the label is removed and re-added test.
disarmOneshot uses void oneshot lint workaround ✅ Cleaned up at lifecycle-service.ts:307-309 (delete nextMeta.oneshot).

🟡 LOW-1 — ensureWorktreeReady now rejects status="closed", which can break --resume without --prompt

bin/src/oneshot.ts:586-591:

if (
  worktree &&
  worktree.mux &&
  worktree.status !== "creating" &&
  worktree.status !== "closed"
) {
  return { ready: true, worktree };
}

The L4 fix is correct for the cold-start path (new worktree, mux just adopted, agent hasn't fired its first hook yet). But for webmux oneshot --resume <branch> without --prompt, the resumed agent has no UserPromptSubmit event to trigger and may legitimately sit at lifecycle: "closed" (e.g., the prior run completed and we're resuming after the fact). openWorktree adopts the existing tmux session without re-prompting, so nothing flips the lifecycle.

Result: the while (Date.now() < deadline) loop spins for 60s and then returns { ready: false }, exiting 1 with timed out waiting for ${branch} session to start. Parsing already supports --resume with no prompt (oneshot.test.ts:21-26: "parses --resume without prompt").

Two-line fix: only require status !== "closed" for the new-worktree path. A simple flag threaded down from the caller would do:

async function ensureWorktreeReady(
  branch: string,
  port: number,
  stderr: (line: string) => void,
  expectColdStart: boolean,
): Promise<...> {
  ...
  const acceptable = worktree && worktree.mux && worktree.status !== "creating"
    && (!expectColdStart || worktree.status !== "closed");
  if (acceptable) return { ready: true, worktree };

Caller passes expectColdStart = !(parsed.resume || existingWorktree).

Fix this →

🟡 LOW-2 — runOneshotForIssue retries every 60s when the Linear branch already exists locally

backend/src/services/linear-auto-create-service.ts:50-52 filters issues by checking existingBranches, which comes from git.listWorktrees() — that only returns checked-out worktrees, not all local branches. If a user has a pre-existing local branch matching issue.branchName (e.g., they hand-created myname/eng-42-fix earlier) and then someone adds the _oneshot label to that issue:

  1. filterAutoOneshotIssues does NOT short-circuit (branch isn't in worktrees).
  2. runOneshotForIssue runs buildSeedFromLinear with source = "none"mode = "new".
  3. lifecycleService.createWorktree({ mode: "new", branch }) throws LifecycleError("Branch already exists: …", 409) (lifecycle-service.ts:452-455).
  4. processedIssueIds.add is NOT called → poll retries in 60s.
  5. Repeats forever, logging [linear-auto-create] failed to launch oneshot for ENG-42: Branch already exists: … every 60s.

Two reasonable mitigations:

  1. Catch the 409 and add to processedIssueIds so the spammed error becomes a one-time log line. User can resolve out-of-band.
  2. In runOneshotForIssue, when source === "none" but the branch exists locally, fall back to mode: "existing". This is the safest "do what the user obviously meant" behavior, though it does mutate the user's existing branch — risky.

Option 1 is probably the right call. Same observation: a Linear-side _oneshot issue whose runOneshotForIssue keeps failing for any reason (e.g., disk full, lifecycle-service bug) will spam every minute until the label or state changes.

NIT — Default lifecycle in project-runtime.ts:56 (makeDefaultState) deserves a comment

The whole MED-1 saga turns on the fact that makeDefaultState sets agent.lifecycle = "closed". That's the only "ambiguous" lifecycle value with two distinct meanings (default state vs. agent exited). A two-line comment at project-runtime.ts:56 explaining "closed is the default; the oneshot watcher must distinguish startup-window-closed from terminal-closed" would save a future reader from re-discovering this footgun.


🟢 Praise

  • MED-1 fix is the minimum delta with the right test scaffolding — the two new test cases (does not immediately fire on a freshly upserted closed worktree + fires on closed once the idle grace has elapsed) precisely express the discriminating behavior. Crisp.
  • Runtime mirror in the watcher (oneshot-watcher-service.ts:125-130) — the inline comment names exactly the case it's defending against (autoCloseOnDone=false + CLI's user-took-over detector), which is the kind of comment that's worth its weight.
  • server-disarm-wiring.test.ts is still the cleverest test in this branch. A source-level regression test that nobody can silently break costs almost nothing and prevents an entire class of "I removed the disarm call and nothing broke in CI" mistakes.
  • pkg.version import path (backend/src/server.ts:5import pkg from "../../package.json") is cleaner than reading package.json at runtime from disk. The Bun ESM JSON import handles the typing.
  • forcePrSync stderr line (bin/src/oneshot.ts:491) — the comment "indistinguishable from agent stuck without opening a PR" perfectly captures why this matters. It's the kind of observability that earns its keep the first time a user hits a flaky network during the post-stable sync.
  • escapeFence (conversation-export-service.ts:122-127) is a small touch but defends against a real footgun in user-generated transcripts.

Severity Count
🟠 Medium 0
🟡 Low 2 (--resume cold-start exclusion, runaway _oneshot retries)
⚪ Nit 1
🟢 Praise 6

LGTM to merge once LOW-1 is addressed (or explicitly accepted as out-of-scope); LOW-2 can land as a follow-up. The architectural pivot from CLI-driven to server-driven oneshot is solid, the disarm wiring is exhaustive and self-checking, and the watcher's race-safe close has the test coverage to back it up.
· feat/oneshot-and-on-merge-action

hugocasa and others added 2 commits May 13, 2026 17:37
A bare `_oneshot` is too generic and easy to collide with unrelated team
conventions; prefixing with `webmux_` makes ownership obvious at a glance and
matches the `webmux` companion label.

No behavior changes — same case-insensitive match, same precedence over the
plain `webmux` label.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Opening a closed worktree (the dashboard's "Enter" / Open button on a stopped
session) is "let me peek at the agent's progress", not "I'm taking over". The
disarm should fire only on actual interaction — terminal input, chat send,
upload, etc. — which already happen via their respective endpoints.

Drops the `open-worktree` disarm + the corresponding wiring-test entry. Close,
archive, and merge still disarm (those are decisive end-of-run actions).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@hugocasa
hugocasa marked this pull request as draft May 13, 2026 15:50
@hugocasa
hugocasa marked this pull request as ready for review May 13, 2026 15:50
@claude

claude Bot commented May 13, 2026

Copy link
Copy Markdown

Claude finished @hugocasa's task in 5m 40s —— View job


Code Review — PR #238 (round 5)

Reviewing the new commits since round 4: efed8b8 (MED-1 cold-start fix + LOW batch), 9c4840e (label rename _oneshotwebmux_oneshot), and 4dfdda0 (drop disarm on open-worktree). All four prior-round LOW carry-overs from round 4 are addressed; the round-4 MED-1 fix is sound and now has crisp scaffolding tests.

This round I have no merge-blockers — just a few small notes, mostly carry-overs and one new edge case in the 4dfdda0 change.

  • Read AGENTS.md / project conventions
  • Survey commits since round 4 (efed8b8, 9c4840e, 4dfdda0)
  • Verify prior-round findings stay fixed
  • Inspect the new 4dfdda0 commit ("don't disarm when reopening a closed session")
  • Look for new bugs or regressions
  • Post the review

Verification of prior-round findings

Concern Status
MED-1 (round 1-3): fresh-worktree closed race ✅ Fixed in efed8b8 at oneshot-watcher-service.ts:73-81. New cold-start guard + genuine close after grace tests at oneshot-watcher-service.test.ts:275-309.
MED-2 (round 2): disarmAttempted re-arm cache ✅ Fixed — the sticky-cache was replaced with a direct projectRuntime.getWorktreeByBranch(branch)?.oneshot check at server.ts:1808,1817. Re-arms now disarm correctly on the same WS.
L1 (round 3): runtime-state mirror on watcher disarm ✅ Fixed at oneshot-watcher-service.ts:124-130.
L2 (round 3): webmuxVersion plumbed but not wired ✅ Fixed — server.ts:1288 now passes pkg.version.
L3 (round 3): forcePrSync silent error ✅ Fixed — bin/src/oneshot.ts:486-492 logs to stderr.
L4 (round 3): ensureWorktreeReady returns at closed ✅ Fixed — bin/src/oneshot.ts:586-591 excludes closed.
LOW (round 2): no unit test for disarmOneshot ✅ Fixed at lifecycle-service.test.ts:1644-1679 — covers happy path + idempotence + field preservation.
LOW (round 2): void oneshot lint workaround ✅ Fixed — lifecycle-service.ts:307-309 now uses delete nextMeta.oneshot.
MED-1 (round 2): README _oneshot retrigger ✅ Fixed by label-dedup eviction (linear-auto-create-service.ts:90-105).

🟡 LOW-1 — 4dfdda0 removes disarm-on-open: a still-running, armed oneshot opened from the dashboard no longer disarms

backend/src/server.ts:1023-1036 (after 4dfdda0): apiOpenWorktree intentionally drops the disarm call. The commit message argues: opening a closed session is "peek at progress", not "I'm taking over". That's correct for the finished oneshot → reopen to read history flow (the watcher disarmed at end-of-run, so meta has no oneshot block to clear anyway).

The edge case is opening an armed oneshot that's still running. Sequence:

  1. CLI fires webmux oneshot foo --prompt …. Worktree armed, agent running.
  2. User opens the dashboard, clicks "Open" on the worktree.
  3. Before 4dfdda0: apiOpenWorktree disarmed (because no oneshot body in this open call). Watcher stops monitoring; user can read the conversation panel without anything terminating.
  4. After 4dfdda0: no disarm. Watcher continues. If the user just reads (no terminal input, no chat send), and the agent reaches idle/stopped, the watcher fires close + post 15 s later. The user thinks "I opened this to inspect, why did it just close?"

This is a behavior regression for the "I want to peek at what the autonomous agent is doing without disabling its autonomy" use case — but only matters if the user also doesn't interact. Once they type or send a chat, disarm fires correctly.

The two reasonable resolutions:

  • Document it. The current behavior is defensible: peek-only doesn't disarm; only an action does. Maybe note this in the README's auto-create section.
  • Re-introduce disarm on open but only when the caller doesn't pass oneshot. That's the if (!oneshot) await disarmOneshotIfArmed(...) line 4dfdda0 removed.

I'd lean toward option 2 if the "peek without disarming" use case isn't important — the symmetry with close/archive/merge is cleaner. Up to you.

Discuss this →


🟡 LOW-2 — Claude --resume without --prompt is broken (carry-over from round 4)

bin/src/oneshot.ts:586-591 excludes status === "closed" from "ready". This was the round-4 L4 fix and is correct for fresh oneshots. But for webmux oneshot --resume <branch> without --prompt, the resumed Claude session never fires UserPromptSubmit (the only event that moves Claude's lifecycle off closed), so:

  1. apiOpenWorktree arms meta with oneshot: { autoCloseOnDone: true } (bin/src/oneshot.ts:777-783 always passes oneshotConfig).
  2. Server spawns claude --continue (no prompt). TUI opens, lifecycle stays closed.
  3. ensureWorktreeReady times out at 60 s ⇒ CLI exits 1.
  4. Additionally the watcher's idle-grace fires after 15 s ⇒ session closed + (if --linear, but that's rejected with --resume) post.

Codex is fine because codex resume --last fires SessionStart regardless of prompt presence — lifecycle moves to running immediately.

parseOneshotArgs already permits --resume without --prompt (oneshot.test.ts:21-26), so this is a partial-support corner. Two paths:

  • Require --prompt with --resume in parseOneshotArgs — the simplest signal that "oneshot resume = nudge a stuck agent", not "attach for streaming".
  • Treat --resume without --prompt as a no-op openWorktree that doesn't arm. Lets the user resume via webmux's normal attach flow without re-running the oneshot machinery.

Neither is essential; the cost today is just a confusing CLI timeout when the user runs the wrong combination.


🟡 LOW-3 — runOneshotForIssue retries every 60 s on permanent failures (carry-over from round 4)

backend/src/services/linear-auto-create-service.ts:131-134 catches errors from runOneshotForIssue but does NOT add to processedIssueIds. The most common cause is lifecycle-service throwing Branch already exists (409) when the issue's branch matches a pre-existing local branch that isn't a worktree (git branch myname/eng-42-fix created out-of-band, then webmux_oneshot added to ENG-42). The poll cycle retries every 60 s with the same error.

Same cure as before — catch the LifecycleError and add to processedIssueIds with a single log line. A user fixing the branch out-of-band can reset by removing+re-adding the label (which already evicts the dedup entry per the round-2 fix).

} catch (err: unknown) {
  const msg = err instanceof Error ? err.message : String(err);
  log.error(`[linear-auto-create] failed to launch oneshot for ${issue.identifier}: ${msg}`);
  // Don't retry every poll cycle on permanent failures (e.g. branch already
  // exists locally outside the worktree set). Dedup eviction on label change
  // still lets the user re-trigger by removing + re-adding the label.
  processedIssueIds.add(issue.id);
}

(Same edit at line 152 for the create-issue path.)

Fix this →


⚪ NIT — Watcher 15 s grace may be too tight for Claude cold-start on heavy repos

backend/src/services/oneshot-watcher-service.ts:11IDLE_GRACE_MS = 15_000. For Claude on a fresh oneshot:

  1. materializeRuntimeSession ⇒ tmux session created, agent CLI invoked.
  2. Claude takes time to load (model warm-up, MCP servers, working-dir scan).
  3. UserPromptSubmit fires after the user prompt is processed — depends on the prompt length, tool initialization, model latency.

On a slow machine / large repo, the gap between materialize and the first UserPromptSubmit could exceed 15 s, causing the watcher to fire mid-cold-start. The round-4 fix made closed go through the grace path (was previously immediate-terminal), so the watcher will now wait the grace window — but if the agent is still in cold-start past 15 s, it'll still fire.

This is a tuning issue, not a bug — bumping to 30_000 would cover the worst case I can think of without meaningfully slowing real end-of-run detection. Or expose it as a per-oneshot config field.


🟢 Praise

  • 4dfdda0 is well-reasoned and minimal — three-line diff + test entry removal. The commit message captures the model: "open = peek; interact = take over." Even with my LOW-1 concern, this is the right framing.
  • efed8b8 lands four fixes in one tight commit with surgical edits — the cold-start race fix is ~10 lines in oneshot-watcher-service.ts:66-80 plus two tests that precisely capture the discriminating behavior.
  • Label rename _oneshotwebmux_oneshot is a quiet but high-leverage change. A bare _oneshot was an accident waiting to happen in any team that uses leading-underscore labels for their own conventions; the namespaced version makes ownership obvious.
  • disarmAttempted → live projectRuntime check at server.ts:1808,1817 is cleaner than the original sticky-cache. No cache invalidation, no per-connection state — just read the truth on each input.
  • server-disarm-wiring.test.ts — still my favorite test in this branch. Maintains the invariant that every "user took over" endpoint disarms, without needing per-endpoint integration tests.
  • oneshot-watcher-service.test.ts:275-309 — the two new cold-start tests are clean: one asserts the watcher doesn't fire immediately on closed, the other asserts it does fire after grace. Together they pin the discriminating behavior precisely.
  • disarmOneshot is now unit-tested at lifecycle-service.test.ts:1644-1679 — covers field preservation + idempotence. Closes the round-2 LOW.

Summary

Severity Count
🔴 High 0
🟠 Medium 0
🟡 Low 3 (open-disarm edge, --resume corner, runaway _oneshot retries)
⚪ Nit 1 (watcher grace tuning)
🟢 Praise 7

LGTM to merge. The three LOW items are edge cases — none of them affect the happy path. LOW-1 (4dfdda0 peek-doesn't-disarm) is worth a quick UX call before merge; the others can land as follow-ups. The architectural pivot from CLI-driven to server-driven oneshot watcher is solid, the disarm wiring is exhaustive and self-checking, and the round-4 MED-1 fix has the test coverage to prevent regressions.
· Branch

… failures

L2: `webmux oneshot --resume <branch>` without `--prompt` was silently broken
for Claude — `claude --continue` doesn't fire UserPromptSubmit, so lifecycle
stays "closed" and the CLI times out at 60s with exit 1. Codex worked because
`codex resume --last` fires SessionStart unconditionally. The parser now
rejects this combination with a clear error pointing at the dashboard for
re-attach-without-prompting.

L3: linear-auto-create now adds the issue id to `processedIssueIds` on
failure as well as success. Without this, a permanent error (e.g. "Branch
already exists" for an out-of-band local branch) would retry every 60s
forever. The label-eviction pass still lets the user retrigger by removing
and re-adding the label after fixing the underlying issue.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@hugocasa
hugocasa requested a review from centdix May 13, 2026 16:03
@centdix
centdix merged commit d43ad25 into main May 18, 2026
2 checks passed
@centdix
centdix deleted the feat/oneshot-and-on-merge-action branch May 18, 2026 09:17
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.

2 participants