Repository navigation
test: move fork-only assertions into fork-owned test suites - #729
Open
lastobelus wants to merge 6 commits into
Open
lastobelus wants to merge 6 commits into
lastobelus wants to merge 6 commits into
Conversation
Part of #720. Restore the preview test to the pinned nightly and preserve the LastCode ledger, evidence index, preview import and released-schema upgrade checks in DatabaseMigrations.test.ts. Carry-Group: incubator Carry-Fix: #720 Carry-Observation: Migration coverage follows the existing Incubator-owned DatabaseMigrations suite; no database behavior changes.
Part of #720. Keep fork-only publication and background-wake cases in sibling LastCode suites while retaining the upstream regressions. Upstream coverage compared with https://redirect.github.com/pingdotgg/t3code/pull/24836 and https://redirect.github.com/pingdotgg/t3code/pull/23542. Carry-Group: incubator Carry-Fix: #720 Carry-Observation: Preserve Incubator-specific conditional persistence, application event history, Pause and ownership assertions outside upstream test files.
Part of #720. Move fork setup and terminal lifecycle tests into sibling LastCode suites; remove the startup-exit duplicate covered by the upstream parameterized regression. Upstream coverage compared with https://redirect.github.com/pingdotgg/t3code/pull/24775. Carry-Group: incubator Carry-Fix: #720 Carry-Observation: Retained fork setup-readiness, admission and terminal-ownership tests follow the Incubator capability; upstream tests retain only required fork fixture adaptations.
Part of #720. Restore upstream process-tree PID fixture coverage and preserve the remaining fork audio cache-policy assertion in a fork-owned suite. Product code is unchanged. Compared upstream https://redirect.github.com/pingdotgg/t3code/pull/17647 at 202b954 and https://redirect.github.com/pingdotgg/t3code/pull/12326 at d547e3b. Carry-Group: upstream-bugfixes Carry-Fix: #720
Part of #720. The pinned native-image prompt test plus shared-helper escaped untrusted JSON test cover the removed combined case. Preserve the unrelated fork host-settings regression. Compared upstream https://redirect.github.com/pingdotgg/t3code/pull/21356 at 550f119. Carry-Group: incubator Carry-Fix: #720
Part of #720. Retain the full Pi captured-window/native-image prompt case in a fork-owned suite, and place alias-read coverage beside the actual client-files helper. Compared upstream https://redirect.github.com/pingdotgg/t3code/pull/21356 at 550f119 and https://redirect.github.com/pingdotgg/t3code/pull/26184 at 8251578. Carry-Group: incubator Carry-Fix: #720
lastobelus
force-pushed
the
lastcode/sync-churn-test-hygiene
branch
from
November 15, 2026 10:01
22e2d95 to
8e12f78
Compare
lastobelus
marked this pull request as ready for review
November 15, 2026 10:08
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #720.
Fork-only assertions sitting inside upstream-owned test files conflict with the nightly sync whenever upstream edits those files. This PR moves that coverage into fork-owned suites, so the upstream files carry less fork text for nightly to collide with. Coverage is kept: the setup, terminal, persistence and delivery cases move unchanged except one setup test, deleted because a pinned upstream test covers all of its assertions. Migration assertions extend the existing fork database suite. Production code is unchanged.
🧭 Summary
055_OrchestrationV2.test.ts)DatabaseMigrations.test.tsProjectSetupScriptRunner.lastcode.test.tsManager.lastcode.test.tsFoundationPersistence.lastcode.test.tsDelegatedCompletionDelivery.lastcode.test.tsPairingGrantStore.lastcode.test.tsAntigravityClientFiles.lastcode.test.tshttp.lastcode.test.tspackages/provider-pi/src/server/adapter.lastcode.test.ts055_OrchestrationV2.test.tsis separate from that count.docs/lastcode/fork-conventions.md(including docs(lastcode): put new fork behavior and its tests in fork-owned files #721) defines ownership but no suffix. New sibling suites use<Module>.lastcode.test.ts; the alias-read suite sits beside theAntigravityClientFileshelper it exercises. All eight are absent fromorigin/main, as is the existingDatabaseMigrations.test.ts.db2cc2026427c5eb647a08538c12402341db0fab, the merge base withorigin/main. Attribution was traced from original base20bcf5d6ca. The branch was then cleanly rebased ontoc1eb9dd53fc1dd8d4c3fc61834dd65975c17a707, and nightly is unchanged.🗑️ The one deleted test
does not type into a shell that exits during startup(from #499). The pinned upstream test that covers it isdoes not type into a shell that exits during startup (before shell lookup: %s),ProjectSetupScriptRunner.test.ts:213.falserow emits the same exit event after startup, asserts abandoned completion (exitCode: null), advances the same virtual minute, and asserts no write and zero listeners.truerow also covers shell lookup finding no running terminal.✅ Validation
pnpm exec vp run check:changed origin/lastcode/mainexited 0 on head8e12f78e4bcd6f35fa92ccc66c2c2dc3e869d276/ basec1eb9dd53fc1dd8d4c3fc61834dd65975c17a707. All 809 tests in 20 files passed. Format, lint, knip (both passes), GraphQL and typecheck (9 packages) passed.📐 Upstream-owned files vs pinned nightly
persistence/reconcileV2PreviewMigration.test.tsproject/ProjectSetupScriptRunner.test.tsterminal/Manager.test.tsorchestration-v2/FoundationPersistence.test.tsorchestration-v2/DelegatedCompletionDelivery.test.tsprovider/acp/AcpSessionRuntime.processTree.test.tsauth/PairingGrantStore.test.tsorchestration-v2/Adapters/AntigravityAdapterV2.test.tshttp.test.tspackages/provider-pi/src/server/adapter.test.tspersistence/Migrations/055_OrchestrationV2.test.tsdoes not exist at nightly. Despite its upstream-shaped name it was fork-owned, and it is now removed. This PR does not claim the other six files equal nightly. The remaining differences are inventoried below, including the fork-adapted HTTP cache-policy case moved out of the upstream suite.Migrations (#279): moved assertions
reconcileV2PreviewMigration.test.ts:upgrades a published preview without replaying V2 or losing import progress(production migration runner plus appended LastCode manifest)DatabaseMigrations.test.ts:LastCode V2 preview ledger (existing LastCode: false)/normalizes the published upstream preview before checking both ledgers. It asserts the combined return list, both persisted ledgers, the creator evidence index, import progress, and that a repeat startup does nothing. Thetruerow keeps the existing partially migrated LastCode scenario.12_ThreadCreatorEvidenceIndexand physical indexorchestration_events_v2_thread_created_target_idxassertCurrent, which checks the complete LastCode ledger and the physical index.Migrations/055_OrchestrationV2.test.ts:upgrades released schema 53 with the creator evidence index53row inDatabaseMigrations.test.ts/upgrades an upstream-only database.assertCurrentchecks the complete ledgers and physical index, and a repeat production startup returns[].No upstream migration list is copied into the fork suite.
Setup runner (#499): 18 moved declarations
All moved bodies and the admission harness are byte-for-byte copies from original base
20bcf5d6ca.T3CODE_WORKTREE_PATH; no pinned equivalent.closeIdle, listener cleanup.restart,activity,editor): stale-probe invalidation and recovery on a fresh readiness marker.Terminal manager (#499): 2 moved declarations
Manager.test.tshas an equivalent.FakePtyProcess,FakePtyAdapter,openInputand both test bodies are byte-for-byte copies. A new localcreateManagerkeeps the original temp directory, history limit, PTY allocation, 1 ms kill grace, 10 ms exit wait, workspace test layer, event subscription/finalizer and per-case options. Its option type includes only the options these two cases use. Theit.layerdependencies,excludeTestServices: trueand per-case platform/TestClock overrides are unchanged. The unrelated global SynchronizedRef mock is not imported.Foundation persistence (#485): 9 moved cases
The 434-line block exactly matches #485's merge tree (
1dace6e4d8). SHA-256 of the moved block:0ae4d4ac347039f983b9102d1cf04b1b5b2b3139fb3794f9717539c4b57de2b4.$caller preserves event consistency and allows later writes after $scenario:writecancel transactionwritefail transactionwritecancel publicationwriteIfRunCurrentcancel transactionwriteIfRunCurrentfail transactionwriteIfRunCurrentcancel publicationrolls back a writer cancelled while waiting for committed publicationpreserves project command consistency when cancelled %s(before commit)preserves project command consistency when cancelled %s(after commit)Pinned upstream's
preserves event consistency and allows later writes after %sis unchanged. It checks thread projection/outbox persistence and live order throughwriteWithEffects, which does not cover the fork assertions, so nothing was deleted.The
Metricand application-event-store imports and the exposed application-event-store fixture moved with the tests. The upstream file's event-store fixture is restored to nightly'sEventStore.layer.pipe(Layer.provideMerge(layerDatabase)), becauseEventStore.layeralready providesOrchestrationEventStore.layer. The new file is 536 lines and omits the upstream worker-thread recording mock, compaction/replay helpers and unrelated tests.Delegated delivery (#484): 4 moved cases, and one #485 case left in place
The 238-line block exactly matches #484's merge tree (
5d3e8b1527). SHA-256:6532eefab249c22d0dcb30098b65958fba8ad1af21b4ee055b7ac2132544365f.delegated background wakes/handles a background command's wake after the original result while %s:activepausedreleasedownershipReleased/ disposed delivery: no delegated task, parent tasks/runs untouched.independentPinned upstream's
delivers the run a background command's wake starts after the original result (%s)(live,checkpoint-retry,checkpoint-offer-retry) is unchanged and was not replaced by the fork cases.layerFollowupsand its resumed-pause fixture moved. The now-unusedEnvironmentPauseStoreimport was removed;makePausedEnvironmentstays because the watch tests use it. The new file is 730 lines and registers only these four cases.Not moved:
recovers an unpublished child failure after its completion delivery was disposed(#485 ↔ pingdotgg#24836) appears once and is byte-identical to nightly. Upstream owns it now, there is no fork residue to move, and deleting it would remove upstream coverage.Remaining diff:
ProjectSetupScriptRunner.test.tsUpdateDrainAdmissionandWorkspaceAcquisitionTestkit, needed to construct the fork service without exercising admission or workspace acquisition.busystate:hasForegroundSubprocessmock, withemitupdatingbusyonstarted/restarted/activity. The fork foreground-readiness implementation needs these; without them, pinned readiness cases fail or call an unmocked method.admitmock for fork service dependencies.expect(listeners.size).toBe(2)before interruption, because the fork implementation has one listener. The upstream test still waits for opening, interrupts, and asserts zero listeners. The fork's exact one-then-zero count lives in the moved cancellation case.No other upstream setup test body differs.
Remaining diff:
Manager.test.tsThe file equals original base
20bcf5d6caminus the two #499 blocks. Everything below already existed there and is untouched. No pinned terminal test was deleted, and no upstream equivalence is claimed for these additions.Supporting code and imports
providerInstanceConfig.tsinstead of upstreamProviderInstanceRegistryHydration.ts.TerminalMetadataStreamEvent,TerminalSummary,Scheduler,UpdateDrainAdmission, workspace acquisition types/testkit,registerWorkspaceLeaseRoot.terminalOutputStreamis absent; fork tests use the manager-owned output stream API.vi.mockis absent (no runtime effect).FakePtyProcessadds kill failures, cached/one-time exit delivery, exit-on-kill, exit-listener clearing and a typedonKillargument, all for retained-process shutdown tests.multiTerminalHistoryLogPathfor terminal-scoped profile and archive cases.CreateManagerOptionsadds workspace acquisition, log directory, local CI settings path, process-exit wait, unregister hook and spawned-shell identity; reorders the registration-hook property.createManagerforwards them, defaults exit wait to 10 ms, and provides the selected workspace acquisition layer.exitSnapshotTestProcessesandcreateSnapshotBoundaryManager, placed between the multiple-terminal test (body unchanged) and the first snapshot test.Modified pinned cases (5)
/bin/zshfixture identity for fork shell classification.manager.attachOutputStream(input, "startup-viewer")instead of upstream'sterminalOutputStreamwrapper.deleteHistory: falseon close; puts the lock-holding terminal in another workspace (fixturebaseDir) so the workspace lease doesn't block reopen.All other common case bodies match nightly.
Added fork cases (85), by behavior covered
Remaining diff:
FoundationPersistence.test.ts(7 hunks)OrchestrationV2Subagenttype importawaitIncomingSummaryWork,runIncomingSummaryOnce,nextIncomingSummaryClaimableAt(3 hunks)groups shutdown commits with rollback=%s,atomically cancels stale runs and their process-bound effects,settles a native subagent's child thread when its provider process is gone.keeps a delayed preparation retry from holding back later thread workprepared-run.auto-retryrow inretires live provider effects and requeues $type after process losscommits recovery checkpoint effects only with the exact running attempt·selects unfinished recovery receipts for startup even without live provider work·guards recovered background rows against completed history and independent work (during finalization: %s)(true,false)Remaining diff:
DelegatedCompletionDelivery.test.tsOrchestratorMcp,ThreadReadBroker,PeerForwarding,RemoteDelegation,ThreadLaunch,GitVcsDriver,ProviderRegistry,ProviderAdapterRegistry,ScheduledTaskService,SecretRequests,CommandReceipts,ActionResume,ActionRunStore,ThreadManagement,ProjectStore,TerminalManager,WorkspaceAcquisitionTestkit,ProviderRegistryMock,ACTION_RUN_ID_ENV,TerminalEventSuspensionRegistryTestkit,ServerEnvironment,EnvironmentId,UpdateDrainAdmissionTestkit,ProviderInstallationGate,makePausedEnvironmentEnvironmentIdalso types remote ownership selectors in the nested-wake test.SourceControlProviderKind,PullRequestOperationError,PullRequestDetail,RunAttemptId,Ref,Option,ProjectionStore,PullRequestWatchReactor,PullRequestService,ProjectionReplay,EventStorelayerTestadds action-run/projection/replay/outbox/receipt/project-store layersEffectOutbox.layerduplicates one already in the original merge and is left as it was.layerTestadds environment identity, open drain admission, provider-installation gate, suspension registry; settings/platform go fromprovidetoprovideMergekeeps a checkpoint completion committed between finalize's read and writereplaced bykeeps checkpoint completion and %s ownership through a nested wake(local,remote,remote-disposed)delivers a %s follow-up independently of the original taskgainscancelled-before-startand its branchescancelled, skip the never-created pending question. Covers the outcomes row and adjacent indentation/status hunks.serializes a recovered child answer with its parent and tracks the follow-up: parent IDthread:question-answer-0-parentplus commentkeeps the %s task open across a real Action launch and result(36 rows, 1,282 lines incl. local fixtures/mocks)owned pull request watch continuationsgroup (1,178 lines incl. shared seed helper)Watch group:
retains exact Task ownership through %s:terminal,later-terminal,later-task-cancel,external-unwatch,unlink,stack-dismiss,replace,owner-unwatch,settle,stop,task-cancel,archive,delete.replays the saved terminal event after rejected admission and a reactor restart without reading the host.retains an earlier pending update and a racing final event, including cancellation before the final wake starts.keeps a %s automatic watch wake outstanding without starting it:paused,held.preserves remote source-task association until %s:complete,disposed.processes an owned reactor %s event:merged,closed,comment-limit,quiet,unreadable-merged.Upstream's
finalizes a child whose run ended before %s settledis unchanged and follows the inserted Action matrix. A histogram diff shows the matrix as a pure insertion.Additional authorized retirements
All comparisons use the same pinned nightly. Product code, including #654 and #236, is unchanged.
202b95403a7d6b7b01390b6a90457fa76e1be439rotates more than 64 live parents without scanning retained tombstones550f119c3ac12b2df070c4fd545ab8b41222debbkeeps native image bytes, saved paths, and untrusted window context in the promptadapter.lastcode.test.ts. Pinned Pikeeps native image bytes and the saved path in the promptcovers saved path/nativeAQIDimage payload, and provider-coreframes captured-window context as escaped untrusted JSONcovers formatting. The fork integration case additionally proves Pi forwards capture metadata alongside the native image into its prompt, so it is retained. Both upstream tests stay unchanged. The fork suite uses the same real adapter/runtime and host layers, with a fake RPC process limited to the ordinary open/ensure/start path.c79ef8c454e1268ff79da5a9dcb74572e86f7861selects native collaboration modes without prompt overrides with hasT3Mcp=%s; Default collaboration replay expectations;Codex runtime contextsuite renamecodexReplayPreamble,stops asking for commands the user allowed for the sessionreplay expectation and runtime-context rename are upstream's versions. No Codex files or transcripts changed.4daec109cdf8e3e92b392017d7564e735e54f6fdpreserves a one-time grant after an $label scope request,emptyandungrantedrowsaccepts lowercase one-time tokens exactly oncecovers omitted scopes and reuse, while the bound-proof test covers rejection then explicit-scope success. Neither covers the complete rejected request → same grant succeeds with omitted scopes and exactlyorchestration:read→ reuse fails sequence. Real SQLite/config/Node services remain.82515785bb6ec7b1b14894016977117864627b33serves confined reads and leaves edits to the agent's diff toolsresolves missing nested paths through a workspace alias before reading. Missing nested read through an external alias into the workspace must return resource-not-found rather than containment denial; after canonical creation the same alias readsnested. Upstream covers an internal existing-file symlink and escaping-link rejection, not this sequence. The sibling calls the actualreadAntigravityClientTextFilehelper with the same allowed root and real filesystem; upstream keeps ACP adapter wiring coverage.d547e3b12a1af79e9dd81e422255a12d6ecf70d1keeps attachment media out of the cache once its signed URL expireshttp.lastcode.test.ts. Effect's upgrade fixed compression, but LastCode still emitsprivate, no-store, no-transformwhere nightly expectsprivate, no-store. Moving the whole case preserves its audio cache-policy and range assertions without changing product code or leaving a cache-policy test that only checks ranges.Pinned covering sources: Pi native image test, shared escaped-context test, pairing tests, alias integration test.
Remaining differences in the additional touched upstream files
AcpSessionRuntime.processTree.test.ts,PairingGrantStore.test.ts, andAntigravityAdapterV2.test.tsare byte-identical to nightly.adapter.test.ts: the entire remaining diff is the unchanged 37-linebinds spawned provider shells to the host's custom settings pathcase. It checks fork host settings/environment precedence. Imports and fixtures match nightly.http.test.ts: unchangedmakeDevHandlerand two CORS tables (83 lines) allow the four desktop app origins and reject hostile origins; unchangedstreams guarded file ranges, including suffixes and conditional requests(40 lines) checks descriptor-backed ranges, conditional fallback, cache policy, validators and contents; the upstream audio cache-policy case is absent because it moved with its fork-specific header assertion; unchangeddoes not apply document policy to raster imagesandserves inline videos with their declared mime type(19 lines) assert fork asset headers. No other differences remain.Local review: 3 rounds with gpt-6.1-terra (xhigh) and claude-opus-5-5 (xhigh), approved. Scope expanded after Terra approved round 1; the old-scope Opus run was cancelled. Round 2’s Pi coverage finding is fixed and both reviewers approved round 3. After a checkpoint rewrote the base, the identical patch was revalidated and both reviewers approved the refreshed head.
Implemented with GPT-6.1 Sol (xhigh), Codex harness in T3 Code.