Repository navigation
🤖 tests: cover createUnderTaskTreeLifecycleLock paths before the split - #7701
Merged
Merged
Conversation
Characterization tests for the create paths no other suite reaches: argument and bestOf validation, the parent gates, the agent hint, AI-settings errors, the queued branch, fork and persist failures, and the launch-failure CAS, admission and superseded-liveness branches. No production change.
Member
Author
|
@codex review |
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. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54c75a0ee5
ℹ️ 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".
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".
Contributor
🟢 e2e stories (Haiku): 14 passed22 agent steps · 7 replayed from cache · 15 model calls · 46.3k tokens (28% cached) All 14 tests in 9 files
e2e 0.20.0 · 1m 16s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 8.0k tokens (32% cached) All 3 tests in 3 files
e2e 0.20.0 · 20.5s · electron · run artifacts 🟢 e2e agent repros (Haiku): no tests selectedSpend: $0.00 of the $1.00 cap (proxy ledger) e2e 0.20.0 · 5.3s · 2 targets (web, phone) · run artifacts |
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…cleLock A launched task whose agent definition sets subagent.skip_init_hook (the built-in explore agent) logs "Skipping .xum/init hook (disabled for this task)" in its background init; one without it (exec) does not. Before this test, inverting the skip_init_hook check in createUnderTaskTreeLifecycleLock survived every suite. No production change. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
Member
Author
|
@codex review |
This was referenced Oct 28, 2026
Merged
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bbffcef6b
ℹ️ 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".
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".
…nderLock tests The skip_init_hook test now writes an executable .xum/init hook into the test project that creates a marker file under the test root, and asserts the marker is absent for explore (skip_init_hook: true) and present for exec, instead of matching the init log text. setup and setupRowlessParent capture every background init the launch registers (the host mock's registerExternalBackgroundInit is a no-op), and afterEach waits for them before it removes the test root, so no init still runs its shell probes under a deleted root or into a later test. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_
Member
Author
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 372a364ca3
ℹ️ 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".
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".
Member
Author
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. 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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 29, 2026
…anch (coder#7702) ## Summary Part 1 of 5: splits the queued branch of `TaskService.createUnderTaskTreeLifecycleLock` in place into named steps, with no behaviour change. The method drops from CC 122 to CC 113. On top of the test PR coder#7701. 329 changed lines. The parts go from the end of the method to its start, as in the `drainTerminalAttention` split (coder#7697 to coder#7699). The code that stays inline must be the start of the method. If a part split the start first, the whole remaining tail had to move into one new step, and the CRAP gate refuses a new function above CC 8. ## Steps in this part - `createQueuedTask` persists the queued row, announces the task and schedules the queue. - `appendQueuedTaskEntry` is the config-edit callback, now a method. - `reserveTaskDesktop` is the closure that reserves the desktop owner, now a method. - This part also defines the readonly records `TaskCreateRequest`, `TaskCreateParent`, `TaskCreatePlacement` and `TaskCreatePlan`. Each one builds on the last with spread. ## Rules every part keeps - The method keeps `await using _lock = await this.mutex.acquire()`. Inside that scope, a bare `return promise` releases the global mutex before the promise settles. So every call from the method to a step is `await` or `return await`, and no step acquires the mutex, which is not reentrant. - The steps form a chain: each step calls the next one directly after its own last await. So no collaborator call runs a microtask later than in the inline code. Only the result of the method resolves later, at approximately 2 microtasks per chain level. The mutex therefore releases that much later, at the same logical point. - `make crap-check` against the parent commit reports 0 violations, with and without coverage. Every new function scores CC 8 or less. - `bun test` on the taskService, ledger, workspaceTurnManager, workflows, tools and workspaceService suites: 4,968 pass, 0 fail. These suites include the 27 tests from coder#7701. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$182.90`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=182.90 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 29, 2026
…ilure (coder#7703) ## Summary Part 2 of 5: splits the launch-failure path (the `failLaunch` closure) of `TaskService.createUnderTaskTreeLifecycleLock` in place, with no behaviour change. The method drops from CC 113 to CC 88. On top of part 1. 442 changed lines. ## Steps in this part - `failTaskLaunch` takes the rollback options as an argument, because its three call sites pass different options. - `isLiveUnderLaunchAttempt` checks whether anything still runs under the launch attempt. - `endPublishedTaskLaunchFailure` writes the row marker as a CAS. When it fails, it logs and returns, with no settlement. - `stopFailedTaskLaunch`, `settleFailedTaskLaunch` and `finishTaskLaunchFailure` handle the rest of the failure. - The mutable `TaskLaunchState` record holds `materializedCheckout`, `attemptId`, `launchAttempt` and `sendAdmitted`. Those values are written after the record exists, and `failTaskLaunch` reads them. `failTaskLaunch` reads `launchAttempt` once when it starts, and nothing writes it during the failure path. - The stop cascade inside the failure path still runs Phase A directly. That is correct, because this method already holds the global mutex. ## Rules every part keeps - The method keeps `await using _lock = await this.mutex.acquire()`. Inside that scope, a bare `return promise` releases the global mutex before the promise settles. So every call from the method to a step is `await` or `return await`, and no step acquires the mutex, which is not reentrant. - The steps form a chain: each step calls the next one directly after its own last await. So no collaborator call runs a microtask later than in the inline code. Only the result of the method resolves later, at approximately 2 microtasks per chain level. The mutex therefore releases that much later, at the same logical point. - `make crap-check` against the parent commit reports 0 violations, with and without coverage. Every new function scores CC 8 or less. - `bun test` on the taskService, ledger, workspaceTurnManager, workflows, tools and workspaceService suites: 4,968 pass, 0 fail. These suites include the 27 tests from coder#7701. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$182.90`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=182.90 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 29, 2026
… announce and send (coder#7704) ## Summary Part 3 of 5: splits sanitize, announce and the launch send out of `TaskService.createUnderTaskTreeLifecycleLock` in place, with no behaviour change. The method drops from CC 88 to CC 78. On top of part 2. 94 changed lines. The moved code keeps its indentation, so git sees few changes. ## Steps in this part - `sanitizeTaskLaunchCheckout` still runs before the metadata emit. - `announceAndInitTask` emits the metadata, reads the secrets, then calls `registerExternalBackgroundInit`. - `sendTaskLaunchPrompt` makes the send admission with no await before it, in the same continuation as `registerExternalBackgroundInit`. ## Rules every part keeps - The method keeps `await using _lock = await this.mutex.acquire()`. Inside that scope, a bare `return promise` releases the global mutex before the promise settles. So every call from the method to a step is `await` or `return await`, and no step acquires the mutex, which is not reentrant. - The steps form a chain: each step calls the next one directly after its own last await. So no collaborator call runs a microtask later than in the inline code. Only the result of the method resolves later, at approximately 2 microtasks per chain level. The mutex therefore releases that much later, at the same logical point. - `make crap-check` against the parent commit reports 0 violations, with and without coverage. Every new function scores CC 8 or less. - `bun test` on the taskService, ledger, workspaceTurnManager, workflows, tools and workspaceService suites: 4,968 pass, 0 fail. These suites include the 27 tests from coder#7701. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$182.90`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=182.90 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 29, 2026
…d materialize (coder#7705) ## Summary Part 4 of 5: splits the launch branch and the `materialize` closure of `TaskService.createUnderTaskTreeLifecycleLock` in place, with no behaviour change. The method drops from CC 78 to CC 61. On top of part 3. 440 changed lines. ## Steps in this part - `launchTask` keeps the `reserveTaskDesktop(materialize)` call shape, so the desktop gate ends before `failTaskLaunch` runs. - `materializeTaskCheckout` keeps every await of the old closure in its own body. It returns the whole checkout record, which has 3 more fields than before. Nothing reads them. - `shareParentCheckout` and `buildTaskForkOptions` prepare the checkout. - `appendLaunchedTaskEntry` is the config-edit callback. After the `editConfig` await, `beginOwnedTaskAttempt` runs in the same continuation, as before. ## Rules every part keeps - The method keeps `await using _lock = await this.mutex.acquire()`. Inside that scope, a bare `return promise` releases the global mutex before the promise settles. So every call from the method to a step is `await` or `return await`, and no step acquires the mutex, which is not reentrant. - The steps form a chain: each step calls the next one directly after its own last await. So no collaborator call runs a microtask later than in the inline code. Only the result of the method resolves later, at approximately 2 microtasks per chain level. The mutex therefore releases that much later, at the same logical point. - `make crap-check` against the parent commit reports 0 violations, with and without coverage. Every new function scores CC 8 or less. - `bun test` on the taskService, ledger, workspaceTurnManager, workflows, tools and workspaceService suites: 4,968 pass, 0 fail. These suites include the 27 tests from coder#7701. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$182.90`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=182.90 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 29, 2026
…parent, placement, agent and AI (coder#7706) ## Summary Part 5 of 5: splits the start of `TaskService.createUnderTaskTreeLifecycleLock` in place, with no behaviour change. The method drops from CC 61 to CC 2. On top of part 4. 423 changed lines. Across the five parts, the method goes from CC 122 to CC 2, and every new function scores CC 8 or less. ## Steps in this part - `readTaskCreateRequest` and the module function `normalizeTaskCreateBestOf` validate the input. - `loadTaskCreateParent`, `readTaskCreateParent` and `refuseTaskCreateUnderParent` load the parent and refuse a create under a wrong parent. - `countAndPlaceTask`, `placeTaskWorkspace` and the module function `decideSharedTaskWorkspace` count active tasks and place the workspace. - `readTaskCreateParentBranch`, `describeRunnableTaskAgents` (the former `getRunnableHint` closure), `checkTaskAgentRunnable` and `resolveTaskCreateAiSettings` handle the parent branch, the agent and the AI settings. The method now holds only the lock and `return await this.loadTaskCreateParent(...)`. That `return await` is the one await this split adds, and the `await using` rule requires it. A script compared the old and new code: every collaborator is called the same number of times. ## Rules every part keeps - The method keeps `await using _lock = await this.mutex.acquire()`. Inside that scope, a bare `return promise` releases the global mutex before the promise settles. So every call from the method to a step is `await` or `return await`, and no step acquires the mutex, which is not reentrant. - The steps form a chain: each step calls the next one directly after its own last await. So no collaborator call runs a microtask later than in the inline code. Only the result of the method resolves later, at approximately 2 microtasks per chain level. The mutex therefore releases that much later, at the same logical point. - `make crap-check` against the parent commit reports 0 violations, with and without coverage. Every new function scores CC 8 or less. - `bun test` on the taskService, ledger, workspaceTurnManager, workflows, tools and workspaceService suites: 4,968 pass, 0 fail. These suites include the 27 tests from coder#7701. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$182.90`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=182.90 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Oct 30, 2026
) ## Summary Adds 21 characterization tests for `TaskService.prepareTaskReservation`, with no production change. Line coverage of the method goes from 81 % (235 of 289 lines) to 98 %. Only a `log.debug` branch stays uncovered. This is the test PR at the start of stack 7 of the TaskService recovery and launch path split, from the "TaskService recovery and launch path plan" artifact, all defaults kept. The split PRs on top of it split the method (CC 62) into named steps. ## What the tests cover The new file is `taskService.prepareTaskReservation.test.ts`. All tests go through `createMany` on a local parent and use the existing harness. Every refusal test also checks that no workspace row was written and, where it applies, that nothing was sent. - **Input:** a malformed agent id. Six bad `bestOf` values are refused before the parent is read, and a valid `bestOf` is saved on the task row. - **Parent gates:** an unknown parent, an untrusted project, a parent that already reported, and a parent nested too deep for `maxTaskNestingDepth`. - **Agents:** a disabled agent, an unknown agent whose discovery fails, and a transport error while reading the definition. - **AI settings:** - An invalid explicit model is refused. - A transport error returns an error result. - Any other error propagates. A second `createMany` then finishes, which shows that the parent tree lock was released. - **Conditions that line coverage missed:** a parent exactly at `maxTaskNestingDepth` is accepted, and one deeper is refused. `isolation: "none"` on a local parent does not share the checkout. `skip_init_hook` decides whether a real `.xum/init` hook runs on launch. That test clears the project-automation kill switch and restores it. ## Validation - The second commit came from a hand mutation run during the split. It covers three conditions whose mutants survived every test before. A fourth mutant, on the runnable-agents hint, changes only message text, so it has no test. - All 21 tests pass on the current code. They passed three runs in a row, and one run with `XUM_DISABLE_PROJECT_AUTOMATION=1`. - A scratch script made 24 edits to the production code, one per covered branch, and restored the file after each run. Each edit failed its own test. - The tests check short parts of the returned errors, such as the field name, as the merged coder#7701 does. They check no log text. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$257.41`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=257.41 -->
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.
Summary
Adds 27 characterization tests for
TaskService.createUnderTaskTreeLifecycleLock, with no production change. Line coverage of the method goes from 94 % (673 of 716 lines) to 100 %, except one check that a generated task name is valid, which no input can reach. This is the test PR at the start of stack 5 of the TaskService hotspot split. The split PRs on top of it split the method (CC 122) into named steps.What the tests cover
The new file is
taskService.createUnderLock.test.ts. It uses the existing harness.bestOfvalues.skip_init_hookflag. An agent with the flag skips the init hook on launch, and one without it runs the hook.running, and the attempt staysclosing.interrupted, and the attempt is settled.Three tests simulate another writer in the middle of the create. Two wrap
config.editConfigonce, and one makeseditWorkspaceEntryreject. No plain input reaches those lines.Validation
skip_init_hooktests close a gap that mutation testing found: no test failed when the check was inverted. Both new tests now fail under that mutant.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$164.39