Skip to content

wait readers: make monitorId required or soften rpc.md, and cover WakeSourceTracker precedence (follow-up to #4228) #4335

Open

Description

@code-yeongyu

Follow-up from the review of #4228 at 51f5fa8 (verdict CLEAR, two LOWs). Not blocking.

  1. docs/rpc.md:156 says terminal monitors are named by their stable mon_ id, but extension-state.ts:88 falls back to the runtime id when entry.monitorId is missing. The field is optional on the snapshot (monitor-types.ts:59), even though every current source sets it (command-monitor-source.ts:99, file-monitor-record.ts:80). Either make monitorId required on the snapshot type or soften the doc sentence.
  2. test/suite/regressions/3864-named-wait-readers.test.ts:84-103: the harness part of the third test goes through the binding, which already preferred items ?? monitors before feat(coding-agent): preserve named waits on the session lifecycle #4228. Only the direct wakeSourceItems assertion covers the tracker's precedence change. Add one WakeSourceTracker.observe assertion so a regression in the tracker fails a test.

Acceptance: (1) the doc and the type agree; (2) a test fails if WakeSourceTracker stops preferring items over monitors.

Related: #3864, #4228.

Activity

  1. code-yeongyu commented on Oct 26, 2026

    @code-yeongyu
    OwnerAuthor

    Two more LOWs from the review of #4231 at 107d39a (verdict CLEAR):

    1. test/suite/extension-edit-assistant-message.test.ts:82,88 asserts a live snapshot with persisted: true and humanInputRevision: 2 while the newest durable record holds revision 1. A crash between /noop and the next input restores revision 1 and reuses revision 2. Add one line to rpc.md saying persisted does not cover a deferred admission revision.
    2. While a custody write is failing, recompute() calls custodyWriteFailed(), which sets persisted=false, so every non-activity recompute appends a record (session-lifecycle.ts:261, lifecycle-ledger.ts:240). Limit those appends while custody is failing; the feat(coding-agent): expose live running activity in the session lifecycle #4231 fold only covered activity-only changes.
  2. code-yeongyu commented on Oct 26, 2026

    @code-yeongyu
    OwnerAuthor

    LOW from the delta review of #4243 at 25acb02 (verdict CLEAR):

    1. In 3864-tui-lifecycle-ownership.test.ts, the new reset test checks that the extension status is gone with a bare substring of the rendered footer (not.toContain("mine")). Assert that my-ext is absent from footerDataProvider.getExtensionStatuses() instead, so both sides of the test are exact.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions