fix: address PR review findings

Preserve terminal prompt states during replay. Tighten review guards and regression coverage for retry delays, abort cleanup, and JavaScript comment scanning.\n\nRefs PR #188 cleanup C3.

Author: elkaix

.agents/skills/agent-core-dev/service-authoring.md

@@ -164,7 +164,7 @@ What belongs here:
 - **Helper classes / functions** used only by this impl (e.g. a built-in writer, an `extractError` helper) — co-located in the same file.
 - **Top-level `registerScopedService(...)`** — one per Service the file owns; importing the impl file runs the registration.
 
-Base class: extend `Service` (from `#/_base/di/service`) when the unit needs capability calls on `this` — `provide` / `effect` / `on` / `get` / `ref` (e.g. contributing a record to a `collection` token). `Service` extends `Disposable`, so `_register` keeps working; constructor-time `provide` / `on` / `effect` calls are buffered and flushed by the kernel after construction, while `get` / `ref` throw inside the constructor (dependencies stay constructor parameters). Otherwise extend `Disposable` — both are full DI units; a service whose own members collide with the `Service` vocabulary (`name` / `state` / `config` / `get`) must stay on `Disposable` (leave a NOTE comment saying so).
+Base class: extend `Service` (from `#/_base/di/service`) when the unit needs capability calls on `this` — `provide` / `effect` / `on` / `get` / `ref` (e.g. contributing a record to a `collection` token). `Service` extends `Disposable`, so `_register` keeps working; constructor-time `provide` / `on` / `effect` calls are buffered and flushed by the kernel after construction, while `get` / `ref` throw inside the constructor (dependencies stay constructor parameters). Otherwise extend `Disposable` — both are full DI units; a service whose own members collide with the `Service` vocabulary (`name` / `state` / `config` / `get`) must stay on `Disposable`.
 
 ## Constructor conventions
 
@@ -298,7 +298,6 @@ Importing the package therefore fires every `register*` side effect, exactly as
 
 - **No comments** (orient.md): no file headers, no statement-level narration, no JSDoc; the only exception is a load-bearing lint-suppression directive.
 - **Methods and fields carry no comments by default.** Well-named identifiers and types say *what*; the code is the source of truth for *how*.
-- Write an inline comment only when the *why* is non-obvious (a hidden constraint, a subtle invariant, a workaround). One short line.
 - For unimplemented stubs, throw `NotImplementedError('feature')` rather than `throw new Error('TODO: …')` (errors.md).
 
 ## Complete minimal example

.agents/skills/agent-core-dev/test.md

@@ -21,7 +21,7 @@ Resolving by interface is what makes `registerScopedService(ISut, Sut, …)` par
 
 Pure functions, value objects, and services with **no** `@IService` dependencies may be constructed directly.
 
-The only other exception is a test that genuinely needs **two independent instances** of the same service with different dependencies (e.g. constructing two `TurnService`s with different `ILoopRunner`s). A singleton-per-container resolution cannot produce both, so `ix.createInstance(Impl)` is acceptable there — annotate it with a comment explaining why.
+The only other exception is a test that genuinely needs **two independent instances** of the same service with different dependencies (e.g. constructing two `TurnService`s with different `ILoopRunner`s). A singleton-per-container resolution cannot produce both, so `ix.createInstance(Impl)` is acceptable there — state the reason in the test name and local identifiers.
 
 ## Two harnesses
 

packages/agent-core-v2/test/agent/llmRequester/llmRequesterService.test.ts

@@ -390,17 +390,20 @@ describe('AgentLLMRequesterService infinite retry', () => {
   });
 
   it('honors the provider retry-after delay while retrying indefinitely', async () => {
+    vi.useFakeTimers();
     const calls = { value: 0 };
     const requester = createRequester(calls, new APIProviderRateLimitError('slow down', null, 1));
     const { service } = createService(requester, undefined, {
       env: { [PYTHINKER_CODE_INFINITE_RETRY_ENV]: '1' },
     });
 
-    const startedAt = Date.now();
-    await service.request();
+    const promise = service.request();
+    await vi.advanceTimersByTimeAsync(0);
+    expect(calls.value).toBe(1);
+    await vi.advanceTimersByTimeAsync(1);
+    await promise;
 
     expect(calls.value).toBe(2);
-    expect(Date.now() - startedAt).toBeLessThan(500);
   });
 
   it('stops retrying when the caller aborts during the backoff wait', async () => {

packages/agent-core-v2/test/workspace/workspaceFs/fsProcess.test.ts

@@ -83,4 +83,20 @@ describe('runCommand', () => {
     await runCommand(fakeRunner(fakeProcess()), ['echo'], { signal: controller.signal });
     expect(listenerCount(controller.signal as unknown as EventEmitter, 'abort')).toBe(0);
   });
+
+  it('removes the abort listener when stream collection fails', async () => {
+    const controller = new AbortController();
+    const proc = {
+      ...fakeProcess(),
+      stdout: new Readable({
+        read() {
+          this.destroy(new Error('stream failed'));
+        },
+      }),
+    };
+    await expect(
+      runCommand(fakeRunner(proc), ['echo'], { signal: controller.signal }),
+    ).rejects.toThrow('stream failed');
+    expect(listenerCount(controller.signal as unknown as EventEmitter, 'abort')).toBe(0);
+  });
 });

