[chrisohearn]

Round 4, ledger key r4/w5-workspaces, items 30–34. One fork migration: provisional id 13 (013_ScheduledRunHistoryIndex, a partial index; see item 33). Re-check the tip's highest id before merging. Every acceptance check below is a read-only query; run it against ~/.t3/userdata/statev2.sqlite with sqlite3 -readonly, or call the named MCP tool. <DEPLOY> is the ISO time the new build began serving.

30. Worktree base: clear error when no repository or no base exists

• Evidence. The 7 setup failures: 3 sync runs (05:11, 06:11, 07:11Z) and 1 Jarvis heartbeat (18:10Z) all failed with Git command failed in GitWorkflowService.resolveWorktreeBaseRemote (...): Failed to resolve the VCS driver. The sync folder ~/.t3/scratch/t3-upstream-maintenance only became a git repo at 07:40Z (anchor commit, no remote on purpose), so the three sync failures predate the repo; the heartbeat predates fix(server): run scheduled tasks in the project folder when Git confirms it is not a repository #40 (merged 20:29Z). With the anchor repo the no-remote path already works (resolveWorktreeBaseRemote returns null, startRef stays the local baseRef; 18 runs completed since).
• What was still broken. checkWorktreeBase (non-repo and missing-base errors) only runs from ws.ts and ThreadLaunchMcpService. A scheduled task saved with a worktree strategy, or a retried launch, skips it and reaches Git as the opaque error above, or as a raw git "invalid reference".
• Already fixed for the reported failures by the anchor repo (07:40Z) and PR fix(server): run scheduled tasks in the project folder when Git confirms it is not a repository #40. I would have kept this item test-only, but a test I wrote for the remaining gap fails on the tip, so the one code change below stays.
• Change. prepareInBackground runs the same checkWorktreeBase before provisioning (branch-name conflicts stay ignored there, since provisioning flattens them). Result: Workspace preparation failed during provision worktree: Project "X" (/path) is not a Git repository. or ... Base ref "main" does not resolve to a commit in project .... The check adds a few local git calls (repository root, one or two ref lookups, a remote lookup; no network) to every worktree launch that was already checked at its entry point; I left them in. A base found on a remote or locally still proceeds; no remote falls back to the local base, as before.
• Tests. ThreadLaunchService.test.ts "a launched worktree $name fails with a clear message" (2 rows; both fail without the change). Harness default hasRefNamed added. fix(server): run scheduled tasks in the project folder when Git confirms it is not a repository #40/fix(server): scheduled runs record their real outcome and refuse a stale project-folder workspace #71 cases (OrchestratorMcpService.test.ts non-git root, ScheduledTaskService.test.ts project-folder workspace) still pass.
• Acceptance. SELECT count(*) FROM orchestration_v2_projection_turn_items WHERE type='error' AND updated_at > '<DEPLOY>' AND json_extract(payload_json,'$.failure.message') LIKE '%Failed to resolve the VCS driver%'; → 0.

31. Missing workspace folder

• Evidence. The 3 "missing /tmp" failures (01:00:11, 01:01:12, 01:04:14Z) are one thread (mcp:launch:2537…) whose project root is /tmp/prospective-allowance-behavior.BCAm7HQY/review. An agent created that project through MCP at 00:40Z; the machine rebooted at 00:59:47Z (uptime -s); the folder was gone 24 s later. It was not a worktree. The server never places worktrees or scratch folders under /tmp (worktrees: ~/.t3/worktrees; rg tmpdir|/tmp apps/server/src finds only caches, test fixtures and command help text). So there is nothing to stop placing under /tmp.
• Decision: do not recreate. Recreating a missing project root would start the agent in an empty folder and hide the loss. The run already fails naming the path; the message now also says why when the folder is under a temp dir.
• Change. (a) ProviderWorkspaceMissingError adds "It is under a temporary folder, which the system clears at reboot." for such paths. (b) New guard project/temporaryFolder.ts: the path is normalized (~, relative, .., symlinks, so macOS /private/tmp is seen) and counts as temporary when it is under /tmp, /var/tmp, /dev/shm, /run/user/<uid> or macOS temp dirs, or when statfs reports tmpfs. os.tmpdir()/TMPDIR is not trusted by itself and /run/media (external drives) never matches. t3_project_create with a workspaceRoot there, and t3_thread_launch into a project rooted there, return a warning field. Creation still succeeds.
• Tests. temporaryFolder.test.ts (prefixes, /run/media, .., a symlink from /tmp to a persistent folder); handlers.test.ts launch warning (warns for /tmp/review, not for /projects/...); ProviderSessionManager.test.ts asserts the new sentence.
• Acceptance. SELECT count(*) FROM orchestration_v2_projection_turn_items WHERE type='error' AND updated_at > '<DEPLOY>' AND json_extract(payload_json,'$.failure.message') LIKE '%workspace folder no longer exists%/tmp/%' AND json_extract(payload_json,'$.failure.message') NOT LIKE '%temporary folder%'; → 0.

