feat(doctor): extend the delivery check to rules, agents, MCP and env - #669
Conversation
`doctor` asked "can this tool receive skills" through `skillsReachTool`, which had to invent a skill name (`__teamai_probe__`) because `skillTargetForTool` fused two questions: whether a tool receives skills at all, and where a given skill lands. Only a comment said the invented name could not affect the first. Split the gate from the path. `skillsDirForTool` answers the gate on its own — OpenClaw's workspace, Hermes' home, Copilot's enabledAgents, else the tool root — and `skillTargetForTool` is that directory plus the skill name, with Codex's shared-directory redirect on top since only that one is per-skill. Add `ResourceHandler.deliveryTargets`, the read-only seam Tencent#624 asks for: where an item lands for each tool that receives it, `null` for a resource with no per-tool file destination. `SkillsHandler` implements it, and its `pullItem` now walks the same resolved targets, so the write path and the check cannot answer differently. `buildDeliveryChecks` consumes the seam through the handler registry instead of importing `SkillsHandler` directly; check names, failure buckets and fix text are unchanged. Also points the inactive-skill cleanup at the same gate. It probed the tool root and then swept `<base>/<skills path>`, which for OpenClaw is a directory delivery never writes to — the real workspace copy was never pruned.
A rule changes both its filename and its bytes per tool: `.md` verbatim for Claude, `.mdc` with derived `globs`/`alwaysApply` for Cursor-compatible tools, `.instructions.md` with `applyTo` for Copilot. Nothing exposed where one lands, so `doctor` could not ask — the extension table lived inside `pullItem`. `RulesHandler.deliveryTargets` answers it, and `pullItem` now walks the targets it returns rather than rebuilding the gate chain, so the check and the write path resolve the same paths. `resolveDesiredRules` joins `resolveDesiredSkills` in pull.ts: the namespace convention and the tag channel are stated once, and the check reads them rather than restating them. The check reports two buckets per tool: a rule that never arrived, and one that arrived without the frontmatter its tool reads — a `.mdc` without `alwaysApply` is inert, which no write-time gate can see because the write succeeded. The fix names the destination directory, since the filename is not the rule's name. A legacy `.md` left beside a correct `.mdc` is deliberately not reported: it is inert leftover that `pullAllRules` already sweeps, not a delivery failure.
Agents break the items × tools shape the skills check assumes: a spec carries `targets:`, so the desired set is a relation, and each tool renders its own format, so the filename comes from the render and not from the agent's name. `AgentsHandler.deliveryTargets` is therefore the only thing that can say where an agent lands, and `pullItem` now walks the same resolution — the YAML and legacy paths merge into one loop instead of two gate chains. `resolveDesiredAgents` joins its skills and rules siblings in pull.ts, so the namespace filter and its stem-collision throw are stated once; `doctor` reports that throw as a failing check rather than stack-tracing, as it already does for skills. Two bugs surfaced while unifying the paths: - `renderedForTool` decided "legacy" from `item.legacy` alone while `pullItem` also accepted a non-`.yaml` source. An item built without the flag was therefore parsed as a spec by the cleanup and copied verbatim by the pull. `isLegacyAgent` now answers it in one place. - The parse-failure warning was Chinese, which the repo forbids in production code, and went through `console.warn` rather than the logger. Also adds a check for an agent that renders for no installed tool at all: the file is in the team repo, `pull` names the reason once, and nothing afterwards says it is still reaching nobody.
An MCP server is an entry inside a tool's native config, not a file of its own, so this check takes the shape of the hook check rather than of the delivery seam: it asks which servers the reconcile would want for a tool, then whether that tool's config carries them. The desired-set pass moves out of `reconcileMcpForConfig` into `desiredMcpForTarget`, unchanged — the `tools:` and `roles:` filters, the transport and policy gates, the `requires:` PATH check and the placeholder resolution all stay in one place, and the reconcile now calls it. A second copy of those filters is precisely how a server skipped once for an unresolved variable gets reported as delivered forever after. That skip reason is the point. A server dropped for `unresolved variable(s)` prints one line during a pull and is never mentioned again, so the member sees "MCP does not work" and goes looking at MCP. The check now names the server, the variable and `env/env.yaml` — including that its top-level key must be `variables:`, since a plain `KEY: value` mapping parses as no variables at all and silently skips every injection (Tencent#662). A server the member excluded on purpose is not reported; an unparseable tool config is, because the write path abandons the injection there too.
The env check asserted that `# [teamai:env:start]` appeared somewhere in the profile. That is true of a block that cannot load and of a run that delivered nothing, so both failures passed and surfaced three layers away as MCP servers skipped for `unresolved variable(s)`, with nothing pointing back at env. It now asks the three questions the marker stands in for: - Does `env.yaml` declare anything? A file with content that parses to zero variables is the shorthand `KEY: value` form, which zod strips to an empty list — the pull then writes nothing and logs nothing (Tencent#662). - Did every declared variable reach `env.sh`? - Would the injected block load it? The block is built with the platform separator, so on Windows it carries backslashes; a POSIX shell reads an unquoted `\` as an escape, the `[ -f ... ]` test fails, `&&` short-circuits and `source` never runs, silently (Tencent#661). Whitespace in the path needs quotes for the same reason. Neither underlying bug is fixed here — Tencent#661 and Tencent#662 own those. This is the row missing from the issue's table: env had no check that looks at the payload, which is why both of them reach `All checks passed!`. The check is still emitted when there is nothing to deliver, passing, since `doctor --json` consumers cannot tell an absent entry from a passing one.
…check The post-pull pass re-runs the registry under a 5s all-or-nothing budget that covers building it as well as running it. Skills and docs cost a stat per item; rules cost a read per rule per tool and agents parse every spec. Adding those to the pass would spend the budget on the expensive checks and lose the cheap ones — and going over means the member gets no check at all. `buildChecks(ctx, stage)` takes 'pull' or 'doctor' and does not build the two expensive registries for 'pull'. The stage is a property of the caller, not of a check, so it is an argument rather than a third optional flag on `Check` beside `source` and `reportedByPull` — which the issue flags as the point where that object stops reading. Skipping is at build time, not a filter over the result: the cost is in building the registry, so filtering afterwards would save nothing.
Review findings, all three from the repo's own standards. `doctor.ts` had grown to 959 lines, most of it domain logic: where a rule lands for Cursor, which tools an agent's spec targets, whether a shell block would load. CONTRIBUTING says commands in `src/*.ts` stay thin and the heavy lifting lives elsewhere. The checks move to `doctor-delivery.ts`, and `doctor.ts` is back to being the registry that runs them — smaller now than before this branch. The three per-tool builders repeated one shape: walk items × targets, bucket the failures by tool, remember the directory, format a check. `walkDelivery` holds that walk and takes a `classify` callback for the part that genuinely differs; `describeProblems` formats the buckets in the caller's label order, so the same broken machine reads the same way twice rather than in the order its failures happened. `envDeliveryProblems` had its own copy of the `$SHELL` → `.zshrc`/`.bashrc` choice, a second spelling of what `EnvHandler.detectShellProfile` already decides — the exact failure this branch exists to prevent, one layer down: it would check `.bashrc` while the pull wrote `.zshrc` and call a correct install broken. That method is now public and the check calls it. No check name, failure bucket or fix string changes.
AGENTS.md's review rules reject unused flexibility, and this was some. The seam returned `DeliveryTarget[] | null`, where `null` meant "this resource has no per-tool file destination" and `[]` meant "no installed tool receives it here". The single caller wrote `?? []` and treated them alike, so the distinction only cost a branch nobody took. The default is `[]` now, and the comment carries the meaning the type was trying to.
Moving the checks into `doctor-delivery.ts` left nine imports in `doctor.ts` with no remaining user: `fs`, `expandHome`, `listFilesRecursive`, `TEAMAI_ENV_END`, `getMcpSharing`, `usesCursorMdcRules`, `usesCopilotInstructions`, `splitFrontmatter` and the `ResourceItem` type. `tsc --noEmit` stays green either way because `noUnusedLocals` is off, so CI could not have caught these. They make `doctor.ts` look like it still reaches into frontmatter parsing and MCP sharing config, which is the impression the move existed to remove.
The PR description includes a test plan and real-CLI/e2e verification record, so it satisfies the testing-description requirement. |
…enders `Every team agent reaches a tool` was gated on `byTool.size > 0`, using successful deliveries as the proxy for "some tool was there to receive an agent". It is the wrong proxy for exactly the case it exists to catch: when every agent is malformed or targets tools that are not installed, no agent renders anywhere, `byTool` is empty, no check is built at all, and `doctor` reports success on a machine where nothing arrived. The gate is now the installed tools themselves. `AgentsHandler.agentToolDirs` answers that on its own — the tool-path, exclusion and install gates without asking any agent to render — and `resolveRenders` and the inactive-agent cleanup, which both carried their own copy of that loop, now go through it.
…names The check asked whether the desired server name was a key in the tool's config. Reconciliation never overwrites an entry teamai does not own, so the one case the write path deliberately skips — a server of your own under a team name — satisfied the check: the key is there, the team's server is not, and every later pull skips it again without a word. `installedMcpEntries` replaces `installedMcpServerNames` and returns the entries in the rendered form `desiredMcpForTarget` produces, so the check compares values. Structurally, via `isDeepStrictEqual`: key order in a JSON config is not meaning, and a tool that rewrites its own file should not read as a failure. Codex stores a TOML block rather than a JSON value, so `codexBlockIn` extracts the block by the same regex `spliceCodexBlock` writes with, trimmed to the single trailing newline `renderCodexBlock` emits. A stale entry and a foreign one are reported alike, as `not the team's definition` — both mean the tool is not running what the team declared — and the fix says that a pull leaves an entry teamai does not own alone, so only `--force` replaces it.
…keys `export KEY=` as a substring is true of the value env.yaml declares and of the one it replaced. A rotated credential that never reached `env.sh` — the pull that would rewrite it skips a scope whose team repo has not changed — passed the check while every shell and every MCP server kept exporting the old value, which is the failure this check exists to name. Each declared variable is now compared against the line `generateEnvFile` would write for it, the injection's own rendering rather than a second copy of its quoting, and a key present with a different value is reported as stale rather than as missing. Neither value is printed: these are credentials, and the key is the whole diagnosis. The e2e fixture delivered an MCP entry and an `env.sh` that were not what teamai writes; it now carries the rendered forms, and covers a foreign server under a team name and a stale `env.sh` through the built CLI.
…y check The MCP and env paragraphs described a name lookup and a key lookup. Both now compare values, and the MCP one reports a server of your own holding a team name — which only `teamai pull --force` replaces — so the guide and the changelog have to say so. Both language versions.
|
All three P1 findings are real. Fixed in 1. Malformed-only agent sets produce no failureConfirmed. The gate is the installed tools now. -if (unreachable.length > 0 && byTool.size > 0) {
+const agentTools = await handler.agentToolDirs(teamConfig, localConfig);
+if (unreachable.length > 0 && agentTools.length > 0) {2. MCP name collisions are falsely reported as deliveredConfirmed, and worse than a one-off: the write path skips that entry on every later pull too, so the machine stays wrong and silent forever.
A foreign entry and a stale one are reported alike, as The manifest would let the message name which of the two it is. I left it out: the same command fixes both, and reading it means the project-scope per-worktree path with its migrate-on-read, inside a check whose contract is that it never writes. 3. Stale environment values pass the delivery checkConfirmed, and it is the credential-rotation case: Each declared variable is compared against the line Verification
New tests, each failing before its fix: The e2e fixture was itself an instance of finding 2: it delivered Through the built CLI, one team repo where every agent is malformed, a All three exited 0 on
|
…tence The check asked only whether something readable sat at the destination, which is the same class of gap the three review findings were: an agent rendered from an older spec passes while the tool runs instructions the team replaced. A plain pull syncs a scope only when its team repo changed, so the copy can sit there indefinitely. `DeliveryTarget` carries the bytes `pullItem` writes, which `resolveRenders` already had in hand and threw away at the seam, and the agents check compares them. It is the same equality the inactive-agent cleanup already uses to decide a deployed copy is the team's. Absent `content` means the handler renders nothing — a skill is a directory tree — and only existence is judged, so skills and rules are unchanged. `walkDelivery` passes the target to `classify` rather than its two fields. The fixtures delivered the literal string `rendered`, which the new comparison correctly rejects: the unit tests now deliver through the handler's own seam, and the e2e fixture carries each tool's render byte for byte.
|
Follow-up in
export interface DeliveryTarget {
tool: string;
dest: string;
+ /** The exact bytes `pullItem` writes at `dest`, when the handler renders
+ its destination rather than copying a tree there. */
+ content?: string;
}-async (_tool, dest) => await isReadableFile(dest) ? null : 'not delivered',
+async ({ dest, content }) => {
+ const delivered = await readFileSafe(dest);
+ if (delivered === null) return 'not delivered';
+ return content === undefined || delivered === content ? null : 'delivered from an older spec';
+},It is the same equality the inactive-agent cleanup already uses to decide a deployed copy is the team's. Absent The fixtures were themselves the bug: they delivered the literal string
Guide, |
|
Findings
|
Routing the legacy `.md` path through `resolveRenders` also gave it the stale-sibling sweep, which the old `pullLegacyMd` never ran. A team agent named `helper` then deleted a `helper.toml`, `helper.json` or `helper.agent.md` the member wrote, with no ownership or content check. Only a rendered spec can leave a sibling behind: its extension follows the tool's format and changes when `targets` does. A legacy `.md` is copied verbatim to one extension for every tool, so anything else on the stem is not ours.
The check read the delivered file for the presence of `alwaysApply` or a nonempty `applyTo`. A `.mdc` whose `globs` no longer match the team rule's `paths:` passes that while Cursor applies it to the wrong files, and so does a body that drifted from the team `.md`. `RulesHandler.deliveryTargets` now carries the bytes `pullItem` writes, the way the agents handler does, and the check compares against them. That makes the render the single spelling of the mapping rather than a contract `doctor` restates in terms of the keys it happens to know about.
`parseTeamMcpServers` answers `[]` to an absent file and to one that does not parse alike. That is right for a pull, which can only skip the run, but it left `doctor` unable to tell a team with no MCP from a team whose every server reaches no tool: the desired set was empty, no per-tool check was emitted, and `doctor --json` reported ok: true. `readMcpYaml` returns the parse failure with its reason and the check reports it. `parseMcpYaml` keeps its old shape on top of it, so the pull path is unchanged.
`parseEnvYaml` answers `[]` to four different files: absent, empty, `variables: []`, and the shorthand `KEY: value` mapping whose unknown top-level key zod drops (Tencent#662). The check equated zero variables with the shorthand form, so an intentional `variables: []` was reported as malformed. `readEnvYaml` returns the reason instead of the count, so the shorthand form and invalid YAML are both named while an empty configuration fails nothing.
…e review The guides and the changelog entry describe what each check compares, and three of them now compare something else: a delivered rule against its render rather than its frontmatter keys, an unparsable `mcp.yaml` as its own failing check, and an explicit `variables: []` as an empty configuration rather than a malformed file. The e2e suite covers all four cases through the built CLI.
|
All five are real. Every one is fixed, each in its own commit with a test that fails without it.
1. The sweep exists because a rendered spec's extension follows the tool's format and changes when 2. Rule frontmatter presence, not values. Correct. Taking your second option: The label is now 3. Invalid
4.
5. Provider matrix. Correct — the Test Plan recorded
18 assertions, all passing. The only failing rows in the three reports are What the run confirms rather than assumes: All four fixes are also covered end-to-end through the built CLI in The 26 skipped are the three live provider suites (cnb, gitlab, gitcode), which need credentials this machine does not have. Two statements in the PR body were wrong once these landed, and are corrected there: the claim that gitlab and github needed no exercise, and the blast-radius note saying the legacy |
|
Findings
The PR description includes a detailed test plan and a real-CLI/e2e verification record, so the description-level testing requirement is satisfied. |
…tool `deliveryTargets` covers what `pullItem` writes under `toolPath.rules`. `pullAllRules` delivers two more things it cannot see, and both fail silently: OpenCode does not auto-scan a rules directory. Every `.md` can be there byte for byte and be inert, because `opencode.json` no longer lists the glob the pull owns — and `Rules delivered to opencode` passes throughout. Hermes has no rules directory at all: its rules are the contents of a managed block in SOUL.md. A deleted or stale block is a tool reading the wrong rules with nothing on disk to show for it. Both take the shape of the hook and MCP checks — one destination, not one per tool. `opencodeInstructionsTarget` and `hermesRulesText` are the single spelling each, so the check reads the answer the pull writes rather than deriving a second one.
A YAML block scalar is a legal env value, and `generateEnvFile` single-quotes it into an export spanning several physical lines. The check split env.sh on newlines and compared each line with a whole generated export, so such a value could never match: a correct pull was reported as a stale value on every run. `parseEnvFile` is the generator's inverse — it reads the assignments back, including the `'\''` encoding of an embedded quote — and the check compares values rather than lines.
…erse Two checks are new and one comparison changed, so the guides and the changelog entry describing them change with it. The e2e suite covers both through the built CLI: OpenCode rules delivered byte for byte while the glob is gone, and a multiline env value that the old line scan called stale.
|
Two of the three are real and fixed. The third does not reproduce on the current head — details below.
1. OpenCode does not auto-scan a rules directory, so a Both are now checks of their own, the shape the hook and MCP checks already take: one destination, not one per tool. Rather than a second derivation of each path, The 2. Docs still claiming rules, agents and MCP are unchecked. This does not reproduce on the PR head. The sentence you quote — "Rules, agents and MCP servers are not checked yet." and its
3. Multiline env values. Correct. A YAML block scalar is a legal env value,
Verification. Every new test fails without its fix; I checked each by reverting the change under it. Real CLI, one sandbox per provider — real bare origin, real clone, real
30 assertions, all passing. The only failing rows remain Both fixes are also covered end-to-end through the built CLI in |
The PR description includes both a test plan and real-CLI/end-to-end verification, so it satisfies the testing-documentation requirement. |
2 participants
Summary
doctorverified two of the seven resource types against what is on disk. The other five reported success while a tool received nothing, which is the failure #598 named and #625 started fixing.Each resource type answers "where does this land for this tool" in one place, and both the sync and the check read that answer.
Two rule destinations are not a file per tool at all, and each gets a check of its own rather than an entry in
deliveryTargets. OpenCode does not auto-scan its rules directory: a.mdcopied there is inert untilopencode.jsonlists the glob the pull owns, soTeam rules are active in opencodechecks that it still does. Hermes has no rules directory — its rules are the contents of a managed block in SOUL.md — soTeam rules are inlined in Hermes SOUL.mdcompares that block with what the team rules inline to.An empty result covers both "no installed tool receives this item" and "this resource has no per-tool file destination at all". Docs lands in one directory, env in one shell profile, and an MCP server is an entry inside a tool's own config, so those three keep checks of their own instead of an invented tool name.
Three cases needed more than a path.
A rule can land and still do nothing. Cursor reads
globsandalwaysApply, Copilot readsapplyTo, and the pull derives both. A copy that arrives without them gets its own failure line, separate from a rule that never arrived at all.An agent is owed to the tools its
targets:lists, and the filename extension comes from the render rather than from the agent's name. An agent that renders for no installed tool gets a row of its own.MCP servers are keys inside a tool's config file. The desired-set pass moves out of
reconcileMcpForConfigintodesiredMcpForTargetunchanged, and the check calls that same function. That is what lets the check report why a server was skipped. An unresolved${VAR}is named once during a pull and never again.The issue's scope list does not include
env. I added it because the same failure appears there one layer down, and two open issues are the evidence. On Windows the injected block carries backslashes, sosourcenever runs (#661). Anenv.yamlwritten asKEY: valueparses to zero variables, so every MCP injection is skipped (#662). Both reachAll checks passed!today. This PR reports them. It does not fix either one.Rules and agents run under
teamai doctoronly. The post-pull pass has a 5 second all-or-nothing budget that covers building the registry as well as running it, and these two read every rule for every tool and parse every agent spec.buildChecks(ctx, stage)does not build them when the stage is'pull', so the cheap checks keep the budget. The stage describes the caller, so it is an argument rather than a third optional flag onCheckbesidesourceandreportedByPull.Type of Change
Test Plan
npx tsc --noEmitpassesnpx vitest runpasses: 3577 tests, 258 files, 0 failuresUnit and integration tests follow the pattern in
doctor-delivery.test.ts: a real temp team repo and HOME on disk, with onlyconfig.jsand the logger mocked.npm run test:e2epasses: 171 tests, 26 skipped, 41 files. The 3 skipped files are the live provider suites (cnb, gitlab, gitcode), which need credentials this machine does not have.src/__tests__/e2e/doctor-delivery-cli.test.tsis new. It spawnsnode dist/index.js doctor --jsonagainst a temp HOME holding a team repo with one of each resource, across Claude, Cursor, Codex, CodeBuddy and OpenCode.Before. A team repo that ships one skill, one rule, one agent, one MCP server and a shorthand
env.yaml, with nothing delivered to any tool. Copied from the built CLI:After. Each file written where its tool reads it, and
env.yamlrewritten in thevariables:form:Five of those seven rows do not exist on
origin/main. Its registry has no rules, agents or MCP checks, and its env check returns true as soon asenv.shexists and the marker string is in the profile, so #661 and #662 both pass there.Provider matrix
Run against the built CLI, one sandbox per provider: a real bare origin, a real clone, a real
teamai pull, thenteamai doctor --json. Agents: Claude, Cursor, Codex, CodeBuddy, OpenCode.gitgitlabgithubteamai pullsyncs skills, rules, agents, env, MCP.mdagent leaves a member'shelper.tomlalone.mdcwith driftedglobsreportedmcp.yamlfails,doctor --jsonnot okteamai pull --forcerestores the globvariables: []fails nothing30 assertions, all passing. The only failing rows in the three reports are the provider auth checks —
GitLab token is configuredandgh CLI is authenticated— which fail because this machine holds no credentials for either. They areorigin/mainchecks this PR does not touch.teamai pullrefreshes the clone through plain git in every case:src/pull.tsnever calls a provider. The provider decides authentication and the initial clone, so it is aninit-time concern, and the delivery checks read local disk. The matrix confirms that rather than assuming it.Related Issues
Closes #624.
Reports #661 and #662 without fixing them. Both become visible
doctorfailures once this merges. I commented on each.Notes for Reviewers
Merge danger
Door: two-way.
Reverting is
git revertover the nine commits. Nothing persists state, no config schema changes, and no file format changes. The one place that deletes files, the inactive-skill cleanup, keeps its data-safety guard: it removes a deployed skill only when the copy is byte-identical to its team source, so the worst a revert has to undo is a directory that was correctly pruned.Blast radius: sync.
Three things reach past
doctor, in descending order of how likely they are to bite:doctorexits 1 where it exited 0, and--jsongrows rows. Anyone gating CI onteamai doctorsees red on the next run. That is the point of the change, but it will look like a regression to whoever hits it first, particularly on Windows ([bug] Windows: env 注入到 shell profile 时写的是原生路径，source 静默失败而 doctor 仍报通过 #661) and with a shorthandenv.yaml([bug] env.yaml 缺少顶层 variables: 时被静默当成空，导致所有工具的 MCP 注入全部被跳过，且 doctor 仍报通过 #662). Both have a comment explaining it.deliveryTargets. Same gates, same destinations, and the full suite covers them, but this isteamai pullrather than a read-only command. The agents legacy.mdpath also changed fromcopyFileto awriteFileof the same bytes. It does not sweep stale siblings: its extension is.mdfor every tool, so it can never leave one of its own behind, and a same-stem.toml,.jsonor.agent.mdbeside it is the member's file. Only the rendered-spec path sweeps, as it did before.Post-pull output is unchanged for a healthy machine, since the expensive checks do not run in the
'pull'stage.Deviations from the issue
Three places where I did not follow the issue, each one deliberate.
DocsHandler.deliveryTargetson the seam. Docs has one destination and no per-tool dimension, so a{tool, dest}[]contract would need an invented tool name. Docs and env stay off the seam.toolReceives(type, tool, ...)function to unify the installed gate. The same comment says it "does not need to exist as its own function" ifdeliveryTargetslands first, which is what happened. Every caller now goes through one function per resource type, andskillsReachToolwith its invented__teamai_probe__name is gone. The special cases that made skills different, OpenClaw's workspace and Hermes' home, live inskillsDirForTool. If rules or agents ever grow one, it has no shared home yet. I left it there rather than write a dispatcher for cases that do not exist.stageargument, not one of the three options the issue lists.Unifying the agent paths surfaced two bugs, both fixed in
97398f9.renderedForTooldecided "legacy" fromitem.legacyalone whilepullItemalso accepted a non-.yamlsource, so the cleanup parsed an item built without the flag as a spec while the pull copied it verbatim. The parse-failure warning was also Chinese, and went throughconsole.warnrather than the logger.The commits are ordered so each one stands alone. If you want this split, it splits at a commit boundary with nothing to redo.