Skip to content

Merge upstream main (95cb068c10) - #99

Open
chrisohearn wants to merge 245 commits into
integration/mcp-events-20261109from
sync/upstream-20261113-0321
Open

chrisohearn wants to merge 245 commits into
integration/mcp-events-20261109from
sync/upstream-20261113-0321

Conversation

@chrisohearn

@chrisohearn chrisohearn commented Nov 13, 2026 •

Copy link
Copy Markdown
Owner

Publication hold

Publication remains held pending maintainer disposition of integration changes from #96 and #100. The separate service rollout safety hold also remains in effect. Passing source checks or reviews does not clear either hold. Do not run the publishing sync wrapper, publish a release, deploy/install this candidate, or activate acknowledge-only behavior until the applicable owner gates are cleared.

Scope

Recover the existing upstream-sync candidate through pingdotgg/t3code main 95cb068c10a6cf0b33eed0a0cab3b298763fe852 (235 upstream commits). Finish this candidate before adding newer upstream commits. Refreshed integration base: a59003e7967e6bad7017d5cb9529392a98d04d64; tested candidate head: ef466593880b7dbb8f48713a5842eba4ed857b2a.

Preserves fork MCP/events, orchestration, thread freeze and agent registry behavior while retaining upstream runtime changes. Updates the composer fixture for upstream's working-state contract. Fork migration files/registry match integration exactly, including migration 15; upstream migration files match the selected upstream cutoff. Fork migration history stays in t3_fork_migrations. MCP tool-list budget is measured at 58,039 bytes after the integration refresh.

CI repair

The failed Test job on d992ff2 reproduced in the existing SEA packaging regression: upstream's later JSONC alias object overwrote the fork's conditional Undici alias. Commit 3803fc0fb7 combines both aliases in one object, retaining the SEA shim and JSONC ESM mapping. The refreshed integration base is merged without adding newer upstream commits.

Review repair

The independent Opus review found that fork-only migrations or legacy fork-history repair logged the migration-start marker without the shared completion marker. The desktop then continued to classify the backend as migrating. Commit 01a98a8397 counts completed fork work when emitting the shared completion marker. The existing fork-migration test now covers both fork-only execution and repair-only execution against the real desktop output parser, while asserting both migration histories remain correct.

Verification

  • Existing SEA regression fails before the alias fix; all 13 packaging tests pass afterward.
  • Integrated focused suite: 8 files / 215 tests pass.
  • Migration regression: both new cases fail before the product fix because the desktop parser remains in the migrating state; both pass after the fix. Integrated focused suite: 5 files / 39 tests pass, including migration namespace, measured context budget, acknowledge-only and temporary-folder behavior.
  • Final-tree vp run check:changed origin/integration/mcp-events-20261109 --concurrency 1 on ef466593880b7dbb8f48713a5842eba4ed857b2a: exit 0; format, lint, knip, generated GraphQL, all 41 package typechecks, 345 test files / 8,664 tests pass (1 skipped), 26 node tests and React compiler checks pass.
  • The first integrated gate exposed a logger-capture test typing error (TS2488, spreading an unknown message). A focused retry reproduced it; commit ef46659388 fixes it, server typecheck passes, and the complete final-tree gate passes. No failing check is waived.
  • Two earlier full local runs hit unhandled browser RPC disconnects without assertion failures. The affected browser files pass independently (18 tests), and the final full gate passes with external socket/CDP/network diagnostics. No product workaround, timeout increase, retry setting, or browser root-cause claim is made.
  • Fresh pre-push integration/remote-head checks and migration namespace comparisons pass. Worktree is clean.

Outstanding gates

Final head ef466593880b7dbb8f48713a5842eba4ed857b2a, base a59003e7967e6bad7017d5cb9529392a98d04d64: all 21 hosted contexts are complete, with 19 successful and 2 workflow-selected mobile checks skipped; none failed or pending. CI and Agent context budget pass. The exact-final-head independent Opus review of record approves both code and security, confirms the migration finding is fixed, and reports no new genuine findings. Review threads: zero at final reconciliation. The review's own earlier CI snapshot still had the final aggregate Check running; the parent subsequently verified that it passes. No hosted quota re-request, stale-head waiver or failing-check waiver is used. The earlier fcad review remains historical evidence, not the final-head verdict.

Source CI/review gates are satisfied, but the publication hold above is unchanged. The publishing wrapper's release build, smoke test and read-only live-database preflight have not run for this candidate and must precede any later authorized merge/publication. Review or CI success is not custody clearance or deployment authorization.

The MCP 2026-07-28 events-extension seam remains an informational compatibility limitation, not a demonstrated Jarvis outage: legacy initialization retains the fork events protocol, while the new discovery adapter does not advertise that extension. Modern protocol support is not reverted. The earlier d992ff2 review is stale and is not used as a final-head verdict. Paid model-behavior evals and live Jarvis event-wake acceptance have not been run; neither is claimed as passing.

The mandatory t3-sync-upstream wrapper must build, smoke-test and run read-only live-database launcher-protocol preflights before merge/publication. Nothing is merged, published or deployed from this candidate. The automatic deployment safety hold remains intact.

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>
juliusmarminge and others added 20 commits November 12, 2026 17:56
Co-authored-by: Julius Marminge <julius0216@outlook.com>
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

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.