32. Items left running on finished runs

• Evidence. Active items on runs that already ended: todo_list 926 (882 completed runs, 24 interrupted, 20 failed, 1 cancelled), compaction 15; oldest todo_list 2026-10-07. Only todo_list and compaction are closed. command_execution, dynamic_tool, subagent can be real background work that outlives a run (they have their own recovery indexes), and reasoning was left alone too, so none of them are touched.
• Change. (a) EventSink adds turn-item.updated events for the run's still-active todo_list and compaction items, in the same transaction as the terminal run.updated (all write paths share normalizeEvents; items in the same batch are taken from the batch). Status: completed run → completed, interrupted → interrupted, failed/cancelled → cancelled. They are real events, so clients see them live and a projection rebuild reproduces them. (b) projectThreadProjectionForWire shows such items on already-ended runs as finished, so the existing ~940 read as finished with no data migration. Rolled-back runs close their items as cancelled. If the end-of-run read of old items fails (one unreadable item), it logs a warning and the run-ending write proceeds; those items then close on read.
• Compaction boundary. A compaction still in flight when its run is marked ended is closed at that moment. Idle compaction starts its own new run, so it is not cut short, and a late provider update to the same item id is stored after the close and wins. The stale comment in ProjectionStore is updated. Logic is in RunItemSettlement.ts.
• Tests. FoundationPersistence.test.ts "closes run-bound items still active when a run ends" (completed/failed/interrupted/rolled_back; asserts a dynamic_tool stays running and a todo list written in the ending batch is closed). WireProjection.test.ts read case.
• Acceptance. SELECT count(*) FROM orchestration_v2_projection_turn_items i JOIN orchestration_v2_projection_runs r ON r.run_id=i.run_id WHERE i.type IN ('todo_list','compaction') AND i.status IN ('pending','running','waiting') AND r.status IN ('completed','failed','cancelled','interrupted') AND r.completed_at > '<DEPLOY>'; → 0. (Rows from before the deploy stay stored as running by design; they read as finished.)

33. Recent failure history in list_scheduled_tasks

