Skip to content

feat(task): observation registry and the Task run-state fields (split 2 of #1914, #1375) - #2384

Open
easonLiangWorldedtech wants to merge 1 commit into
Zoo-Code-Org:mainfrom
easonLiangWorldedtech:fws/u9-1914-cut2-observation-registry-task
Open

easonLiangWorldedtech wants to merge 1 commit into
Zoo-Code-Org:mainfrom
easonLiangWorldedtech:fws/u9-1914-cut2-observation-registry-task

Conversation

@easonLiangWorldedtech

Copy link
Copy Markdown
Contributor

What this is

Unit 2 of the five-way split of #1914, planned and issued on #2020. Whole-file module cut, 5 files, no partial hunks: src/core/task/observationRegistry.ts with its spec, the src/core/task/Task.ts changes that expose the run-state fields its consumers read, and the two Task spec files. It is an independent pull request rather than a stack - nothing in it depends on any other unit, and it compiles on this main tip alone.

Half of this unit is content a reviewer may already have read. observationRegistry.ts is blob b3b7a719871d, byte-identical to what #1913, #2357 and #2359 already carry, and it accounts for 20 of this unit 25 counted executable lines. Task.ts is blob 77a1333b39e0, which is #1914 own blob and one of 8 distinct versions of that file across this series. If #1913 lands first, the production content here reduces to the 5 counted executable lines in Task.ts, because the registry blob would already be on main; the spec file travels with it regardless, tests being uncounted. That is the honest price of this unit, stated up front rather than discovered at merge time.

Numbers, with the base named

  • Base: main tip e487cd7b2a497aa1d9b60f9b0cb497f7839bb629, which is this commit own parent (git merge-base --is-ancestor exits 0 against it, so the gate merge-base and the base coincide).

  • Counted executable lines: 25 against the 500 limit, headroom 475. Per file: observationRegistry.ts 20 and Task.ts 5. The three __tests__ files count for nothing, because packageForPath returns no package for test paths.

  • Added and deleted lines, beside that number and not instead of it: 349 (349 added, 0 deleted) over 5 files.

  • zdt split measure against this base printed the following, quoted exactly as measured, with the exit code exactly as measured:

    standalone e487cd7b2a497aa1d9b60f9b0cb497f7839bb629..3ce2d0522b279aabd9b272613cebb97302e78243: 349+ / 0- = 349 a+d, 5 files  -> UNDER-SOFT
    

    exit code 0. This unit is under both added-deleted caps and under the executable-line cap measured standalone against its own base; the pull request it came out of is not, which is why it was split.

  • Typecheck: tsc --noEmit -p src/tsconfig.pcmap.json from the verification worktree root (node v22.23.4): exit 0, 0 error lines.

  • Tests, run from the same worktree src over this unit three spec files with vitest.core.config.ts: it printed Test Files 3 passed (3) and Tests 241 passed (241), 0 failed and 0 skipped, and exited 0. Named rather than left silent: this unit lane reports 0 skipped, and the six test.skipIf(process.platform === "win32") skips in this split chain live in src/utils/__tests__/safeWriteJson.test.ts, which belongs to held unit cut5.

  • One advisory worth knowing before reviewing: the local preflight printed mutation exclusions must use "// Stryker disable next-line MutatorName: concrete reason" against src/core/task/Task.ts:4454, which is existing content this unit inherits. It is an advisory rather than a blocking finding, and this unit adds no Stryker directive of its own - no directive was added anywhere in this split.

Gate numbers and what they mean

Two different questions get two numbers, and the base is named for both. Counted executable lines come from the repository own buildManifest in scripts/stryker-diff.mjs, run through .tmp-cr-audit/u8-measure-chain.mjs (node v22.23.4), with per-path diffs at --unified=0 because parseChangedLines marks every line a hunk covers and selectFromGit passes --unified=0 at scripts/stryker-diff.mjs:306. Added-and-deleted lines are the plain git diff --shortstat figure, printed beside the counted number and not instead of it.

The base named below is the current main tip, which is also this commit own parent. git merge-base --is-ancestor exits 0 against it, so the CI gate merge-base computation and my base coincide exactly and no two-dot approximation is involved in these figures.

Mutation gate: local attempt, and an honest deferral

A local zdt mutation preflight was attempted at this base. The wrapper printed preflight exit code: 0 and => PASS (local gate green) while writing no reports (mutation testing not applicable or run incomplete) and a digest with an empty package list, and the wrapped Stryker log says why: it instrumented the changed files and then died with Cannot find module 'vitest/config' from its .stryker-tmp sandbox copy of vitest.config.ts, whose require stack resolves vite out of the parent clone .pnpm store rather than through this worktree junctioned node_modules. A green verdict with an empty report set is not a green verdict, so this pull request carries no local mutation reading. CI mutation-diff will be the first real one.

That deferral is safe for two measured reasons rather than evasive ones. First, #1914 mutation-diff has never produced a mutant verdict for this content at all: it fails on the 1,044-versus-500 width gate before the runner reports, so there is no mutation history being inherited here. Second, if a surviving mutant does appear, the answer is test hardening, which lands in __tests__ paths and therefore costs zero counted executable lines, because packageForPath returns nothing for test paths. A mutation-red cannot move this unit further from mergeable than it already is.

Content fidelity

Every file here is #1914 verbatim content, taken from the merge commit that closes it (7209a5f1681b387e397434172707cb028e33ec3b, tree 3c919730a813) with no edits. The five whole-file units of this split are path-disjoint and their file sets sum to the 26 files #1914 changes. Applied onto this main tip they produce tree 83cdd9cb0223, and git diff --name-only 83cdd9cb0223 7209a5f1681b387e397434172707cb028e33ec3b prints 14 files - every one of them newer content that arrived on main after the merge base 5319fe91fb0caf4aa4ec66aac68fa03744ebbc23, none of them among the 26. The split therefore neither loses nor invents content, and each unit measures the same counted executable lines against the merge base and against this tip.

Series housekeeping

This split was planned and issued on #2020 before any split pull request opened (comments 6738707507, 6739312036, 6739376708, 6739428842, 6739719219), and the version question for the contested files is asked there in comment 6739877102. #1914 stays open until the units of its split have been reviewed, because its review history is the only record of what has already been said about this content; its 14 open review rows were answered on the pull request itself. On #1914 there is no review event from anyone except the bot and the author, and no non-empty review body from anyone except the bot (140 review events over 2 pages, 81 from the bot and 59 from the author account, 0 from anyone else).

No .changeset file and no CHANGELOG edit, per AGENTS.md. src/eslint-suppressions.json is not touched here: the 96-to-94 count change for core/tools/__tests__/readFileTool.spec.ts belongs to held unit cut4 and to that unit alone, so no two pull requests in this split edit that file. knip.json is likewise not touched here - held unit cut3 owns its one entry, and the fact that #1914 and #2359 both carry blob 34450fc441a3 while main, #1911, #1912, #1913, #1917 and #2368 carry 1f4956faa6cc is a known conflict cost of the series, recorded on #2020, not a mistake of this unit.

@coderabbitai

coderabbitai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • Improvements

    • Task activity now keeps file observations separate between tasks and clears them when a task ends.
    • Cancellation state is tracked consistently across abort requests and task disposal.
  • Tests

    • Added coverage for task isolation, observation cleanup, and cancellation state changes.

Task now owns an isolated ObservationRegistry for file-version observations and a cancellation generation counter. Abort requests and disposal advance the counter. Disposal closes the registry and clears its observations.

Changes: Task State

Layer / File(s) Summary
Observation registry contract
src/core/task/observationRegistry.ts, src/core/task/__tests__/observationRegistry.spec.ts
Adds an in-memory registry keyed by absolute path. Observations include a version, timestamp, and completeness flag. Tests cover replacement, lookup, clearing, and closure behavior.
Task ownership and lifecycle
src/core/task/Task.ts, src/core/task/__tests__/Task.dispose.test.ts, src/core/task/__tests__/Task.spec.ts
Each Task receives its own registry. Abort requests and disposal advance cancellationGeneration; disposal closes the registry. Tests cover registry isolation, disposal, and generation changes.

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Feature

Possibly related issues

Possibly related PRs

Merge Risk: Minimal · up to 3ce2d

The change adds task-local observation storage and cancellation state without an established behavioral regression. It is mergeable subject to normal checks.

Pre-merge checks: Passed 8

Check name Status Explanation
Title check Passed The title names the main change: a new observation registry plus Task run-state fields. It also names the series position (split 2 of #1914, #1375). The title is clear and specific. It uses a conventi…
Description check Passed The description follows the repository template sections: Related GitHub Issue, Description, Test Procedure, and Pre-Submission Checklist. It gives a clear account of the unit scope, the changed files…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Regression Evidence Passed The changed registry and task lifecycle behavior have focused unit coverage. Tests cover observation replacement, completeness, isolation, closure, cancellation generation, resume, and disposal. The P…
Security Boundaries Passed The changed production code adds an in-memory observation registry and task lifecycle counters. It does not add a path that leaks secrets or PII, executes unvalidated input, or bypasses approval or al…
Persistence Integrity Passed The production changes add in-memory task state and do not change a persistence path. The only changed file operation removes a fixed test fixture before a test.
Lifecycle Resource Cleanup Passed No changed lifecycle path shows a concrete resource leak or duplicate work. Task.disposeOnce() closes and clears the new registry before its first await. The new cancellation counter has no producti…
Full details: Title check

Explanation

The title names the main change: a new observation registry plus Task run-state fields. It also names the series position (split 2 of #1914, #1375). The title is clear and specific. It uses a conventional commit prefix and fits the 13-word, 88-character length guidance, which is slightly long. The title does not use Simplified Technical English style cleanly because of the parenthetical references, but the content match is strong.

Full details: Description check

Explanation

The description follows the repository template sections: Related GitHub Issue, Description, Test Procedure, and Pre-Submission Checklist. It gives a clear account of the unit scope, the changed files, the counted executable lines, the typecheck result, and the Vitest result. The description is long and heavy with commit hashes and gate figures. It has no explicit 'Closes: #' issue link in the template format; it references #2020 as the planning issue and #1914 as the parent PR, but the template asks for an approved issue number in 'Closes:' form. The Test Procedure section gives commands but not reviewer repeat steps in plain form. The Pre-Submission Checklist is not ticked, and the Visual Snapshots, Videos, Documentation Updates, and Get in Touch sections are not addressed. The Simplified Technical English guidance is not followed closely: many sentences are long and use contractions such as 'It is an independent pull request' is fine, but phrases like 'the honest price' and 'nothing in it depends' vary style. These gaps are mostly non-critical, so the description is mostly complete.

Resolution

Optional improvements: add a 'Closes: #...' line that names the approved issue, tick the checklist items that apply, and state briefly that the Visual Snapshots, Videos, and Documentation Updates sections do not apply. Shorten long sentences to follow the Simplified Technical English guidance.

Full details: Regression Evidence

Explanation

The changed registry and task lifecycle behavior have focused unit coverage. Tests cover observation replacement, completeness, isolation, closure, cancellation generation, resume, and disposal. The PR does not change visible UI.

Full details: Security Boundaries

Explanation

The changed production code adds an in-memory observation registry and task lifecycle counters. It does not add a path that leaks secrets or PII, executes unvalidated input, or bypasses approval or allowlist controls. The new registry and counter have no production consumers beyond Task.ts in this PR.

Full details: Lifecycle Resource Cleanup

Explanation

No changed lifecycle path shows a concrete resource leak or duplicate work. Task.disposeOnce() closes and clears the new registry before its first await. The new cancellation counter has no production consumers in this PR.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between e487cd7 and 3ce2d05.

Files selected for processing: 5

  • src/core/task/Task.ts
  • src/core/task/__tests__/Task.dispose.test.ts
  • src/core/task/__tests__/Task.spec.ts
  • src/core/task/__tests__/observationRegistry.spec.ts
  • src/core/task/observationRegistry.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 2 remain after this review.

Recent review details

Context from checks skipped due to timeout: 2

  • GitHub Check: CodeRabbit
  • GitHub Check: extension-host-visual
Additional context used

Path-based instructions: 5

Check persistence and lifecycle invariants: awaited atomic writes, rollback or explicit partial-failure behavior, cross-window state consistency, stale listeners/watchers, cancellation, idempotency, and safe restart/resume without lost or d...

CodeRabbit configuration file

Files:

  • src/core/task/__tests__/Task.dispose.test.ts
  • src/core/task/__tests__/observationRegistry.spec.ts
  • src/core/task/observationRegistry.ts
  • src/core/task/__tests__/Task.spec.ts
  • src/core/task/Task.ts
Require regression coverage at the lowest valid harness with behavior-focused assertions, including relevant negative, error, false/unset, and boundary cases.

CodeRabbit configuration file

Files:

  • src/core/task/__tests__/Task.dispose.test.ts
  • src/core/task/__tests__/observationRegistry.spec.ts
  • src/core/task/__tests__/Task.spec.ts
Check strict typing and exhaustive behavior across normal, boundary, error, cancellation, retry, and compatibility paths.

CodeRabbit configuration file

Files:

  • src/core/task/__tests__/Task.dispose.test.ts
  • src/core/task/__tests__/observationRegistry.spec.ts
  • src/core/task/observationRegistry.ts
  • src/core/task/__tests__/Task.spec.ts
  • src/core/task/Task.ts
Verify extension/webview contracts, cancellation and error propagation, VS Code lifecycle correctness, and behavior under retries and partial failure.

CodeRabbit configuration file

Files:

  • src/core/task/__tests__/Task.dispose.test.ts
  • src/core/task/__tests__/observationRegistry.spec.ts
  • src/core/task/observationRegistry.ts
  • src/core/task/__tests__/Task.spec.ts
  • src/core/task/Task.ts
Act as an adversarial second-opinion reviewer.

CodeRabbit configuration file

Files:

  • src/core/task/__tests__/Task.dispose.test.ts
  • src/core/task/__tests__/observationRegistry.spec.ts
  • src/core/task/observationRegistry.ts
  • src/core/task/__tests__/Task.spec.ts
  • src/core/task/Task.ts
Learnings: 1

Learning: 2026-10-20T22:54:54.079Z

Learnt from: CR
Repo: Zoo-Code-Org/Zoo-Code PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-10-20T22:54:54.079Z
Learning: Source excerpt:
# AGENTS.md

## ESLint Suppressions

When writing new code:

- Avoid floating promises; add `void`, `await`, or `.catch()` as appropriate.

Applied to files:

  • src/core/task/__tests__/Task.spec.ts

@github-actions

github-actions Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Review status

Thanks for contributing. This comment tracks the review sequence and the next action.

Current step: Awaiting fresh human maintainer or CODEOWNER approval.

Automated review is complete for the latest commit but does not replace human approval.

Review-state labels are managed by this workflow; do not edit them manually. community-approved is managed the same way — do not add or remove it manually. It signals a fresh community code approval for the current head as an advisory priority only; maintainer review is still required.

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Changed-code mutation testing

Report for PR head 3ce2d0522b27.

Surviving / uncovered mutants Advisories
0 1
Details: mutants, guidance, and reproduction

Changed-code mutation testing

PackageChanged executable linesValidKilledTimeoutSurvivedNo coverageResult
extension251111000Passed

Focused tests

  • extension (cwd src): core/task/__tests__/ask-auto-deny.spec.ts, core/webview/__tests__/ClineProvider.spec.ts, core/webview/__tests__/ClineProvider.taskHistory.spec.ts, core/task-persistence/__tests__/TaskHistoryStore.spec.ts, core/task/__tests__/ask-queued-message-drain.spec.ts, __tests__/extension.spec.ts, core/task/__tests__/Task.spec.ts, core/tools/__tests__/executeCommand.spec.ts, core/task/__tests__/Task.persistence.spec.ts, activate/__tests__/registerCommands.spec.ts, core/tools/__tests__/executeCommandTool.spec.ts, core/webview/__tests__/ClineProvider.sticky-mode.spec.ts, core/webview/__tests__/ClineProvider.sticky-profile.spec.ts, core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts, __tests__/ClineProvider.history-resume-delegation.spec.ts, core/task/__tests__/Task.dispose.test.ts, core/task/__tests__/ask-clear-approval-buttons.spec.ts, core/task/__tests__/ask-allowlist-cwd.spec.ts, core/task/__tests__/Task.streaming-tool-calls.spec.ts, core/webview/__tests__/ClineProvider.stale-view.spec.ts, __tests__/single-open-invariant.spec.ts, core/task/__tests__/grace-retry-errors.spec.ts, core/task/__tests__/flushPendingToolResultsToHistory.spec.ts, core/webview/__tests__/ClineProvider.lockApiConfig.spec.ts, core/task/__tests__/Task.throttle.test.ts, core/task/__tests__/Task.run-shadow.spec.ts, core/assistant-message/__tests__/presentAssistantMessage-custom-tool.spec.ts, core/assistant-message/__tests__/presentAssistantMessage-tool-usage-attribution.spec.ts, __tests__/ClineProvider.delegation.spec.ts, core/task/__tests__/reasoning-preservation.test.ts, core/webview/__tests__/checkpointRestoreHandler.spec.ts, core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts, core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts, core/task/__tests__/grounding-sources.test.ts, activate/__tests__/registerCodeActions.spec.ts, __tests__/ClineProvider.subtask-links.spec.ts, services/code-index/__tests__/code-index-status-manager.spec.ts, core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts, core/task/__tests__/Task.ignored-disabled-tools-notice.spec.ts, extension/__tests__/api-task-conversation-history-length.spec.ts, core/task/__tests__/Task.abort-reason-race.spec.ts, core/assistant-message/__tests__/presentAssistantMessage-auto-deny.spec.ts, core/task/__tests__/Task.resume-eviction-race.spec.ts, __tests__/removeClineFromStack-delegation.spec.ts, extension/__tests__/api-start-task-logging.spec.ts, activate/__tests__/handleUri.spec.ts, core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts, core/assistant-message/__tests__/presentAssistantMessage-tool-repetition.spec.ts, extension/__tests__/api-configuration.spec.ts, core/task/__tests__/Task.pending-action.spec.ts, extension/__tests__/api-start-new-task.spec.ts, extension/__tests__/api-has-task-ui-message-text.spec.ts, activate/__tests__/handleTask.spec.ts, __tests__/abandonSubtask.spec.ts, core/task/__tests__/ask-abort-race.spec.ts, core/assistant-message/__tests__/presentAssistantMessage.spec.ts, extension/__tests__/api-send-message.spec.ts, __tests__/nested-delegation-resume.spec.ts, core/webview/__tests__/ClineProvider.pending-action.spec.ts, core/webview/__tests__/ClineProviderFactory.spec.ts, core/task/__tests__/observationRegistry.spec.ts, core/task/__tests__/ToolExecutionContext.spec.ts, extension/__tests__/api-delete-queued-message.spec.ts, extension/__tests__/api-delegation-events.spec.ts, core/task/__tests__/Task.sticky-profile-race.spec.ts, __tests__/task-run-dispatch.spec.ts, services/code-index/__tests__/code-index-scope.spec.ts, __tests__/api-subtask.spec.ts, extension/__tests__/api-theme-fixture.spec.ts, __tests__/new-task-delegation.spec.ts, core/task/__tests__/Task.terminal-operation.spec.ts, test-utils/__tests__/provider.spec.ts, extension/__tests__/api-terminal-profile.spec.ts, core/assistant-message/__tests__/toTelemetryToolName.spec.ts

Reproduce locally

From a full checkout containing both commits:

pnpm install --frozen-lockfile
node scripts/stryker-diff.mjs ci --base e487cd7b2a497aa1d9b60f9b0cb497f7839bb629 --head f122498368e464db1635b8b0163f10d1ea871b8b

Mutation reports

  • extension: reports/mutation/extension/mutation.html

The workflow uploads generated reports in the changed-code-mutation-report artifact. A direct artifact link appears below after upload.

Advisory findings

  • src/core/task/Task.ts:4454: mutation exclusions must use "// Stryker disable next-line MutatorName: concrete reason"; broad or unreasoned exclusions are not allowed.

@codecov

codecov Bot commented Nov 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions github-actions Bot added coderabbit-review-active Required CI passed; CodeRabbit review is active awaiting-coderabbit Waiting for CodeRabbit to approve the latest commit labels Nov 9, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-maintainer CodeRabbit approved; waiting for a human maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant