🤖 refactor: retarget tests from test-only wrappers to the functions production calls - #4487
Merged
ThomasK33 merged 1 commit intoSep 25, 2026
Conversation
ThomasK33
added this pull request to stack #4488
September 25, 2026 08:57
Codex Review SummaryThis 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. |
ThomasK33
force-pushed
the
test-audit/ws2-test-only-wrappers
branch
from
September 25, 2026 09:11
da212d3 to
97ffb7d
Compare
github-merge-queue
Bot
removed this pull request from the merge queue because a pull request earlier in the stack was removed
Sep 25, 2026
github-merge-queue
Bot
removed this pull request from the merge queue because a pull request earlier in the stack was removed
Sep 25, 2026
ThomasK33
force-pushed
the
test-audit/ws2-test-only-wrappers
branch
from
September 25, 2026 10:39
97ffb7d to
cae4be7
Compare
ThomasK33
force-pushed
the
test-audit/ws2-test-only-wrappers
branch
from
September 25, 2026 10:59
cae4be7 to
dbab755
Compare
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Sep 25, 2026
…ervice context tests (coder#4530) ## Summary Makes `WorkflowService.context.test.ts` independent of test order by resetting the process-global workflow archive admission guard before each test. Test-only. ## Background `WorkspaceService`'s constructor installs a module-level guard with `setWorkflowArchiveAdmissionGuard` that calls `this.config.loadConfigOrDefault()`. Since coder#4475 split `workspaceService.test.ts`, the split files can land in the same bun unit shard as `WorkflowService.context.test.ts`. When one runs first with a partial config double, the leftover guard throws `this.config.loadConfigOrDefault is not a function` inside every workflow start/resume, and 9 context tests fail. Reproduced on current `main` (7c133d0) with one process: ``` bun test ./src/node/services/workspaceService.archive.test.ts ./src/node/services/workspaceService.bashAndFiles.test.ts ./src/node/services/workflows/WorkflowService.context.test.ts ``` Main's own CI passes only because shard balancing currently keeps those files apart. The WS2 test-audit stack (coder#4483–coder#4487) removes test files, which reshuffles the shards, and its merge groups failed `Test / Unit (4/6)` on exactly these tests. ## Implementation `beforeEach` resets the guard to admit everything, the same reset `WorkflowService.test.ts` already uses. The underlying leak (every `WorkspaceService` instance overwrites a process-global guard and never unregisters it) belongs to the WorkspaceService owner and is reported to that workstream rather than redesigned here. ## Validation The reproduction command above: 9 failures before, 102 pass after. `bun test src/node/services/workflows/` passes. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$4.45`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=4.45 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Sep 25, 2026
…on sets, assisted matchers, workflow transitions) (coder#4483) ## Summary Removes four production exports that only tests called, from the test-audit follow-up campaign (WS2, crosscutting candidates coder#1–coder#4). Behavior-preserving: no production caller changes. ## Removed | Symbol | Why it was dead | Tests | | --- | --- | --- | | `isCopilotRoutableModel` (`copilot/modelRouting.ts`) | Always returned `true`; no caller since coder#3104 | 2 tests restating the constant deleted | | `gatherInstructionSets` (`instructionFiles.ts`) | No caller since coder#439; `systemMessage.ts` calls `readInstructionSet` directly | 4 tests deleted; `readInstructionSet` tests already cover null, local files, scope and `projectName` | | `findAssistedMatch`, `hunkMatchesAssisted` (`assistedReview.ts`) | Replaced by the `*Candidate*` versions in coder#3370; only tests called them | Range overlap, deletion fallback (old-side span), rename (`oldPath`), first-match index and no-match cases moved onto `findAssistedCandidateMatch`, the function `ReviewPanel` calls. `hunkMatchesAssistedCandidate` is now module-private | | `WorkflowRunStatusTransitionSchema` + its table (`orpc/schemas/workflow.ts`) | Never enforced; coder#4089 kept it only because a test imported it. Real enforcement lives in `WorkflowService`/`WorkflowRunStore` (tested in `WorkflowRunStore.test.ts`) and follows different rules | "rejects impossible status transitions" deleted | ## Validation - Mutation check: disabling the deletion fallback and the `oldPath` match in `hunkMatchesPathAndRange` fails the two moved tests. - `bun test src/common/utils/review/ src/common/utils/copilot/ src/node/utils/main/instructionFiles.test.ts src/common/orpc/schemas/workflow.test.ts src/node/services/tools/review_pane.test.ts` - `make static-check` on the stack top. Stack: 1/5 (coder#4483 → coder#4484 → coder#4485 → coder#4486 → coder#4487). --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$4.45`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=4.45 -->
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
1 participant
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.
Summary
Deletes production wrappers that existed only so tests could call them, and points those tests at the functions production actually calls.
Changes
evaluateGoalContinuationWorkspaceGoalServicerunsevaluateGoalContinuationBeforeGoal, loads the goal, thenevaluateGoalContinuationGoallistMemory…consolidateMemory)router.tsyields the*Effectfunctions viahandlerGenEffect.runPromise(<name>Effect(...))buildSystemMessage,readToolInstructionsturnContextAssemblerloads sources once, thenbuildSystemMessageFromSources/extractToolInstructionsFromSourcesbuildSystemMessageFromSourceshasInterruptedStream,isEligibleForAutoRetryStreamingMessageAggregator/ChatPanereadgetInterruptionContext(...)getInterruptionContext(...).<field>(incl. one browser test file, mechanical)scanHistoryRowshistoryReplacementRowsopens its own handle and callsscanHistoryRowsFromHandlehistoryRowScanner.testHarness.ts(excluded from the main build). "observes cancellation during file disposal" tested only the wrapper's own dispose path and is deletedComments that pointed at
buildSystemMessagenow namebuildSystemMessageFromSources.Validation
bun testfor goalContinuationPolicy, memoryOperations, systemMessage, retryEligibility, StreamingMessageAggregator.tokenBudget, historyRowScanner, historyMessageEvidence, historyScalarEvidence, historyReplacementRows;make static-check.Stack: 5/5.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$4.45