• Evidence and a correction. The scheduled task row keeps only the last run's status. The brief's "84 runs, 3 failed, 6 cancelled" counts every run in the sync threads, including 63 PR-watch/restart-continuation runs. The 6 cancels are PR-watch wake runs superseded by a newer one (e.g. message:pr-watch:… in threads 076901c7…, 852e1120…, e42e6166…), not scheduled fires. The scheduled fires are 22 runs: 3 failed (the preparation failures above), 0 cancelled, the rest completed. History is therefore counted per scheduled fire.
• Change. ScheduledTaskService.reliability() (new ScheduledTaskReliability.ts) derives history from the runs themselves (message id scheduled-task-message:<taskId>:<ms>:<trigger>, or …:webhook:delivery:<uuid> for webhook runs, which are counted too), so no second record can drift and no migration is needed. Per task, last 7 days: recentFailures {failed, cancelled, lastAt, lastStatus, lastError} (lastError = latest failed run's error text, capped at 500 chars). A dispatch that failed before a run existed is added from the task row. Present on list_scheduled_tasks only, and only when there is something to report. A failed history read never hides the task list. The 7-day read runs on every list_scheduled_tasks and bridge_status, so fork migration 13 adds a partial index on requested_at for failed and cancelled runs (query plan: SEARCH … USING INDEX); no scan of the full run table.
• Tests. ScheduledTaskService.test.ts "reliability counts the last 7 days…" (7-day window, error text, row-only failure, no double count, webhook run); OrchestratorMcpService.test.ts merge case.
• Acceptance. Call list_scheduled_tasks; for task <ID>, recentFailures.failed|cancelled must equal SELECT status, count(*) FROM orchestration_v2_projection_runs WHERE status IN ('failed','cancelled') AND requested_at >= strftime('%Y-%m-%dT%H:%M:%fZ','now','-7 days') AND substr(json_extract(payload_json,'$.userMessageId'),1,length('scheduled-task-message:<ID>:'))='scheduled-task-message:<ID>:' GROUP BY status; (plus at most one row-only failure counted from scheduled_tasks.last_run_error). For the Hourly upstream sync task the 3 preparation failures show until 2026-11-19.

34. Missed scheduled runs

• Change. Pure missedScheduleRun (Schedule.ts): an enabled, non-webhook task whose nextRunAt passed by more than one interval (interval: everyMs; fixed time: gap to the following slot, computed in the scheduler's time zone; one-time: the existing 10-minute grace). Reason run_in_progress when the row is still running (later slots are skipped while it runs; listDueTasks skips those rows) versus not_dispatched. Reported as missedRun {dueAt, overdueMs, reason} in list_scheduled_tasks, and as missedScheduledRuns [{scheduledTaskId, dueAt, overdueMs}] in bridge_status (ids and times only).
• Tests. Schedule.test.ts (interval, running, a Los Angeles weekly case that a UTC reading gets wrong, disabled/no-next/webhook, fixed-time, one-time); BridgeStatusService.test.ts; OrchestratorMcpService.test.ts.
• Acceptance. bridge_status.missedScheduledRuns is [] on a healthy server and equals SELECT task_id FROM scheduled_tasks WHERE enabled=1 AND json_extract(schedule_json,'$.type')='interval' AND next_run_at IS NOT NULL AND (unixepoch('now') - unixepoch(next_run_at))*1000 > json_extract(schedule_json,'$.everyMs'); for interval tasks (empty today).

Notes for review

• check:changed passes against origin/integration/mcp-events-20261109 (16 test files); the default base origin/main selects 109 files and one unrelated step fails there, so run it with the integration ref. The 200 test files in the changed directories also pass.
• Model-facing text: result fields recentFailures/missedRun on list_scheduled_tasks, missedScheduledRuns (ids, due time, overdue ms) on bridge_status, and warning on t3_project_create and t3_thread_launch. The CI agent-context check refuses growth, so the new fields carry no descriptions, the bridge_status description is unchanged, and mcpToolListJsonBytes stays at the base's 56426. I did not run the paid mcp-tool-use eval (no API keys here); tool inputs and guidance are unchanged.

🤖 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-13T02:56:38.023109Z We couldn’t complete this review. Comment @codex review to try again.	973c686	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]

Independent Claude Opus review of 17e6963 (review of record)

Mergeability: MERGEABLE / CLEAN against base abaa6b1 (after #66 and #83). The merged tree is clean. Its only shared file is ProjectionStore.ts, where this PR changes one comment, and no new ScheduledTaskService or git mock on the base side reaches the new code. CI is green on the head. That run started at 03:00Z, before the base moved.

P2

• scheduledTasks/ScheduledTaskReliability.ts:37: webhook deliveries are never counted. The deliveryId is delivery:<uuid> or delivery:relay:<id> (ScheduledTaskService.ts:2423), so the message id ends in :webhook:delivery:<uuid>. /^(.+):webhook:[^:]+$/ returns null for that, the row is skipped, and the task-row fallback excludes webhooks, so webhook tasks never show recentFailures. Use :webhook:(.+)$, or split on the first :webhook:, and add a webhook case to the test.
• project/temporaryFolder.ts:3: /run matches /run/media/<user>/..., where udisks mounts external drives on Arch/Omarchy. A project on a persistent drive gets a false "cleared at reboot" warning. provider/Errors.ts:27 then tells the user that an unplugged drive "is under a temporary folder, which the system clears at reboot". Drop /run or exclude /run/media. NodeOS.tmpdir() also trusts TMPDIR, so a persistent TMPDIR flags everything under it.

P3

• mcp/toolkits/project/handlers.ts:111: the warning checks the raw input, not the normalized created.workspaceRoot (resolve + expandHome + trim). So ~/..., relative paths and /tmp/../x are judged wrongly. Nothing calls realpath, so symlinks into /tmp and macOS /private/tmp are missed. These are false negatives only.
• scheduledTasks/Schedule.ts:117: the fixed_time gap uses DateTime.make(nextRunAt), which is UTC, so the slot's time of day and weekday are set in UTC, not in the scheduler's zone (localNow). A daily 09:00 PT task gets a 16h gap instead of 24h. It can only flag early, and the tests use UTC only.
• orchestration-v2/EventSink.ts:443: the run-ending transaction now reads turn items and catches only ThreadNotFound. A read or decode error on one old item now fails the write that ends the run. Consider orElseSucceed to an empty list.
• RunItemSettlement.ts: closing a running compaction as completed also moves the runIdsSinceLastCompaction boundary (ProjectionStore.ts:3511) for healthy runs. The active-run clauses guard it, but the PR doesn't mention this change. rolled_back runs never close their items.
• WireProjection.ts:298: the read-time mapping covers full snapshots only. History pages (ProjectionStore.ts:6754) and live events still show stuck items as running; the live DB has 961.
• ScheduledTaskReliability.ts:69: on the live DB (17.6k runs) the query does a full SCAN in about 9 ms. It runs on every list_scheduled_tasks and every bridge_status call, and list reads the task rows twice. An index on (status, requested_at) would keep it bounded. Slots skipped as "previous occurrence still queued" appear in neither 33 nor 34.
• ThreadLaunchService.ts:520: the check runs on every worktree provision, entry-point launches included, because no commandId is passed. That adds 2-5 git calls, and its errors are tolerated, so the cost is latency only.

Verified: (a) only todo_list and compaction items close, from pending, running or waiting. Tool and subagent items are untouched, the close happens inside sql.withTransaction for every normalizeEvents caller, batch copies win, and event ids are deterministic. (b) The comparison is a strict >, and disabled, webhook and null tasks are skipped. A one-time task gets a null nextRunAt after it runs. running is a dispatch stuck between markRunning and markCompleted, which is the right meaning for run_in_progress. (c) Live error items decode as expected, colon task ids parse, and the dedupe uses the same startedAtMs. (e) Otherwise healthy runs are unchanged.

Verdict: no blocking findings

[chrisohearn]

Independent delta review of 1714f3a (review of record, follows 17e6963 review)

Scope: rebase onto 7836910 (#84). git range-diff shows only commit 1 changed: closeEndedRunItems now runs after stampRunStops and normalization. Also the fix commit 1714f3a. MERGEABLE/CLEAN, CI green on the head. I read the source only and ran no tests.

• P2 webhook counting: partly fixed. :webhook:delivery:<uuid> now matches, and the test covers it. Relay deliveries are still dropped. Their id is delivery:relay:<id> (ScheduledTaskService.ts:2430), so the key ends :webhook:delivery:relay:<id>, and (?:delivery:)?[^:]+$ rejects the extra colon. Use /^(.+?):webhook:(.+)$/ and add a relay case.
• P2 temp folder: fixed. Paths are resolved (~, relative, ..) and realpath'd through the nearest existing ancestor. A statfs tmpfs check is added, and the prefix list is narrow (/run is now only /run/user/<uid>). TMPDIR is no longer read, and /run/media/ returns false before any other check. Tests cover symlinks and /run/media.
• P3 item 34 zone: fixed. The gap is measured in now.zone, the scheduler's localNow. The new Los Angeles weekly test tells the two readings apart (7d vs 6d16h).
• P3 item 32: fixed. A failed end-of-run read logs a warning and returns no items, so the run still ends. rolled_back maps to cancelled, and the test covers it. The PR body now explains the compaction boundary.
• P3 item 33: fixed. 013_ScheduledRunHistoryIndex is a partial index only, CREATE INDEX IF NOT EXISTS ... (requested_at) WHERE status IN ('failed', 'cancelled'). The query uses the same literal IN term, so SQLite can prove the index applies. It follows FORK.md: a fork file, registered as id 13, under a new name. Building it is one pass over about 17.6k rows and indexes only failed and cancelled runs, so expect tens of ms, once.
• Merge order: #85, #87, #90, #91 and #92 also hold provisional id 13. Per FORK.md, renumber to the tip's highest id + 1 at merge time.
• New P3: isTemporaryFolder now calls existsSync, realpathSync and statfsSync synchronously. It runs on every t3_thread_launch with a project (ThreadLaunchMcpService.ts:376), on project create, and in an error-message getter. Errors fall back to false, but nothing bounds a hang: a dead hard-mounted NFS, SSHFS or gvfs path would block the server's event loop, where before it was a string compare. Also, /run/user/<uid>/gvfs (network shares) and /run/user/<uid>/doc now match as temporary. Consider an async statfs with a timeout and excluding gvfs and doc.
• Healthy runs: nothing new breaks them. The broader catch only removes a failure path. Closing items on rolled_back adds one indexed read per ended run in the rollback batch, on items that reads already hide.

Verdict: no blocking findings

[chrisohearn]

Independent delta review of f2133a8 (review of record, follows 1714f3a review)

Scope: rebase onto 7eddc7f (#85, #93) plus f251fc2 and f2133a8. git range-diff shows that commits 1-7 match, apart from the migration renumber and the context-budget lines the rebase dropped (base and head both have 57258). I read the source only and ran no tests. CI on this head is still queued; only Agent context budget has passed.

• Async temp-folder check: fixed. isTemporaryFolder is now an Effect. It probes with fs/promises (access, realpath, statfs) under Effect.timeoutOption(500 ms). A timeout or a rejected probe falls back to the name check. Both callers yield* it: ThreadLaunchMcpService.ts:376 and project/handlers.ts:111. The error getter in Errors.ts uses isTemporaryPath, which is lexical only. The PR adds no other sync fs calls outside tests. Effect.timeoutOption and Option.flatMapNullishOr both exist in effect 4.1.2. The hung-probe test uses TestClock.
• gvfs/doc: fixed. PERSISTENT_MOUNTS (/run/user/<uid>/{gvfs,doc} and /run/media/) is tested on the lexical path before any probe and again on the resolved path. Tests cover both mounts and that no probe runs under them.
• Relay webhooks: fixed. /^(.+?):webhook:delivery:.+$/ matches both delivery:<uuid> and delivery:relay:<id>, the only two forms ScheduledTaskService.ts:2425-2430 builds. The new task:relay test row is counted as cancelled.
• Migration: consistent. It is 014_ScheduledRunHistoryIndex.ts, registered as [14, "ScheduledRunHistoryIndex"] after #85's 013_WorkLedgerHygiene. The "registers every fork migration file" test checks that the file and the registration match. It is still a partial index, CREATE INDEX IF NOT EXISTS ... (requested_at) WHERE status IN ('failed','cancelled'). Renumber it again if another fork migration lands first.
• New P3 (non-blocking): the timeout stops the wait, not the fs call. On a dead hard mount, each launch leaves one libuv threadpool thread (4 by default) stuck until the mount returns. After 4 such launches, async fs, dns and crypto work across the server would queue behind them. The event loop itself stays free. A per-path cache, or a single-flight probe, would bound this.
• Healthy runs: nothing new breaks them. A normal path adds a few async fs calls to launch and project create. The worst case adds 500 ms.

Verdict: no blocking findings