Skip to content

fix(chat): follow-ups from the Copilot refresh-retention review (#2068) #2112

Closed as not planned

Description

@devswha

Follow-up to #2068 (LOW findings from the merge pass's reviews; none blocked the merge).

  • Peer claims on the success path: a successful Copilot conversation read reuses the peer claims gathered at the start of the request (server/conversation.ts, success-path copilotTranscriptPath(..., resolved.peerClaims)). A peer that claims the same session during the read is seen only by the next request.
  • Liveness is not identity in the failed-read recheck: the recheck fails closed when a cached peer Copilot process has exited (processAlive). It does not catch pid reuse within one herdr call, a zombie peer, a new Copilot in a peer pane that had none cached, or a process that changes identity without exiting. Comparing the process start time (as process-start.ts can) would close most of these.
  • Unit coverage: no unit test covers a failed read with a live peer that still retains history (503); only the contract test does.
  • Persistent unexpected read error: a read error that never clears keeps returning 503, so Chat keeps the old messages indefinitely instead of reaching the terminal fallback.
  • Comment wording: server/conversation.ts says "never reuse peer claims here" just above code that reuses cached peer process info; "never reuse the request's earlier peer claims" says what is meant.
  • Test coverage of the 409 late-response case: the 409 variant of the late-response deadline test passes on the base too, so it guards against a future regression but does not show this fix.

Activity

  1. devswha commented on Oct 28, 2026

    @devswha
    OwnerAuthor

    Closing under the new follow-up bar: a follow-up issue is kept only for a reproduced, user-visible MEDIUM finding. The remaining notes are LOW: peer-ownership races inferred from code with no observed reproduction, plus test-coverage notes. Below the follow-up bar: LOW/unreproduced; recorded on PR #2068 and in this issue for reference.

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