fix(server): prevent ResultManager from leaking mutable Task references.

[leehpham]

Summary

Fixes #412. ResultManager was mutating this.currentTask in place on every status-update and artifact-update event, and storing the task with a shallow copy on the initial task event. The non-blocking sendMessage path in DefaultRequestHandler resolves the HTTP response promise with resultManager.getCurrentTask() — a live reference. Subsequent events then mutate the object the HTTP layer is about to serialize, so the client receives the post-execution state (e.g. failed) instead of the handoff snapshot (e.g. submitted).

This is reference aliasing, not concurrency: the single for await consumer of ExecutionEventQueue processes events strictly in order. The handed-out reference and the in-flight state machine just happen to be the same object.

Changes

• src/server/result_manager.ts — processEvent no longer mutates this.currentTask:
  • task ingest: { ...taskEvent } → structuredClone(taskEvent), so the executor's history/artifacts arrays are not aliased into ResultManager.
  • status-update: builds a new Task with the updated status (and, if a message is present, a new history array). The previous reference is no longer touched.
  • artifact-update: builds a new artifacts array; when an existing artifact is being appended to, constructs a fresh artifact object rather than mutating parts/metadata in place.
  • The in-memory and rehydration branches collapse into one shared copy-on-write code path for each event kind.
• src/server/request_handler/default_request_handler.ts — structuredClone the result of getCurrentTask() at the non-blocking response boundary. Defense-in-depth: even if anyone later reintroduces internal aliasing in ResultManager, the response handed to the caller is guaranteed to be a snapshot.

Tests

• New test/server/result_manager.spec.ts (3 tests, this class previously had no direct test coverage):
  • A reference captured via getCurrentTask() is unaffected by a subsequent status-update.
  • A reference captured via getCurrentTask() is unaffected by a subsequent artifact-update.
  • The caller-owned history/artifacts arrays passed in on the original task event are not mutated by later updates.
• New regression test in test/server/default_request_handler.spec.ts mirroring the reproduction in [Bug]: ResultManager:processEvents (second events modifies first arriving event) which leads to unexpected behavior and race conditions #412: publishes task(submitted), advances timers, publishes status-update(failed), then re-asserts the originally-returned reference still shows state: 'submitted'.

All four tests fail on the prior implementation and pass after the fix. Full suite: 413/413 passing.

Behavior notes for reviewers

• Publish-then-mutate is no longer observable. An executor that publishes a Task value and then mutates its own copy expecting ResultManager to observe the mutation will no longer see that. This pattern is anti-spec (publish should have value semantics) and I'm treating it as a behavior fix rather than a regression. No tests or sample executors depended on it.
• artifact-update rehydration path is now consistent with the in-memory path. The pre-existing rehydration branch (when no currentTask was in memory and the task was loaded from the store) only handled parts on append, silently dropping description/name/metadata merges. The consolidated copy-on-write path applies the same merge logic in both branches.
• structuredClone requirement. Already satisfied — package.json requires Node ≥ 18, and structuredClone has been a Node global since 17.

Test plan

• npm test — full suite green (413 tests)
• npx tsc --noEmit — no new type errors in changed files (pre-existing errors in src/samples/ are unrelated to this change)
• npx eslint on changed files — clean
• Manual reviewer check: confirm the regression test at test/server/default_request_handler.spec.ts reproduces the issue when reverting the fix locally

[github-actions]

🧪 Code Coverage

⬇️ Download Full Report

	Base	PR	Delta
src/server/request_handler/default_request_handler.ts	79.69%	79.73%	🟢 +0.04%
src/server/result_manager.ts	61.7%	77.98%	🟢 +16.28%
Total	81.12%	81.61%	🟢 +0.49%

Generated by coverage-comment.yml

[gemini-code-assist]

Code Review

This pull request introduces a copy-on-write strategy and utilizes structuredClone within the ResultManager and DefaultRequestHandler to ensure that task snapshots remain immutable once they are handed to callers. These changes prevent background updates from mutating previously returned task references. The PR also adds comprehensive unit tests to verify this isolation. The review feedback identifies additional opportunities to strengthen this isolation by deep-cloning incoming status and artifact update events to prevent internal state from aliasing external objects that might still be modified by the executor.

[gemini-code-assist]

To fully isolate the ResultManager state from subsequent mutations by the executor and to avoid a potential race condition during the await at line 58, consider deep-cloning the updateEvent. Currently, next.status (line 68) aliases the status object from the event, meaning any later mutation of that object by the executor would be reflected in ResultManager's internal state and subsequent snapshots.

Suggested change

	const updateEvent = event as TaskStatusUpdateEvent;
	const updateEvent = structuredClone(event as TaskStatusUpdateEvent);

References

1. Use structuredClone for deep copying objects instead of JSON.parse(JSON.stringify()) to ensure that undefined fields are preserved.

[gemini-code-assist]

Similar to the status update, the artifactEvent should be deep-cloned to ensure full isolation. Currently, the artifact objects are aliased into the artifacts array (lines 104 and 117), making the internal state susceptible to later mutations by the executor and race conditions during the await at line 89.

Suggested change

	const artifactEvent = event as TaskArtifactUpdateEvent;
	const artifactEvent = structuredClone(event as TaskArtifactUpdateEvent);

References

1. Use structuredClone for deep copying objects instead of JSON.parse(JSON.stringify()) to ensure that undefined fields are preserved.