[chrisohearn]

Two scheduled-task bugs, fixed together because they share the launch path. Updated after the substitute review: the stored and wire statuses are the four existing ones again.

1. A run no longer reads as succeeded before it has finished

Problem. runTask wrote last_run_status = succeeded as soon as the launch was accepted, and run_scheduled_task_now returned succeeded at that moment too. A run that then failed workspace preparation read as succeeded while it was preparing. On 2026-11-12 the Jarvis inbox heartbeat (17:44 UTC) showed succeeded for 26 minutes before it failed with Workspace preparation failed during provision worktree: … Failed to resolve the VCS driver. The outcome follower from pingdotgg#34815 lived only in memory, so a restart (every deploy) dropped it. It also returned early in several cases without writing anything.

Fix.

• Accept. An accepted run writes back the status the task had before the run (never for a first run), together with that status's error. It no longer writes succeeded.

• In progress. Accepted runs whose outcome is pending are held in memory. Results leaving the service carry an optional runInProgressSince, which is never stored. Web shows such a task as running (scheduledTaskRunDisplayStatus). MCP reports started through its own OrchestratorMcpScheduledTaskRunStatus, so run_scheduled_task_now returns started.

• Outcome. The follower writes succeeded for a completed run. Any other terminal status writes failed, with the root provider failure (for example the preparation error) or a generic reason. The write is guarded by last_run_at = ? AND last_run_status <> 'running', and also requires the run to still be the task's pending one in memory.

• Every path ends. These cases now record failed:

  • a missing or soft-deleted (deletedAt) thread;
  • ThreadNotFound or RunNotFound during the wait;
  • a run that never started.

  A steered message is followed into the run it joined; the comparison also accepts the restart continuation id. A run still active after 24 h keeps its previous status.

• Restart. At startup each task's latest run is found again by rebuilding its message id from last_run_at and the trigger (scheduled, manual, or the webhook delivery id).

  • A run that ended while the server was down gets its outcome recorded.
  • A run still active is followed again.
  • A row with no message, or whose recorded status already matches the run, is left alone.

• Missed slot. A missed-slot failure recorded while an older run is pending stands; that run's outcome no longer overwrites it.

• No migration.

2. #69: a saved project-folder workspace in a folder that became a repository

Problem. MCP stores {type:"root"} for an unbound task whose folder is not a repository (#40). After git init or a clone there, the task kept running directly in what is now a shared checkout.

Fix: fail closed. Before an unbound task launches, refuseStaleRootWorkspace checks its project folder with findGitEntry. The check applies when the task has a root strategy, creationSource: "mcp" (MCP always derives the workspace), and is not in a scratch project. If a .git entry exists, the run fails with an error naming the .git that was found. When it is in a parent folder, the error warns that an update would make a worktree of that parent repository. The way out: an MCP schedule_task update re-derives an unbound MCP root workspace even when the binding does not flip.

Edits now keep the stored creationSource, as mobile already did. So renaming an MCP task in Settings no longer exempts it from the recheck.

I chose to fail closed instead of switching to a worktree automatically. Switching at launch needs base-ref resolution, which lives in the MCP layer. It would also start creating per-run branches and worktrees in a folder someone has just cloned or initialised without asking.

Bound tasks are unchanged.

Known limits.

• A project folder deliberately chosen in Settings or mobile for an MCP-created task in a repository is still refused. Choosing Worktree works.
• A web-created task that is later unbound through MCP is not rechecked.
• A refused once-only task is used up, as with other dispatch failures.

Compatibility

• Stored and wire statuses are the original four. runInProgressSince is an optional key, and older decoders drop it. Upstream desktop and store mobile clients, and a guard rollback to an older server build, all read these rows unchanged.
• started exists only in MCP output, which is encoded for models and not decoded by clients. The mcp-tool-use eval was not run; the only model-facing change is that enum value, and the tool description already says "Success means started, not finished."
• While a run is pending, the stored row shows the previous run's status and error. Older clients see that; this fork's web shows running.
• succeeded and failed stay due-eligible, so a run longer than its interval can overlap the next one. That is unchanged from before.
• No before/after capture; no component or variant changed.

Tests

• Outcome table: runNow returns the previous status with runInProgressSince, then the row records:
  • failed, interrupted, cancelled, and completed → succeeded;
  • a steer into a completing run → succeeded;
  • a workspace-preparation failure on an unbound first launch;
  • a soft-deleted thread, a thread missing during the wait, and a removed run → failed.
• Overlap: a run still going when the next one starts cannot overwrite the newer run.
• Restart: manual and webhook runs are found again and their outcomes recorded; a row without a message is left as it was.
• Missed slot: a slot missed during a restart is not overwritten by the run still going from before. This test fails without the fix.
• Stale root:
  • Refused: an unbound MCP task; an MCP task whose parent folder became a repository; an MCP task later edited in Settings.
  • Launched: a bound task, a folder chosen in Settings, a scratch project, a folder that is still not a repository.
• MCP update: an unbound MCP root is re-derived; a Settings-chosen root and a saved worktree are kept.
• Integration tests assert the accepted, pending state. Their harness runs never finish (the Scratch test asserts starting), so final outcomes are covered at the unit level.
• vp run check:changed passes: 89 files, 2313 tests.

Model and harness: Claude Opus 5.5 in Claude Code (T3 Code).

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	❌ Failed 2026-11-12T21:09:24.803295Z We couldn’t complete this review. Comment @codex review to try again.	3f78bf9	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[chrisohearn]

Substitute review (Claude Opus 5.5), done in place of the hosted Codex reviews, which are blocked by a usage limit. Head 3f78bf9. Verdict: merge after fixes. The #69 half is sound; the new started literal has to go.

High 1: a new literal in an RPC contract

• Why it matters: ScheduledTaskRunStatus is a strict Schema.Literals (contracts/scheduledTask.ts:217), used in the scheduledTasks.subscribe stream and the upsert, setEnabled and runNow results (rpc.ts:2005-2034). No ParseOptions are set. The owner runs the upstream nightly desktop app and the store mobile app against this server. Verified with effect 4.1.2:
  • The list uses ForwardCompatibleArray (upstream feat(server): scheduled tasks can run on a webhook pingdotgg/t3code#15085), so in upstream clients a started task silently disappears from Settings, the Automations panel, timeline titles and mobile ThreadFeed. A store build from before 2026-10-05 would fail the whole list.
  • ScheduledTaskRunNowResult fails to decode, so Run now from upstream desktop or mobile always shows an error, even though the run started. Toggling or editing a started task errors too.
  • The hourly sync spends many minutes in this state every hour.
• Fix: keep the four statuses on the wire and in the database. At accept, write back the status from before the run, not started or succeeded. Expose progress through an optional field such as runInProgressSince (older decoders drop unknown keys, verified), added only where results leave the server (ws.ts and the MCP mapping). MCP text can still say "started".

High 2: a guard rollback poisons the rows

• Where: the base decodeTask (line 180) decodes strictly.
• What breaks on an older build with a started row:
  • listRows fails the whole list for every client (:552-559).
  • listDueTasks skips the row, so the task never runs (:446-463).
  • findTask, Run now and edits fail.
• Nothing resets it: the hourly sync is likely to be the stuck row, and it's the job that produces releases.
• Fix: the same as High 1. The database only ever holds the old statuses. Avoid a migration: servicePreflight.ts:94-101 would drop every older build as a rollback target.

Medium

• M3. A soft-deleted thread stays started forever (followRunOutcome, :903-975).
  • getThreadShell returns deletedAt rows, so a shell === null check misses them.
  • A ThreadNotFound or RunNotFound error is only logged, and the row is retried on every restart.
  • Fix: treat deletedAt != null as missing, and record failed on those errors.
• M4. creationSource resets to "web" on any edit (:1688). A Settings rename of an MCP task then exempts it from Saved root strategy for an unbound scheduled task goes stale if the folder becomes a repository #69. Fix: preserve the stored value, or record that the workspace was derived.

Low

• L5. The Saved root strategy for an unbound scheduled task goes stale if the folder becomes a repository #69 message names the project folder even when the .git is in a parent folder. The suggested update then derives a worktree of that parent repository. Fix: report the .git path found, and warn when it's a parent.
• L6. rescheduleMissedRun with an error can overwrite a started row (:1333-1337). Acceptable; note it.
• L7. A refused once-only task is used up. That matches other dispatch failures.
• Note: started/succeeded stays due-eligible, so a sync longer than an hour can overlap the next one. Not a regression; consider an overlap policy.

Confirmed correct

• The outcome write is guarded on last_run_at.
• The resume runs before scheduler.register, so it can't create duplicate followers.
• Message ids are rebuilt exactly.
• The Saved root strategy for an unbound scheduled task goes stale if the folder becomes a repository #69 check is the exact negation of the MCP derivation gate, fails closed, and records a counted failure.
• The scratch exemption is correct.
• An MCP update re-derives the workspace.

Tests to add

• a soft-deleted or deleted thread during the wait, and RunNotFound;
• a run-in-progress row followed by the next occurrence;
• restart resume for a webhook or manual run;
• a missed-run overwrite;
• a .git in a parent folder;
• an MCP task edited in Settings.

The integration tests should wait for the final outcome, and the restart test should time out rather than hang.

The PR's Compatibility section should be corrected to match the above. The #69 half could be split out and merged on its own.

🤖 Generated with Claude Code

[chrisohearn]

Substitute re-review (Claude Opus 5.5) of the fixes 3f78bf9..c720472 (the head ffe675e adds only empty CI commits). Verdict: merge after fixing N1.

Fixed: High 1, High 2, M3, M4 and L5. The compatibility check passed: the fork's ScheduledTaskRunNowResult and ScheduledTaskListResult decode with upstream/main's scheduledTask.ts for all four statuses, with runInProgressSince dropped. started exists only in the MCP contract. Tests pass: 11 files with 276 tests, plus ClaudeAutomaticDelivery (6). Server typecheck is clean.

N1 (Medium, blocking): each restart erases missed-slot failures (ScheduledTaskService.ts:1036-1047 and :1393-1402).

• rescheduleMissedRun writes failed but keeps last_run_at on the earlier run.
• At the next startup, resumeRunOutcome rebuilds that run's message id, finds it completed, and overwrites the row with succeeded.
• Reproduced; it affects every fixed-time task.
• Fix: in the miss write, also set last_run_at = now and last_run_webhook_delivery_id = NULL. Tested: the repro passes and all 223 tests still pass. Add a second-restart test.

Low:

• N2: a race between the in-memory check and the outcome write (:927-938 vs :1410). The N1 fix closes it.
• N3: the restart skip ignores the error text, so a stale error can stay (:1041-1047). Drop the skip, or compare the error.
• N4: the "previous status" is read before markRunning (:1099 vs :1264-1270). Read it in the same transaction.
• N5: the fork web app shows a running badge next to the previous error (ScheduledTasksSettings.tsx:803). Hide the error while runInProgressSince is set, and drive the Run now button from the display status (ThreadAutomationsPanel.tsx:227).

Tests still missing: a second restart after a missed slot, and a "stored status already matches" case. The restart test lost coverage of the :scheduled message id and still uses Stream.runHead with no timeout.

🤖 Generated with Claude Code