Skip to content

Merge upstream main (95cb068c10) - #99

Merged
chrisohearn merged 247 commits into
integration/mcp-events-20261109from
sync/upstream-20261113-0321
Nov 13, 2026
Merged

chrisohearn merged 247 commits into
integration/mcp-events-20261109from
sync/upstream-20261113-0321

Conversation

@chrisohearn

@chrisohearn chrisohearn commented Nov 13, 2026 •

Copy link
Copy Markdown
Owner

Scope

Finish the existing upstream-sync candidate through pingdotgg/t3code main 95cb068c10a6cf0b33eed0a0cab3b298763fe852, before adding newer upstream commits. Candidate 0537f39fff2561b2465872c8a2021b397a59240f merges current fork integration 782fe7fa606df2f3ae88e0540bf43b2700f8ed04.

Preserves fork MCP/events, orchestration, thread freeze, scheduled-model and snooze behavior, agent registry, and fork-only migration history in t3_fork_migrations. Fork migration files and registry match integration; upstream migration files match the selected upstream cutoff. The contract merge preserves both upstream title-regeneration state and fork scheduled-model state. The MCP tool-list budget is remeasured at 58,309 bytes.

Repairs

  • 3803fc0fb7: combine the SEA Undici and upstream JSONC aliases instead of letting the later alias object overwrite the first. The packaging regression fails before the fix and all 13 packaging tests pass afterward.
  • 01a98a8397: emit the shared migration-finished marker after fork-only migration execution or legacy-history repair. The existing regression exercises both paths through the actual desktop output parser and asserts the separate migration histories. Both assertions fail before the product fix and pass afterward.
  • ef46659388: correct the regression logger's message typing after the integrated gate exposed TS2488. No failing check is waived.
  • 147b9691ac: merge the current integration branch without adding newer upstream commits; preserve both contract additions and remeasure the context budget.
  • 0537f39fff: supply upstream's required provider-config hydration dependency in the newly merged terminal test table. A focused retry reproduced all three failures and TS2741; after the one-line fixture repair all 89 terminal tests and direct server typecheck pass.

Verification

  • Refreshed integration-focused suite: 9 files / 348 tests pass, covering contracts, context budget, both migration systems, orchestration runtime, thread freeze, snoozed delivery, MCP orchestration and Codex home layout.
  • Exact-head 0537f39fff vp run check:changed origin/integration/mcp-events-20261109 --concurrency 1 passes: all 41 affected package typechecks, 345 test files / 8,678 tests (1 skipped), 26 node tests, format, lint, both knip checks, generated GraphQL and React compiler checks. The additional cross-file test pass reports 227 passing tests.
  • The previous ef46659388 gate passed: all 41 affected package typechecks, 345 test files / 8,664 tests (1 skipped), 26 node tests, format, lint, knip, generated GraphQL and React compiler checks. This is historical evidence, not a waiver for the refreshed head.
  • Both current integration and the selected upstream cutoff are ancestors of the candidate. The committed worktree is clean and has no conflict markers or whitespace errors.

Review And Release Gates

Hosted Codex review failed on its usage limit and is not requested again. The independent exact-ef466 code/security review approves the earlier tree and confirms the migration repair. The independent exact-head code/security review of 0537f39fff2561b2465872c8a2021b397a59240f against 782fe7fa606df2f3ae88e0540bf43b2700f8ed04 is complete: both APPROVE. It covers the integration delta, terminal fixture repair and merge interactions while reusing unchanged review evidence. Its final-tree check condition is satisfied. Final-head hosted checks completed with 19 successes and 2 workflow-selected mobile skips; there were no review threads, unresolved findings or running latest-head reviews at merge.

The maintainer's PR96/100 ancestry-publication and automatic-update holds have been cleared by the owning coordinator. This does not substitute for source, CI, review, release-build or deployment verification. Acknowledge-only activation remains separately owned and is not part of this work. The kernel issue is not claimed fixed.

The mandatory t3-sync-upstream wrapper exited 0 after full format/lint/typecheck checks, 8,253 passing server tests (5 skipped), the container release build and runtime smoke test, and ready read-only live-database preflights for launcher protocols 3 and 4. The PR merged as dc3ee559385459a00ff1086e1419802a8f618fcf at 2026-11-13T19:22:25Z and v0.1.10-preview.20261113.4 was published at 19:24:14Z. The candidate, merge, integration tip and release tag share verified tree 8b64c6393a45f4805019ec7ecec0947cb24f9db4. The Linux archive and SHA256SUMS are present; the archive's GitHub SHA256 digest matches the published checksum (0434d8ab481d1ac8cab9782993be9acc988c4942b94eb998e19b0250a2d4f17c).

Publication is not live deployment. A normal t3-service-update attempt deferred for 4 active threads; the existing guarded idle/three-hour-cap timer remains responsible. No --now or interrupting deployment was used. As of 19:25Z, 0.1.10-preview.20261113.3 still serves, trusted/healthy under launcher protocol 4, with upstream migration 85, fork migration 15 and zero namespace-name collisions. Local/Tailscale serving probes and T3 Connect reachability passed. The next maintenance run must verify .4 serving, guard health, launcher protocol, migration histories and both remote routes before claiming it live.

The MCP 2026-07-28 events-capability seam remains an informational compatibility note, not a proven live outage. Modern protocol support is retained. Paid model-behavior evals and live Jarvis event-wake acceptance have not been run and are not claimed passing.

Related #89; newer upstream remains for the next candidate.

Prepared by Codex in Codex CLI for the authorized hourly fork-maintenance task.