packages/agent-gateway/AGENTS.md

@@ -4,7 +4,7 @@ The Pythinker Code server, backed by the DI × Scope agent engine (`@pymodel/age
 
 ## Comment conventions
 
-No comments — no file headers, no section banners, no statement-level narration; the code is the source of truth. The only exception is JSDoc attached to exported symbols (it flows into the generated `.d.ts` and IDE hover). Lint-suppression directives (`oxlint-disable` / `eslint-disable`) are allowed where they suppress an active rule for a deliberate pattern; other tooling directives (`@ts-expect-error`, `@ts-ignore`, …) stay banned — fix the underlying type problem instead. Enforced by `scripts/check-no-comments.mjs` (part of `pnpm lint`).
+No comments — no file headers, no section banners, no statement-level narration, and no JSDoc. The only exception is a load-bearing lint-suppression directive (`oxlint-disable` / `eslint-disable`) for a deliberate pattern; other tooling directives (`@ts-expect-error`, `@ts-ignore`, …) stay banned — fix the underlying type problem instead. Enforced by `scripts/check-no-comments.mjs` (part of `pnpm lint`).
 
 ## Routes
 

packages/agent-gateway/src/services/transcript/coreEventMap.ts

@@ -1257,7 +1257,7 @@ export class AgentTranscriptProjector {
   private onPromptStarted(event: PromptStartedEvent): TranscriptOperation[] {
     const prompt = this.upsertPrompt(event.promptId, (prev) => ({
       promptId: event.promptId,
-      status: 'running',
+      status: prev !== undefined && isTerminalPromptStatus(prev.status) ? prev.status : 'running',
       userMessageId: prev?.userMessageId,
       content: prev?.content,
       createdAt: prev?.createdAt ?? new Date().toISOString(),

packages/agent-gateway/test/services/transcript.test.ts

@@ -1775,6 +1775,36 @@ describe('AgentTranscriptProjector', () => {
     });
   });
 
+  it('preserves terminal prompt status when prompt.started is replayed', () => {
+    const projector = new AgentTranscriptProjector('main');
+    const tx = new AgentTranscript('main');
+    const feed = (event: ProjectorBusEvent): void => void tx.apply(projector.map(event));
+
+    for (const status of ['completed', 'failed', 'blocked'] as const) {
+      const promptId = `p-${status}`;
+      feed(
+        ev({
+          type: 'prompt.completed',
+          promptId,
+          finishedAt: '2026-01-01T00:00:00.000Z',
+          reason: status,
+        }),
+      );
+      feed(ev({ type: 'prompt.started', promptId }));
+      expect(tx.getPrompt(promptId)?.status).toBe(status);
+    }
+
+    feed(
+      ev({
+        type: 'prompt.aborted',
+        promptId: 'p-aborted',
+        abortedAt: '2026-01-01T00:00:00.000Z',
+      }),
+    );
+    feed(ev({ type: 'prompt.started', promptId: 'p-aborted' }));
+    expect(tx.getPrompt('p-aborted')?.status).toBe('aborted');
+  });
+
   it('projects prompt.steered media content to the wire shape (no daemon ref or path leak)', () => {
     const projector = new AgentTranscriptProjector('main');
     const tx = new AgentTranscript('main');

packages/transcript/AGENTS.md

@@ -4,7 +4,7 @@ The isomorphic transcript rendering data layer — agent-granular L1 store, idem
 
 ## Comment conventions
 
-No comments — no file headers, no section banners, no statement-level narration; the code is the source of truth. The only exception is JSDoc attached to exported symbols (it flows into the generated `.d.ts` and IDE hover). Lint-suppression directives (`oxlint-disable` / `eslint-disable`) are allowed where they suppress an active rule for a deliberate pattern; other tooling directives (`@ts-expect-error`, `@ts-ignore`, …) stay banned — fix the underlying type problem instead. Enforced by `scripts/check-no-comments.mjs` (part of `pnpm lint`).
+No comments — no file headers, no section banners, no statement-level narration, and no JSDoc. The only exception is a load-bearing lint-suppression directive (`oxlint-disable` / `eslint-disable`) for a deliberate pattern; other tooling directives (`@ts-expect-error`, `@ts-ignore`, …) stay banned — fix the underlying type problem instead. Enforced by `scripts/check-no-comments.mjs` (part of `pnpm lint`).
 
 ## Cold rebuild
 

scripts/check-no-comments.mjs

@@ -99,7 +99,7 @@ for (const pkg of PACKAGES) {
         const p = path.join(d, e.name);
         if (e.isDirectory()) {
           if (e.name !== 'node_modules') stack.push(p);
-        } else if (/\.(ts|tsx|mts|mjs)$/.test(e.name)) {
+        } else if (/\.(ts|tsx|js|jsx|mts|mjs)$/.test(e.name)) {
           files.push(p);
         }
       }