Skip to content

refactor(server): drop the concatBytes imports the signature change orphaned - #224

Merged
chrischall merged 1 commit into
mainfrom
claude/drop-dead-imports
Aug 6, 2026
Merged

refactor(server): drop the concatBytes imports the signature change orphaned#224
chrischall merged 1 commit into
mainfrom
claude/drop-dead-imports

Conversation

@chrischall

Copy link
Copy Markdown
Owner

The one nit from #223, on a fresh branch — #222 merged while I was fixing it.

Six integration fixtures (mutual-auth-idb, graphql-query, read-cookies, all-bootstrap-verbs, two-mcps, reconnect) and peer.ts still imported concatBytes after their signature payloads moved to readySignaturePayload(). Checked each rather than trusting the list: all seven had zero remaining call sites.

host.ts keeps its import — it still concatenates for the peer hello signature (mcpId || sessionNonce), which is a different payload and unaffected by #222.

Worth removing beyond tidiness in one case: in peer.ts the import was the last trace of the payload shape that #222 established is a security bug, sitting a few lines from the code that no longer builds it.

1233 tests, typecheck clean across all six workspaces.

Closes #223

…rphaned

#223. Six integration fixtures and `peer.ts` still imported `concatBytes` after
their signature payloads moved to `readySignaturePayload()`. Dead either way,
and in `peer.ts` actively misleading: the import was the last trace of a
payload shape that is now a security bug.

`host.ts` keeps its import — it still concatenates for the PEER hello
signature, which is a different payload and unaffected.

Closes #223

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chrischall chrischall added the refactor Code refactoring (no functional change) label Aug 6, 2026
@github-actions github-actions Bot added the auto-review Trigger Claude + Copilot review on this PR label Aug 6, 2026
@claude

claude Bot commented Aug 6, 2026

Copy link
Copy Markdown

Claude finished @chrischall's task in 56s —— View job


Review: Remove unused concatBytes imports

Task List

  • Read CLAUDE.md and understand project conventions
  • Review PR diff to identify all changes
  • Verify concatBytes usage is actually removed in changed files
  • Verify concatBytes is still used in host.ts (as PR claims)
  • Verify context and payload correctness
  • Post review findings

Analysis

This PR removes concatBytes imports from 7 files (1 source + 6 integration test files) after PR #222 refactored the ready-signature payload to use readySignaturePayload().

Verification performed:

  1. Grep for remaining usage in modified files:

    grep -n "concatBytes" packages/server/src/peer.ts packages/server/tests/integration/*.test.ts
    

    Result: No matches — confirms all 7 imports removed had zero call sites.

  2. Confirmed host.ts still uses concatBytes:

    grep -n "concatBytes" packages/server/src/host.ts
    

    Result: Line 6 (import), line 302 (usage in peer hello signature). ✅ Correct — different payload.

  3. Payload context verified:

    • Removed from: ready-signature payload (now readySignaturePayload(mcpHelloNonce || extHelloNonce || extensionSessionPub))
    • Still used in host.ts:302: peer hello signature (mcpId || sessionNonce)
    • Still used in build-server-hello.ts:63: MCP server hello signature (mcpId || sessionNonce)

    These are intentionally different payloads — no cross-contamination.

  4. Adherence to conventions:

Findings

No issues found. The removals are correct, targeted, and well-scoped. The PR author correctly identified that the imports were dead code after the signature-payload refactor and preserved the legitimate uses in host.ts and build-server-hello.ts.

Verdict: pass


@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

✅ Auto-review verdict: pass — All seven concatBytes imports removed had zero call sites after PR #222's refactor. Legitimate uses in host.ts and build-server-hello.ts were correctly preserved.

@chrischall chrischall added the ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green label Aug 6, 2026
@chrischall
chrischall enabled auto-merge (squash) August 6, 2026 00:20
@chrischall
chrischall merged commit 4985ba7 into main Aug 6, 2026
16 checks passed
@chrischall
chrischall deleted the claude/drop-dead-imports branch August 6, 2026 00:21
chrischall added a commit that referenced this pull request Aug 6, 2026
🤖 I have created a release *beep* *boop*
---


##
[2.0.0](v1.11.0...v2.0.0)
(2026-08-06)


### ⚠ BREAKING CHANGES

* **protocol:** bind the ephemeral key into the ready signature
([#222](#222))

### Features

* **protocol:** add write_cookies, the one verb that can repair a
rotated session
([#211](#211))
([b2557c2](b2557c2))
* **protocol:** bind the ephemeral key into the ready signature
([#222](#222))
([c13aeed](c13aeed))
* **server:** let a request name the tab that relays it
([#207](#207))
([c5d3f4d](c5d3f4d))
* **server:** pin the extension's identity, and verify it on the peer
path ([#213](#213))
([0eeced7](0eeced7))


### Bug Fixes

* **cli:** let a real filesystem error be itself, not "no extension pin"
([#221](#221))
([c87a864](c87a864)),
closes [#220](#220)
* **cli:** validate --via-tab before connecting, like the request URL
([#210](#210))
([959fcc5](959fcc5))
* **extension:** reattach the write_cookies doc block, and name the
writable cookies as writable
([#215](#215))
([2730c4a](2730c4a))
* **extension:** use the guarded caps local for the cookie heading
([#217](#217))
([f95c832](f95c832))
* **server:** release only our own extension claim, and stop guessing
scoped names
([#219](#219))
([3d90a64](3d90a64)),
closes [#218](#218)
* **server:** type no-tab rejections so they stop reading as version
mismatches ([#205](#205))
([dc30bd9](dc30bd9))


### Refactor

* **server:** drop the concatBytes imports the signature change orphaned
([#224](#224))
([4985ba7](4985ba7)),
closes [#223](#223)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-review Trigger Claude + Copilot review on this PR ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green refactor Code refactoring (no functional change)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auto-review follow-ups for PR #222: feat(protocol)!: bind the ephemeral key into the ready signature

1 participant