Srijan-D and others added 30 commits November 12, 2026 09:38
…preparing tile (pingdotgg#35265)

Co-authored-by: Julius Marminge <julius0216@outlook.com>
…st their pin (pingdotgg#21249)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
Co-authored-by: Yash Singh <saiansh2525@gmail.com>
… an agent started (pingdotgg#35405)

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
dpreil-ydlp and others added 14 commits November 12, 2026 18:17
Co-authored-by: David Preil <dzpreil@gmail.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
… pass (pingdotgg#26935)

Co-authored-by: Julius Marminge <julius0216@outlook.com>
…ingdotgg#35783)

* feat(mobile): secret field gets an eye toggle to show what you typed
…t frame (pingdotgg#35683)

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
… into sync/upstream-20261113-0321

# Conflicts:
#	apps/server/src/provider/agentContextBudget.json
@chatgpt-codex-connector

Copy link
Copy Markdown
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-13T05:20:59.122570Z
We couldn’t complete this review. Comment @codex review to try again.
d992ff2 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

Copy link
Copy Markdown

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

@chrisohearn

Copy link
Copy Markdown
Owner Author

Independent fallback review: #99, "Merge upstream main (95cb068)"

Reviewer: Claude Opus 5.5, read-only. This is the review of record for code and security because the hosted Codex review hit its usage limit.

  • Base: 3791db6ba6a0ce4868a5c012ec41135628c3214a (integration/mcp-events-20261109)
  • Head: fcad53a4e7bb3dafa201ad422c25cb25a7e8da8e (sync/upstream-20261113-0321)
  • Upstream tip merged: 95cb068c10a6cf0b33eed0a0cab3b298763fe852
  • Previous upstream sync point: d0f9d4dcc01ba89f28c5cc3d948829dff6fcf6e5

Identity checks (git and gh):

  • The candidate worktree's HEAD is fcad53a4….
  • gh pr view 99 reports headRefOid=fcad53a4… and baseRefOid=3791db6b….
  • Both the base and 95cb068c10 are ancestors of the head.
  • The range holds 242 commits. 7 belong to the candidate:
    • 39bf0584fd: the upstream merge, with conflict resolutions
    • 7beccb3290: a test fix
    • 3803fc0fb7: a build fix
    • 9887d019b1, 8502646d7b, d992ff20e8, fcad53a4e7: merges of the integration branch back in

Verdicts

  • Code review: APPROVE. Nothing blocks the merge. There is one Low finding and two informational notes.
  • Security review: APPROVE. I found nothing within the surface I reviewed (listed below). The fork's boundaries for MCP auth, the migration namespace and self-update are unchanged.

Findings

1. Low: a run with only fork migrations pending logs the "migrations started" marker but never the "finished" marker.

  • Where:
    • apps/server/src/persistence/Migrations.ts:271, :285, :327, :335-337
    • packages/shared/src/databaseMigrationLog.ts:7-14
    • apps/desktop/src/backend/DesktopBackendManager.ts:975
  • What happens:
    • Upstream's new runMigrations logs DATABASE_MIGRATIONS_STARTED_MESSAGE whenever hasPendingMigrations() is true (line 285).
    • The fork's version also returns true when only fork migrations are pending, or when only the legacy fork-row repair is needed (hasPendingForkMigrations, line 271).
    • In that case no upstream migration runs, so migrations is empty. The run logs "Database schema is current" at debug level instead of DATABASE_MIGRATIONS_FINISHED_MESSAGE (lines 335-337).
    • The fork's own line, "Fork migrations ran successfully" (line 327), does not match "Migrations ran successfully". isRunningDatabaseMigrations uses lastIndexOf, which is case-sensitive.
  • Effect:
    • In a desktop-managed backend, the output tail keeps showing a migration in progress until later output pushes the marker out of the truncated tail.
    • If the server goes quiet and readiness fails for an unrelated reason in that window, onReadinessFailure logs "backend is still migrating its database; waiting" and keeps probing. It does not call reportStartupFailure, so a real startup hang is not shown to the user.
    • This happens only on the first start after a release that adds a fork migration, or needs the legacy repair, while adding no upstream migration.
    • The headless Omarchy service does not read this marker.
  • Cause: a merge interaction between upstream's start and finish markers and the fork's wider hasPendingMigrations. Neither side alone is wrong.
  • Suggested fix: whenever the started line was logged, also emit DATABASE_MIGRATIONS_FINISHED_MESSAGE after fork migrations run. One way is to log it when repairedForkMigrations.length > 0 || executedForkMigrations.length > 0. Add a Migrations.test.ts case where only fork migrations are pending.

2. Informational: clients that negotiate MCP 2026-07-28 do not get the fork's events extension.

  • Where: apps/server/src/mcp/McpHttpServer.ts:1473-1476
  • What the resolution does: it keeps McpEventsProtocol.protocol (based on 2025-06-18) first, so initialize still falls back to it. It also adds upstream's sessionless McpProtocol.v2026_07_28.
  • Why the events are missing: effect 4.1.3's protocol registry prefixes each request with its protocol and routes it to that adapter's own clientRpcs (internal/mcpProtocolRegistry.ts). The 2026-07-28 adapter has no events/* RPCs and no capabilities.events.
  • Who is affected:
    • Clients that call initialize still get the events extension, so nothing regresses for them.
    • A client that uses server/discover with 2026-07-28 gets no events.
    • No test covers events with 2026-07-28. Upstream's new test runs without McpEvents.
  • Action: none needed for this merge. Track it if an events consumer moves to 2026-07-28.

3. Informational: the model-facing description of t3_thread_read changed.

  • Where: apps/server/src/mcp/toolkits/orchestrator/tools.ts:252
  • The resolution combines the fork's sinceRevision sentence with upstream's newest-first paging text.
  • The code matches the text. OrchestratorMcpService.ts:3274-3279 short-circuits only when the read has no afterPosition, beforePosition or itemId, and the merged activity test asserts this.
  • AGENTS.md asks for evals when model-facing text changes. I did not run them.

What I verified

Migration namespace

  • apps/server/src/persistence/Migrations/ is byte-identical to upstream 95cb068c10.
  • Migrations.ts differs from upstream only by the existing fork hooks:
    • repairForkMigrationHistory, before upstream's new ledger read
    • hasPendingForkMigrations, which only reads
    • runForkMigrations, after a full run
  • The new ids, 86 QueuedTurnInputIndex and 87 ProjectionProjectsCustomTitle, are upstream's own.
  • ForkMigrations/001–015, ForkMigrations.ts and ForkMigrations.test.ts are unchanged from the base. No fork name collides with the new upstream names.
  • The repair still runs before upstream's new per-migration loop reads MAX(migration_id), so stray fork rows cannot hide ids 86 and 87.
  • The fork writes to fork_run_exclusions and mcp_approval_answers existed before this merge.

Fork preservation

  • I compared the fork delta before the merge (d0f9d4dc..base) with the fork delta after it (95cb068c..head), file by file and line by line.
  • Every file in the old delta is still present: 283 before, 284 after. The added file is the candidate's one-line test fix in useComposerSelection.browser.test.tsx (isWorking: false).
  • The delta's content changed in 16 files, and I reviewed each one. In every other file, including auto-merged overlaps such as PeerMcpClient.ts, orchestrator/handlers.ts, ThreadManagementService.ts and DesktopUpdates.ts, the fork's added and removed lines are identical before and after.

Notes on specific files:

  • ProjectionStore.ts:
    • The fork's fork_run_exclusions model replaces upstream's timestamp filtering in forkInheritedRunOrdinals, visibleTurnItemsThroughRun, itemCountThroughRun and the timeline rebuild. Upstream filtered with forkCreatedAt, runRolledBackAt and isRolledBackBeforeFork.
    • Upstream's new createInheritedTurnItemVisibility treats rolled-back runs as visible and leaves the cutoff to run ordinals. That fits the exclusion set, which still drops runs that were rolled back before the fork.
    • Both call sites (lines 1518 and 6561) pair it with the exclusion-based ordinals.
    • A grep of non-test apps/server/src finds no leftover forkCreatedAt, rolled_back_at or julianday(run.completed_at) filtering outside the fork's backfill migration 011.
  • ProviderTurnStartService.ts: the fork delta is identical. Dropping recreateMissingWorktree and isManagedWorktreePath follows upstream's move to makeRestoreMissingWorktree.
  • OrchestratorMcpService.readThread: the fork's revision and sinceRevision short-circuit are kept. The short-circuit now also requires beforePosition === undefined and returns hasEarlier: false. It fits upstream's newest-first paging, earlierPosition and budget trimming.
  • Workspace search:
    • The fork's out-of-process fff finder (WorkspaceFinderProcess.ts) is kept. Upstream's scanFilesAsync was added to FinderMethod.
    • refresh and searchVisible go through finder.call(...), and the child process awaits each call.
    • Upstream's in-process createRequire FileFinder loader was correctly dropped. The SEA __workspace-finder subcommand in binMain.ts is intact.
  • The d992ff20e8 and 8502646d7b merges:
    • threadBackgroundWork and frozen are both kept.
    • LastThreadTitleRegeneration and ThreadFreeze are both kept.
    • The mobile imports from both sides are kept.
    • agentContextBudget.json was re-measured at 57906.
  • CI workflows:
    • The fork's runs-on override delta in ci.yml is unchanged.
    • The jobs in fork-integration-ci.yml match ci.yml exactly, including upstream's newly SHA-pinned actions. The only extra job is the informational generated_workflow_current.
  • DesktopUpdates.ts:

Packaging

  • 3803fc0fb7 fixes a real defect. After the merge, the pack object in apps/server/vite.config.ts had two alias keys. The later key (upstream's jsonc-parser alias) silently replaced the fork's SEA alias from undici to seaUndici.cjs.
  • The fix combines both into one object. I checked the whole pack block and found no other duplicate keys.
  • scripts/fork/*, FORK.md and docs/jarvis/ are unchanged.
  • effect moves from 4.1.2 to 4.1.3. The fork's McpEventsProtocol depends on adapter internals, and the server test shards pass on this head.

Security boundaries

Hosted state (status only; not used as review evidence)

  • Every check on this head passes, including Lint, Build and Typecheck, Test Server 1–8, Test Web 1–2, Release Smoke and Agent context budget. Android and iOS were skipped.
  • The PR has no reviews, review threads or inline comments (checked through GraphQL).
  • Codex's summary comment on d992ff2 shows code review status failed, but its hidden state reads has_any_findings: true, has_unresolved_findings: true. No finding text can be retrieved, and the next comment is the usage-limit notice. I could not see what that marker refers to. This review covers the later head independently.

Coverage and limitations

  • Reviewed directly:
    • Every candidate-owned commit.
    • The fork-delta comparison for all 284 files.
    • Remerge diffs:
      • d992ff20e8 and 8502646d7b: all files.
      • 39bf0584fd: the MCP, orchestration, workspace and web vite files. Its other conflicted files (fork-integration-ci.yml, Migrations.ts, AgentContextBudget.test.ts, agentContextBudget.json) I checked by their final state instead.
    • The interactions in the migration, MCP, workspace search, packaging, CI, desktop-update and self-update code, as described above.
  • Not reviewed line by line: the roughly 235 upstream commits themselves (about 34k inserted lines), apart from where they touch fork code and the spot checks named above. For auto-merged overlap files, I confirmed that the fork's lines survived, but I did not read every upstream change for its effect on fork code. Upstream's own review is relied on for those.
  • Not run (the brief was read-only): tests, typecheck, evals, the build, a runtime smoke test and the live-database preflight. Finding 1 comes from reading the code and has not been reproduced.

@chrisohearn

Copy link
Copy Markdown
Owner Author

Reconciliation of the independent review

Finding 1 is genuine and fixed in 01a98a8397339c6c87980f9f1cce5ecb521884e9, retained in final candidate ef466593880b7dbb8f48713a5842eba4ed857b2a against base a59003e7967e6bad7017d5cb9529392a98d04d64.

The existing ForkMigrations.test.ts case now covers fork-only pending migrations and legacy-history-only repair. Before the fix, both produced Running database migrations followed by Fork migrations ran successfully, and the real desktop parser still reported migration in progress. After the fix, successful fork work also emits the shared completion marker, so both cases report completion. The tests also assert unchanged upstream history and the complete, separate fork registry. No migration files, IDs or names changed.

Proof:

  • Both regression cases fail on the pre-fix implementation and pass with the fix.
  • Integrated focused suite: 5 files / 39 tests pass.
  • Full vp run check:changed origin/integration/mcp-events-20261109 --concurrency 1 on the final candidate: exit 0; 41 package typechecks, 345 test files / 8,664 passing tests (1 skipped), 26 node tests and React compiler checks pass.
  • The first full run found TS2488 in the new test logger. A focused retry reproduced it; ef46659388 fixes the capture without assuming the unknown message is iterable. Server typecheck and the complete final gate then pass. This failure was not waived.
  • The generated tool-list budget conflict from refreshing integration was resolved using the measured 58,039-byte value, not by trimming descriptions. Fork migration files/registry equal the refreshed integration base; upstream migration files equal upstream 95cb068c10a6cf0b33eed0a0cab3b298763fe852.

Informational notes remain bounded: legacy initialization keeps fork events, the 2026-07-28 discovery adapter does not advertise events, and actual Jarvis negotiation/outage was not demonstrated. Modern compatibility is retained. No live event-wake acceptance or paid model-behavior eval pass is claimed. A recovered review of d992ff2 is stale; its SEA/JSONC finding is already fixed by 3803fc0, and it is not used as the latest-head review.

A fresh independent exact-head code/security review and latest-head CI remain required. The explicit publication hold covering integration #96/#100 ancestry and the independent service rollout hold both remain in force. Passing this repair does not clear them. No publishing wrapper, merge, release, install, deployment or acknowledge-only activation has occurred.

@chrisohearn

Copy link
Copy Markdown
Owner Author

Exact-head fallback code and security review: #99 (final-head recheck)

Reviewer: Claude Opus 5.5 (claude-opus-5-5), read-only. This replaces the hosted Codex review, which was refused because of its usage quota. It is the one required recheck after the repair of a genuine finding. It is not a second, parallel reviewer.

  • Head: ef466593880b7dbb8f48713a5842eba4ed857b2a (sync/upstream-20261113-0321)
  • Integration base: a59003e7967e6bad7017d5cb9529392a98d04d64 (integration/mcp-events-20261109)
  • Upstream cutoff: 95cb068c10a6cf0b33eed0a0cab3b298763fe852
  • Prior review: comment 6849298398, on head fcad53a4e7 and base 3791db6ba6

Identity, checked once:

  • The candidate worktree's HEAD is ef46659388, and its tree is clean.
  • gh pr view 99 reports headRefOid=ef46659388.
  • a59003e796 and 95cb068c10 are both ancestors of the head.
  • origin/integration/mcp-events-20261109 resolves to a59003e796.
  • faf159f77d has the parents 01a98a8397 and a59003e796.

Commits since the prior review:

Verdicts

This verdict covers the code only. It does not clear either control that is still in force: the maintainer's publication and custody hold on #96/#100 ancestry, and the separate kernel/cgroup hold on automatic updates. This review grants no authority to merge, publish, release, deploy or turn on acknowledge-only.

Disposition of the migration finding

Prior finding 1 (Low): a run with only fork migrations pending logged the "started" marker and never the "finished" marker. It is fixed.

The fix: apps/server/src/persistence/Migrations.ts:326-338.

  • hasForkChanges = repaired.length > 0 || executed.length > 0.
  • The finished line DATABASE_MIGRATIONS_FINISHED_MESSAGE is now logged whenever upstream migrations ran or fork work happened. The run logs "Database schema is current" at debug level only when neither happened.

Every way the started marker can be logged now ends with the finished marker on a full run. The started marker is logged when hasPendingMigrations() returns true, which happens in three cases (Migrations.ts:259-272):

  • The latest recorded id is below the latest upstream id: upstream migrations run, so migrations is not empty.
  • V2 preview rows exist: reconcileV2PreviewMigration results are added to executedMigrations.
  • hasPendingForkMigrations is true (ForkMigrations.ts:202-215): there are legacy rows to repair, giving a non-empty repairedForkMigrations (ForkMigrations.ts:146-165), or the fork head is ahead of the recorded history, giving a non-empty executedForkMigrations.

The case not covered can't happen in production. A partial run (toMigrationInclusive set) with fork work pending still skips the finished marker. But every production caller runs runMigrations() with no argument: Sqlite.ts:56, :64 and :101, and scripts/migrate-dev-db.ts:524. Only tests pass toMigrationInclusive.

The fork's own log line does not count as the finished marker, so the fix is needed. "Fork migrations ran successfully" contains a lowercase "migrations", while isRunningDatabaseMigrations (packages/shared/src/databaseMigrationLog.ts:7-13) uses a case-sensitive lastIndexOf on "Migrations ran successfully".

One cosmetic note: a run with only fork work logs the finished line with migrations: []. The fork details are on the line logged just before it. No change is needed.

The regression test is good. It is in apps/server/src/persistence/ForkMigrations.test.ts:95-130, in the existing case, which is now it.effect.each. That follows the AGENTS.md rule to extend an existing test before adding a new one.

  • "pending migrations":
    • It migrates to the newest upstream id only, then runs the full set, so only fork migrations are pending.
  • "legacy history repair":
    • After a full run, it puts fork migration 1's name back at latestUpstreamId + 1 in effect_sql_migrations.
    • Repair then takes the delete-only path, because the fork table already holds that row with the same name (ForkMigrations.ts:155-163).
    • No fork migrations run, and moved is not empty.
  • Both cases assert the following:
    • hasPendingMigrations() is true beforehand, so the case can't pass because nothing ran.
    • The run returns no upstream migrations ([]).
    • The captured output contains DATABASE_MIGRATIONS_STARTED_MESSAGE, so it can't pass vacuously.
    • The desktop's real isRunningDatabaseMigrations parser returns false.
    • Afterwards nothing is pending.
    • The upstream history equals migrationManifest, and the fork history equals forkMigrationManifest.
  • The logger replaces the default one (mergeWithExisting: false), so the test sees only its own output.
  • The ef46659388 change (messages.push(message) instead of a spread of unknown) is correct. String(array) joins the parts with commas, which the substring checks handle.

Migration namespace (FORK.md)

  • git diff 95cb068c10 HEAD -- apps/server/src/persistence/Migrations/ is empty, so upstream's migration files match the upstream cutoff.
  • git diff a59003e796 HEAD -- apps/server/src/persistence/ForkMigrations/ apps/server/src/persistence/ForkMigrations.ts is empty, so the fork's migration files and registry match the integration base.
  • Migrations.ts differs from upstream only by the fork's hooks: the three imports, the extra clause in hasPendingMigrations, repairForkMigrationHistory before the ledger is read, and runForkMigrations plus the marker fix after the upstream loop.
  • No migration file, id or name was added, renamed or moved, and the repair still runs before upstream's MAX(migration_id) read.

Merging the integration branch (faf159f77d)

  • Conflicts: git show --remerge-diff shows exactly one conflict, in apps/server/src/provider/agentContextBudget.json, resolved to 58039.

    • Integration's own change was 57928 → 58061, an increase of 133.
    • The candidate's change was 57906 → 58039, the same 133.
    • So the new value is consistent with the integration change, not a guess. The hosted "Agent context budget" check passes on this head.
  • The feat(server): acknowledge-only threads answer messages without a provider #96/fix(server): one in-flight temporary-folder probe per path #100 change set arrived intact. I compared the -U0 added and removed lines of git diff 3791db6ba6 a59003e796 with those of git diff 01a98a8397 faf159f77d. They are identical across all 21 files, apart from the budget number above.

  • Files that upstream also changed between d0f9d4dc and 95cb068c. I checked how the two sides interact in each:

    • ProviderTurnStartService.ts:

      • The acknowledge-only short-circuit (:370-389) still runs after the check for missing projection state and before settleRunBeforeStart.
      • It also still runs before upstream's new restoreMissingWorktree (:594), which matches its position before the old recreateMissingWorktree on the base.
      • I compared the file with git diff a59003e796 HEAD. The upstream changes are:
        • the restore refactor;
        • a makeRestoreMissingWorktree that is resolved when the layer is built;
        • the removal of the inline recreate helpers;
        • the goalDriver capability check, which is below the branch.
      • None of these adds side effects above the branch for each run.
      • An acknowledge-only run therefore never touches the worktree or the provider, the same as on the base.
    • ThreadManagementService.ts:

      • Upstream added the "unmatched" reason for background waits.
      • The fork added acknowledgeOnly and setAcknowledgeOnly, the latter behind a one-permit semaphore.
      • The two changes don't overlap, and both are kept.
    • thread/tools.ts:

      • Upstream added preview to the question options.
      • The fork added acknowledgeOnly to t3_thread_configure's parameters and result, the result field being optional for linked environments on older builds.
      • Both are kept.
    • contracts/settings.ts:

      • Upstream added keepAwakeWhileAgentsWork.
      • The fork added acknowledgeOnlyThreadIds, which defaults to [] and drops invalid entries one at a time, and its patch key.
      • Both are kept.
    • core.test.ts: it adds 44 lines inside the existing case "a read-only client reads threads and is refused every write…". The test checks that:

      • a read of t3_thread_configure reports acknowledgeOnly: false;
      • read-only and auto callers get capability_denied when turning it on, and auto gets it when turning it off;
      • full-access can turn it on and off;
      • a later read reports true;
      • dispatched is still length 1, so no orchestration command ran.

      Upstream's own changes to this file are kept.

    • AgentAwarenessRelay.test.ts and PeerForwarding.test.ts: only their fake ThreadManagementService mocks gained acknowledgeOnly and setAcknowledgeOnly.

    • settings.test.ts: it adds a check that acknowledgeOnlyThreadIds: ["thread:inbox", 7, ""] decodes to ["thread:inbox"].

  • Files upstream did not change between those points:

    • McpToolAccess.ts
    • thread/handlers.ts
    • ProviderRuntimeRecoveryService.ts
    • ProviderReplayHarness.ts
    • PeerForwarding.test.ts
    • temporaryFolder.ts, along with every importer of it: ThreadLaunchMcpService.ts, project/handlers.ts and provider/Errors.ts

    The fork's changes there apply to the same upstream code they were written against.

  • The rest of the fork's work is unaffected. The rest of the fork delta and every upstream file are the same as at the previously reviewed head fcad53a4e7, except for the three commits above. The prior review's checks of how the two sides fit together still hold: ProjectionStore's run exclusions, the readThread sinceRevision short-circuit, the out-of-process workspace finder, the SEA/JSONC alias fix 3803fc0fb7, the CI workflows, DesktopUpdates and self-update.

Security boundaries

Who can turn acknowledge-only on or off

  • Over MCP:
    • t3_thread_configure sends any call that sets acknowledgeOnly down the write branch (handlers.ts:257-261).
    • That write branch is writesThread, which goes through McpToolAccess.writesThreads and authorizes the target thread.
    • configureAcknowledgeOnly then calls McpToolAccess.requireFullAccess (McpToolAccess.ts:343-348): orchestratingCaller, then assertNotLinked, then assertFullAccess(caller, fullAccessRequired).
    • Those are the same checks writesEnvironment makes (:364-371), without its opt-in linkedCallers: "allowed".
    • It also refuses acknowledgeOnly combined with modelSelection or runtimeMode.
    • core.test.ts tests this boundary, as described above.
    • Upstream did not change McpToolAccess.ts in this range.
  • Over the WebSocket:
    • server.updateSettings needs AuthSettingsWriteScope (auth/RpcAuthorization.ts:95 and :275).
    • requiredScopesForServerSettingsPatch (settings.ts:1976-2003) counts acknowledgeOnlyThreadIds as a non-provider key. That sets changesSettings, which returns [AuthSettingsWriteScope].
    • Upstream's only change to clientRpcPermissions.ts adds a guard for serverCheckSecretSource, which tightens access.
    • git diff d0f9d4dc 95cb068c -S updateSettings finds only SettingsPanels.tsx, a web UI that calls the same guarded RPC, and serverUsage.test.ts. Upstream added no new way to write settings.
  • The MCP environment-preferences tool can't set it. It accepts only a list of named fields (environment/tools.ts:62-66), and acknowledgeOnlyThreadIds is not one of them.

What acknowledge-only puts in front of the agent

Temporary-folder probe (#100)

  • The cache is bounded (256 paths, 30-second TTL).
  • A path whose probe hasn't answered returns undefined at once, so the caller goes by the name, the same fallback as before.
  • None of the code involved touches credentials or auth.

MCP, migrations and self-update: unchanged since the prior review. layerMcpAuthMiddleware still wraps every protocol, and selfUpdate.ts, pinnedRuntime.ts, scripts/fork/* and FORK.md are untouched by the three new commits.

Informational, not a finding

The MCP 2026-07-28 events seam (prior note 2) still stands.

  • Clients that call initialize with the legacy protocol keep the fork's events adapter.
  • Clients that negotiate 2026-07-28 through server/discover get an adapter that advertises no events.
  • This is a coverage and compatibility note, not a regression. Modern support should stay.
  • No actual Jarvis negotiation or outage has been verified, and this review doesn't claim live acceptance of event wake-ups.

Evidence inspected

All logs are in /home/cohearn/.cache/t3-fork-release/logs/pr99-migration-markers-QhiYLl/. I read them and did not rerun anything. The logs don't record the head SHA, so they are tied to these commits only by their timestamps and content.

  • red.log: both new cases fail before the fix with AssertionError: Running database migrations\nFork migrations ran successfully: expected true to be false at the isRunningDatabaseMigrations assertion. That is the intended reason for the failure. The other 7 tests in the file were skipped.
  • green.log: 2 files and 20 tests pass.
  • integrated-focused.log: 5 files and 39 tests pass. The log doesn't name the files.
  • Only the full run covers the final test. Both green.log and integrated-focused.log predate ef46659388, which changed the test's logger capture. The final test body was exercised only by check-changed-final.log. That log reports a summary only, so it doesn't name ForkMigrations.test.ts, but this file is a changed test, which check:changed runs.
  • check-changed-first.log:
    • The typecheck failed, yet 345 test files (8,664 tests, 1 skipped) passed.
    • typecheck-retry.log reproduces ForkMigrations.test.ts(99,68): error TS2488.
    • typecheck-fixed.log has no errors, only the existing schemaClassMissingBrand suggestions.
  • check-changed-final.log: these all pass:
    • unused patches, format, lint, knip 1/2 and 2/2, and graphql
    • typecheck (41 packages)
    • test: 345 files, 8,664 passing, 1 skipped
    • node --test: 26 tests
    • React compiler: 222 files
  • reconciliation.md: it agrees with what I found in the source.

Hosted CI on ef46659388 (status only; not used as review evidence), from one fresh snapshot taken just before this review was returned:

  • 18 checks passed:
    • Changes and Lint
    • Build and Typecheck
    • Test Server 1–8, Test, and Test Web 1–2
    • Rust and Mobile Native Static Analysis
    • Release Smoke and Agent context budget
  • 2 were skipped: Android and iOS.
  • 1 was still running: Check.
  • The PR had 0 formal reviews.

Earlier reviews: the recovered review of d992ff2 is stale, and this review does not rely on it. Its SEA alias issue was fixed by 3803fc0fb7, and its migration issue is the finding fixed here.

Limitations and what I did not run

PR #99, head ef46659, checks and reviews 18 passed, 2 skipped, 1 in progress (Check); 0 formal reviews; this review is APPROVE (code) and APPROVE (security) and not yet posted

@chrisohearn

Copy link
Copy Markdown
Owner Author

Exact-head fallback code and security review: #99 (integration re-merge, final head)

Reviewer: Claude Opus 5.5 (claude-opus-5-5), read-only, the single allowed fallback reviewer. Hosted Codex review was refused on usage quota. This review covers only what changed since my previous review of ef46659388. Every part of that review that this delta does not touch still holds and is not repeated here.

Identity, checked read-only (no fetch, no ref changes):

  • The candidate worktree's HEAD is 0537f39fff, and its tree is clean.
  • 0537f39fff has one parent, 147b9691ac.
  • 147b9691ac has the parents ef46659388 and 782fe7fa60.
  • 782fe7fa60, ef46659388, 01a98a8397 and 95cb068c10 are all ancestors of the head.

Commits since the previous review:

  • 147b9691ac: merges the fresh integration branch.
  • 0537f39fff: a one-line test repair, for a defect this review found in 147b9691ac.

Verdicts

  • Code review: APPROVE, for 0537f39fff exactly.
    • I found one Medium finding in 147b9691ac, and it is fixed.
    • I found no other new findings.
  • Security review: APPROVE, for 0537f39fff exactly. I found no findings.
    • The merged integration features add no new way to write.
    • They loosen no auth check.
    • The repair touches only a test.

Gate condition: this approval assumes the final-tree check:changed (integration-782-check-changed-fixed.log) passes. It is the only full gate run on a committed tree.

  • When I read the log at 11:48 MST, these steps had passed: unused patches, format, lint, knip 1/2 and 2/2, both graphql checks, and typecheck (41 packages), in 17.1s.
  • The test step was still running.
  • If that run fails, this approval doesn't cover what it finds.

This verdict covers the code only. It grants no authority to push, merge, publish, release, deploy or turn on acknowledge-only. Those controls stay with the coordinator.

Finding 1 (Medium, fixed in 0537f39fff): the merge dropped a required argument in Manager.test.ts

  • The defect:
  • The effect on 147b9691ac:
    • integration-782-check-changed.log reported FAIL test: 3 rows failed with input.deriveProviderInstanceConfigMap is not a function at Manager.ts:1517.
    • The same run reported FAIL typecheck.
    • The root's direct server typecheck (integration-782-typecheck-repro.log) reports exactly one error: src/terminal/Manager.test.ts(2303,95): error TS2741: Property 'deriveProviderInstanceConfigMap' is missing. That rules out a second error that would only show up in type checking.
  • The repair is correct and minimal:
    • git diff 147b9691ac 0537f39fff is one inserted line at Manager.test.ts:2304: deriveProviderInstanceConfigMap,.
    • The import at :1 was already there.
    • The call now matches upstream's own version (95cb068c10:…:2298-2299) and the previous head's.
    • Nothing else changed, and no production code changed.
  • Proof, from logs I read:
    • integration-782-terminal-red.log: 3 failed and 86 skipped, the expected red run.
    • integration-782-terminal-green.log: 89 of 89 Manager.test.ts tests passed.
    • integration-782-typecheck-green.log: the server typecheck has 0 errors, and only TS377141 suggestions.

What the merge brings in, and how I checked it (unchanged from 147b9691ac)

#94/#98/#97/#101 arrived intact.

  • git diff --name-only a59003e796 782fe7fa60 and git diff --name-only ef46659388 147b9691ac list the same 37 files.
  • For every file, the -U0 added and removed lines are identical between the two diffs, except agentContextBudget.json.
  • The two conflicts are the only places where the merge chose content (git show --remerge-diff 147b9691ac):
    • packages/contracts/src/orchestrationV2.ts is resolved correctly.
      • Upstream's OrchestrationV2LastTitleRegeneration and the fork's OrchestrationV2ScheduledModelSelection are both declared before OrchestrationV2AppThread, and both are used.
      • lastTitleRegeneration (upstream) and scheduledModelSelection (fork) are both optional fields on the thread.
      • The fork's snoozeDeferred, explicitModelSelection, applyAt and respectSnooze sit beside upstream's checksPendingBefore, checks-reopen, nativeItemId, userOnly and outcome without overlapping.
    • apps/server/src/provider/agentContextBudget.json is resolved correctly to 58309.
      • Integration's own change was 58061 → 58331, an increase of 270.
      • The candidate's change was 58039 → 58309, the same 270.
      • integration-782-context.log holds the remeasurement: the test reported a measured 58309 against the 58331 taken from integration's side.

Files that both upstream (d0f9d4dc..95cb068c) and integration changed: 17 files, auto-merged.

  • Orchestrator.ts:
    • Upstream added:
      • checks-reopen and syncedPullRequestWatch;
      • lastTitleRegeneration;
      • the Antigravity goal guard;
      • the archive guard that refuses while delegated tasks are running;
      • republishing a retained user item when a queued run fails.
    • The fork added the scheduled model selection and the snooze hold.
    • The two sides touch different branches and different case arms.
    • threadAtRunStart spreads the whole thread, so upstream's lastTitleRegeneration is kept in the payloads.
    • How the release paths treat snooze-held turns:
      • Resume and unfreeze, and an unattended completion, keep a snoozeDeferred run held while isSnoozed holds the thread.
      • A failure that owns the queue never releases a snoozeDeferred run.
      • Wherever a run is released (a fresh send, resume, or unattended completion), withoutSnoozeDeferral clears the flag.
      • Upstream added no release path in this range.
  • ThreadSettlementService.ts:
    • Upstream's check that a pull-request watch blocks settlement, and its confirm-terminal path, both stay.
    • isSnoozed moved unchanged into SnoozeState.ts and is re-exported.
    • toMillis is still used locally.
  • ThreadManagementService.ts: upstream's "unmatched" wait reason and the fork's respectSnooze passthrough don't overlap.
  • OrchestratorMcpService.ts:
    • readThread and threadResult resolve the reference, then rebind input = { ...request, threadId }.
    • So every later read uses the resolved id, including upstream's newestFirst/beforePosition paging, the read budget and earlierPosition.
    • The fork's sinceRevision short-circuit is kept, and includes upstream's new required hasEarlier: false.
  • thread/tools.ts: upstream's preview option is kept, alongside the fork's applyAt and scheduled result. The scheduled result is ordered before the plain dispatch result, so its fields aren't dropped.
  • CodexDriver.ts, CodexAdapterV2.ts and CodexManagedRuntime.ts:
    • Upstream's models hook, Cyber Access and resolveProviderProbeTimeoutMs(…, processEnv) stay.
    • processEnv is now withCodexSqliteHome(homeLayout, instanceEnv), a superset of the old value.
    • Upstream didn't change CodexHomeLayout.ts in this range.
  • terminal/Manager.ts: upstream's config-map injection and output-protocol changes stay. The fork's CODEX_SQLITE_HOME logic uses HostProcess.Environment, which is already imported. Its one test problem is Finding 1, now fixed.
  • Other upstream type changes:
    • ProviderRegistry.applySecretAndRefresh is satisfied by the partial Layer.mock fake in the new core.test.ts case.
    • Upstream didn't change the other modules the integration code imports in this range: serverActivation, McpToolAccess.testkit, shared/threadLinks, shared/model, DueHint, Scheduler, QueuedRunOrder and ThreadFreeze.
  • Upstream code the fork's changes might not cover. I looked for upstream code in this range that bypasses the new fork behavior:
    • CODEX_SQLITE_HOME: upstream changed no CODEX_HOME assignment. The remaining sites that set CODEX_HOME without withCodexSqliteHome are the same at base 782fe7fa60, so they are integration's own design and not something this merge introduced:

      • CodexProvider.ts:621;
      • CodexTextGeneration.ts:287;
      • CodexAdapterV2.ts:1659, which spreads the environment that withCodexSqliteHome has already filled in.

      Upstream's codexUsage.ts and CodexManagedProvider.ts changes touch only how usage is counted, not environments.

    • respectSnooze: upstream added no new agent or MCP send path in server code (no creationSource: "mcp", message.dispatch or send(). The three senders that the decision record names cover them all.

    • Scheduled model selection: upstream added no new read of thread.modelSelection, and no new snooze read, in non-test server code. So no new turn-start path bypasses nextTurnModelSelection.

  • Docs: the merged MCP and outside-agent docs are additive. docs/jarvis/decisions/0017-… collides with no other file. The duplicate 0012-* and 0013-* numbers come from older commits already on the base.

Migration markers and namespace still hold

Security

  • applyAt: "next_turn":
    • Setting applyAt forces t3_thread_configure down the write branch. The read predicate requires applyAt === undefined, so the call goes through the same writesThread authorization as modelSelection.
    • It is refused when it would switch provider instances.
    • It is refused together with acknowledgeOnly, so acknowledge-only stays behind requireFullAccess alone.
    • It checks the model and its options against the advertised catalog before it schedules anything.
  • respectSnooze:
    • It defaults to true for MCP post, send and attachment sends.
    • It can only hold a turn the caller was already allowed to send. Passing false gives the same wake behavior as before.
    • Usage-limit snoozes are excluded.
  • resolveReadableThread:
    • It runs only on read tools (readThread, threadResult), after loadCaller.
    • It resolves only ids in this environment, and it refuses links that name another environment.
    • Its "Closest:" suggestions come from listActiveShellIndex. That exposes no more than t3_thread_list with allProjects: true already does.
    • Write tools keep their exact-id checks.
  • The CodexHomeLayout sidecar settling (fix(server): keep Codex SQLite sidecars out of shadow homes #98/fix(server): keep foreign shadow database links with their SQLite sidecars #101):
    • It works only on the shadow home's own directory entries, and only on names matching *.sqlite-{wal,shm,journal}.
    • It removes symlinks, never their targets.
    • It moves aside, never deletes, any sidecar that holds WAL or journal data.
    • It leaves files alone when the database isn't the shared one.
    • It doesn't touch credentials. ensureShadowAuthIsPrivate is unchanged.
  • CODEX_SQLITE_HOME:
    • It is set only for authOverlay layouts, and only when it isn't already set.
    • It points at the configured shared home, and it adds no credentials.
  • Between the previous head and this one, nothing changed in McpToolAccess.ts, RpcAuthorization.ts, settings write scopes or the acknowledge-only boundary.

Evidence and limitations

  • Logs I read:
    • integration-782-focused.log: 9 files and 348 tests passed.
    • integration-782-context.log: the expected red run that remeasured the budget.
    • integration-782-check-changed.log, on 147b9691ac: typecheck and test (Finding 1) failed, and everything else passed.
    • integration-782-terminal-red.log, -terminal-green.log, -typecheck-repro.log and -typecheck-green.log: the repair proof above.
    • integration-782-check-changed-fixed.log: still running when I read it (see the gate condition).
  • Timing:
    • The focused (11:31) and context (11:30) runs, and the first check:changed (its test step started at about 11:33 local, because the log's clock runs one hour behind), predate the merge commit 147b9691ac (11:38). They are evidence about the working tree, not about a committed tree.
    • The repair's red and green runs (11:42–11:44:11) predate commit 0537f39fff (11:44:42) by about 30 seconds. The worktree is clean, and the delta is the one line shown, so this gap doesn't matter.
    • The fixed check:changed started after the commit.
  • Instructions I read:
    • Global AGENTS.md, the worktree's AGENTS.md and FORK.md, and docs/jarvis/ (decision records, COORDINATORS.md, upgrades.md).
    • This merge follows FORK.md:
      • No file under Migrations/ was added.
      • No fork migration was renamed or renumbered.
      • The budget conflict was remeasured.
      • The delta doesn't touch .github, so fork-integration-ci.yml doesn't need regenerating.
    • Nothing in those files changes a verdict.
  • What I did not do: I did not build, test, fetch, push, post, or operate any service or database. All test and typecheck results above come from the root's logs.
  • Pushing this head: once pushed, PR Merge upstream main (95cb068c10) #99's headRefOid must equal 0537f39fff for this review to apply.
  • Informational, unchanged: the MCP 2026-07-28 events adapter seam stays informational and non-blocking, as before.

@chrisohearn

Copy link
Copy Markdown
Owner Author

Daily upstream sync: merges pingdotgg/t3code main (95cb068, 235 commits) into integration/mcp-events-20261109.

Format, lint, typecheck, server tests, the container release build with its smoke test, and the service preflight against the live database passed. Prepared release candidate v0.1.10-preview.20261113.4; publication follows a successful merge.

🤖 Generated with Claude Code

@chrisohearn
chrisohearn merged commit dc3ee55 into integration/mcp-events-20261109 Nov 13, 2026
21 checks passed
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.