An automated review sweep over #790 surfaced these. None are caused by that branch — it touches 26 files and none of the ones below. I verified each against origin/main before filing; where the review's framing was wrong, I've said so.
Filed together because they came from one pass, but they are unrelated to each other and want separate fixes.
Correctness defects
1. deleteRunEntries orphans the rows it deliberately leaves behind
packages/task-graph/src/storage/FsFolderTaskOutputRepository.ts:249 — verified
It deletes only the listed rows, then calls deleteBlobsByPrefix(nsPrefix), which sweeps every sidecar blob under the run prefix:
await deleteRowsBounded(doomed, (key) => this.storage.delete(key));
this.emit("output_pruned");
await this.deleteBlobsByPrefix(nsPrefix);A run that crashes after writing private cache rows and then resumes under the same runId hits this: RunPrivateCacheRepo.writeSet covers only the current process's writes, so clearRun names just the new rows. The earlier process's rows survive by design — but their blobs are unlinked anyway. Those rows now carry CacheRefs resolving to nothing, and CacheCoordinator.lookup returns them as a hit when no consumer hint is set, so the dangling ref reaches the caller rather than degrading to a miss.
The blob sweep needs to be scoped to the rows actually deleted.
2. ConditionalTask abort leaves the previous run's routing readable
packages/task-graph/src/task/ConditionalTask.ts:211 — verified
if (context.signal?.aborted) {
return undefined;
}
// Clear previous branch activation state
this.activeBranches.clear();
this.portActiveStatus = undefined;The early return precedes the clear, so after an aborted re-run getPortActiveStatus() keeps reporting which branch the previous input selected instead of falling back to the config.branches derivation. The clear belongs above the abort check, beside activeBranches.clear().
3. ConditionalTask.outputSchema decides the shape from branches[0] alone
packages/task-graph/src/task/ConditionalTask.ts:478 — verified
const hasFunctionBranches = branches.length > 0 && typeof branches[0].condition === "function";
A heterogeneous array — a serialized branch first, a function branch second, or the reverse — publishes the wrong port set. Dataflows wired off the ports the run actually writes then fail the graph's compatibility check, and RunScheduler reads a getPortActiveStatus() map whose keys don't match the declared ports.
4. xAI ignores its own effort policy
providers/xai/src/ai/common/Xai_Client.ts:124 — verified
getXaiReasoningEffort returns EFFORT_TO_XAI[model.effort] whenever model.effort is set, never consulting xaiEffortPolicy. That policy declares {supported: []} for grok-4-fast-non-reasoning and the xAI image models, so a ModelConfig carrying an inherited or defaulted effort still gets reasoning_effort attached — a parameter the provider has just declared the model does not accept. Reached from buildChatParams, the structured-generation path, and the rewriter/summary run-fns, none of which consult the policy either.
Resource retention
5. IteratorTask retains up to 64 completed subgraph clones
packages/task-graph/src/task/IteratorTask.ts:450 — verified (cap and clear points; retention behavior read, not measured)
concurrencyLimit is optional and undefined by default, so iterationGraphCap() falls back to a hard cap of 64. clearIterationGraphs() is called only at the start of the next map/reduce run, so a MapTask over a large collection keeps the 64 most recently completed TaskGraph clones — each holding its tasks and their runOutputData — alive after the map finishes. The retention is on the domain object and happens whether or not a UI is attached; it exists as a UI affordance.
Release tooling — decisions, not obviously bugs
Flagging rather than asserting: both may have been intentional. Both are on main today.
6. publish-all has no test gate
package.json:54
"publish-all": "bun run format && bun run rebuild && bun run bunset && bun run publish-login && bun run publish-workspaces"
No bun run test in the chain, so nothing the vitest suite would catch blocks a release. bunset only versions and tags.
7. overrides is down to 3 entries
package.json — verified: main currently has 3
The review described this as a regression "from 16 to 3", which is wrong — main has had it this way across the 0.3.42–0.3.44 releases. Whether the dropped pins should come back is a real question (the @codemirror/* / @lezer/* single-instance pins in particular guarded against duplicate editor instances in the web example), but restoring them moves the lockfile and should be its own change.
Refactor opportunity
8. The effort-policy module is duplicated across six providers
providers/{anthropic,deepseek,google-gemini,openai,openrouter,xai}/src/ai/common/*_EffortPolicy.ts — verified
151 lines across six files, each with the same ALL/NONE ModelEffortPolicy literals, the same modelName(model) helper reading provider_config.model_name, and the same stampEffortOptions(record, policy(...)) call in its *_ModelSearch.ts. Only the id-matching differs.
MODEL_EFFORTS and stampEffortOptions already live in @workglow/ai/worker; a makeEffortPolicy(matchers) helper there would collapse the boilerplate. As it stands, any change to the policy shape — adding a per-model default, say — has to be made six times or drift. Item 4 above is arguably the first instance of that drift.
An automated review sweep over #790 surfaced these. None are caused by that branch — it touches 26 files and none of the ones below. I verified each against
origin/mainbefore filing; where the review's framing was wrong, I've said so.Filed together because they came from one pass, but they are unrelated to each other and want separate fixes.
Correctness defects
1.
deleteRunEntriesorphans the rows it deliberately leaves behindpackages/task-graph/src/storage/FsFolderTaskOutputRepository.ts:249— verifiedIt deletes only the listed rows, then calls
deleteBlobsByPrefix(nsPrefix), which sweeps every sidecar blob under the run prefix:A run that crashes after writing private cache rows and then resumes under the same
runIdhits this:RunPrivateCacheRepo.writeSetcovers only the current process's writes, soclearRunnames just the new rows. The earlier process's rows survive by design — but their blobs are unlinked anyway. Those rows now carryCacheRefs resolving to nothing, andCacheCoordinator.lookupreturns them as a hit when no consumer hint is set, so the dangling ref reaches the caller rather than degrading to a miss.The blob sweep needs to be scoped to the rows actually deleted.
2.
ConditionalTaskabort leaves the previous run's routing readablepackages/task-graph/src/task/ConditionalTask.ts:211— verifiedThe early return precedes the clear, so after an aborted re-run
getPortActiveStatus()keeps reporting which branch the previous input selected instead of falling back to theconfig.branchesderivation. The clear belongs above the abort check, besideactiveBranches.clear().3.
ConditionalTask.outputSchemadecides the shape frombranches[0]alonepackages/task-graph/src/task/ConditionalTask.ts:478— verifiedA heterogeneous array — a serialized branch first, a function branch second, or the reverse — publishes the wrong port set. Dataflows wired off the ports the run actually writes then fail the graph's compatibility check, and
RunSchedulerreads agetPortActiveStatus()map whose keys don't match the declared ports.4. xAI ignores its own effort policy
providers/xai/src/ai/common/Xai_Client.ts:124— verifiedgetXaiReasoningEffortreturnsEFFORT_TO_XAI[model.effort]whenevermodel.effortis set, never consultingxaiEffortPolicy. That policy declares{supported: []}forgrok-4-fast-non-reasoningand the xAI image models, so aModelConfigcarrying an inherited or defaulted effort still getsreasoning_effortattached — a parameter the provider has just declared the model does not accept. Reached frombuildChatParams, the structured-generation path, and the rewriter/summary run-fns, none of which consult the policy either.Resource retention
5.
IteratorTaskretains up to 64 completed subgraph clonespackages/task-graph/src/task/IteratorTask.ts:450— verified (cap and clear points; retention behavior read, not measured)concurrencyLimitis optional and undefined by default, soiterationGraphCap()falls back to a hard cap of 64.clearIterationGraphs()is called only at the start of the next map/reduce run, so aMapTaskover a large collection keeps the 64 most recently completedTaskGraphclones — each holding its tasks and theirrunOutputData— alive after the map finishes. The retention is on the domain object and happens whether or not a UI is attached; it exists as a UI affordance.Release tooling — decisions, not obviously bugs
Flagging rather than asserting: both may have been intentional. Both are on
maintoday.6.
publish-allhas no test gatepackage.json:54No
bun run testin the chain, so nothing the vitest suite would catch blocks a release.bunsetonly versions and tags.7.
overridesis down to 3 entriespackage.json— verified:maincurrently has 3The review described this as a regression "from 16 to 3", which is wrong —
mainhas had it this way across the 0.3.42–0.3.44 releases. Whether the dropped pins should come back is a real question (the@codemirror/*/@lezer/*single-instance pins in particular guarded against duplicate editor instances in the web example), but restoring them moves the lockfile and should be its own change.Refactor opportunity
8. The effort-policy module is duplicated across six providers
providers/{anthropic,deepseek,google-gemini,openai,openrouter,xai}/src/ai/common/*_EffortPolicy.ts— verified151 lines across six files, each with the same
ALL/NONEModelEffortPolicyliterals, the samemodelName(model)helper readingprovider_config.model_name, and the samestampEffortOptions(record, policy(...))call in its*_ModelSearch.ts. Only the id-matching differs.MODEL_EFFORTSandstampEffortOptionsalready live in@workglow/ai/worker; amakeEffortPolicy(matchers)helper there would collapse the boilerplate. As it stands, any change to the policy shape — adding a per-model default, say — has to be made six times or drift. Item 4 above is arguably the first instance of that drift.