Repository navigation
fix(herdr-core): drop untyped hc call arguments that broke main's typecheck - #2693
Merged
Merged
Conversation
…echeck #2674 added hc.open<...>/hc.call<...> with result types for the untyped HcConnection; #2671, merged just before it, types call and open by the hc method name, so origin/main failed typecheck. The method name now types both calls, including terminal.attach's terminal and terminal.read's text.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 22 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
This was referenced Nov 3, 2026
Owner
Author
This was referenced Nov 4, 2026
devswha
added a commit
that referenced
this pull request
Nov 4, 2026
…2701) ## Change Based on #2565 by @Haeminway1 (co-authored); this takes its standalone server parts. - **OmO full tool output:** `transcriptToolOutput` had no OmO branch, so an OmO pane's "show the whole output" read the entire session file (`readFileSync`) and scanned it as text. It now looks only in the latest transcript window (16 MiB, the same `windowStart` the command-history reader uses), skips a record only when the window starts inside it, resets at a `context_clear`, and caps the answer at the existing 2,000,000 characters. An older call's output outside the window can be unavailable. - **Devin diagnostic:** a Devin pane with no session lock and no reported or resumed id now carries the bounded diagnostic `devin_current_session_unverified` (through `ConversationDiagnostics`, rate-limited like the Copilot ones). The reason sent to clients stays `no_session_id`, so no HTTP contract changes. #2565's rename of the wire reason was not taken: #2585 has since changed this code, and a diagnostic says the same without a new client-visible code. ## Why Reproduced on main: the three new OmO output tests fail there (no tail bound, no context-clear reset, reads past the window), and the new Devin test fails (no diagnostic). ## Validation - `bun test server/conversation.test.ts server/conversation-diagnostics.test.ts`: 203 pass, 0 fail. - New tests fail on origin/main (12a7c63): 3 OmO output tests, 1 Devin diagnostic test. - `bun run check fast`: passed (workflow syntax, generated types/themes, typecheck, build, unit tests). origin/main currently fails typecheck in `server/herdr-core/adapter.ts`; #2693's two-line fix was applied locally for this run only and is not part of this PR. - Codex (Terra) adversarial review of the OmO part: no HIGH. Fixed: a complete record starting exactly at the window boundary was skipped (new regression test). Not changed: the window start rounds up to a 64 KiB chunk like the other transcript windows; a record over 4 MiB whose shape cannot be read inside the window makes the lookup answer `null` (fails closed) instead of scanning past it. Co-authored-by: Haemin <haemin331@gmail.com>
devswha
added a commit
that referenced
this pull request
Nov 4, 2026
## Change Based on #2565 by @Haeminway1 (co-authored); this takes its terminal-snapshot part. A pane with no agent showed herdr's scrollback split into guessed chat messages, each with its own MD/TXT copy controls, in the proportional chat font, so `ls -la` columns and indentation were lost. It now shows the read as one preserved monospace snapshot with a single "Copy as plain text" control. The agent fallback's "Conversation unavailable — show terminal output" shows the raw read the same way. Command outcomes beside the snapshot keep their state chip and actions. Each chip names its command (the chips sit together under the snapshot), repeats a recorded result only when the snapshot lacks it, and never repeats the screen preview. Whitespace-only scrollback stays the empty state. Design reference: Warp and the Codex app show command output as one terminal block with one copy action rather than as chat messages; this follows that. Screenshots (before → after, 390 px and 1280 px, dark theme): | | Before | After | |---|---|---| | 390 px | <img src="https://raw.githubusercontent.com/devswha/ovis/ad8377ac51d2f0ca249c721396a33e039a9f26a9/raw-terminal-snapshot-before-390.png" width="260"> | <img src="https://raw.githubusercontent.com/devswha/ovis/ad8377ac51d2f0ca249c721396a33e039a9f26a9/raw-terminal-snapshot-after-390.png" width="260"> | | 1280 px | <img src="https://raw.githubusercontent.com/devswha/ovis/ad8377ac51d2f0ca249c721396a33e039a9f26a9/raw-terminal-snapshot-before-1280.png" width="420"> | <img src="https://raw.githubusercontent.com/devswha/ovis/ad8377ac51d2f0ca249c721396a33e039a9f26a9/raw-terminal-snapshot-after-1280.png" width="420"> | ## Validation - `bun test src/components/ChatView.scrollback.test.ts`: 8 pass; 6 of them fail on origin/main. - `bun run scripts/chat-selection-copy-regression.ts` (Chromium): passes at 1440/390 for native copy and 360/393/430 for snapshot wrapping, verbatim copy and composer clearance. - `bun run check fast`: passed (workflow syntax, generated types/themes, typecheck, build, unit tests). origin/main currently fails typecheck in `server/herdr-core/adapter.ts`; #2693's two-line fix was applied locally for this run only and is not part of this PR. - CodeRabbit's two findings on #2565 for this part are addressed (command label on status-only chips; a recorded result absent from the snapshot is shown). Codex (Terra) review: no HIGH; its whitespace-only empty-state finding is fixed. --------- Co-authored-by: Haemin <haemin331@gmail.com>
devswha
added a commit
that referenced
this pull request
Nov 5, 2026
## What
A phone can pair through the relay **without a pairing code**. Under the
QR code, **Copy a link without a code** gives a link (server id, public
key, relay; nothing secret, no expiry) to send to the phone any way at
all. The phone and the PC then show the same six digits, and the pairing
finishes only when **both** the phone says "The numbers match" **and**
the owner approves on the PC. The result is the same relay device record
and key a pairing with a code makes. QR pairing is unchanged.
Design note: `docs/relay.md`, "Pairing by comparing digits" (under *Who
gets in*), plus the new frames and error rows.
## Protocol (commit, then reveal)
Feature-negotiated: the phone offers `pair-sas` in `HELLO`, and a PC
that supports it selects it in `WELCOME`. An older phone never offers
it. With an older PC the phone stops at the update screen. Nothing
changes for either.
1. Both ends compute a **channel binding**: SHA-256 over a domain, the
PC's static key and the two Noise handshake messages (`channelBinding`
in `shared/relay-e2ee.ts`, recorded by the carrier guard; the vendored
code is untouched). A relay that answers the phone with its own key runs
two handshakes, so the two ends hold different bindings.
2. `PAIR_REQUEST` (phone): label, device key, `commitment =
H(commit-domain ‖ binding ‖ device key ‖ Np)`.
3. `PAIR_CHALLENGE` (PC): `Nc`.
4. `PAIR_REVEAL` (phone): `Np`. The PC checks the commitment in constant
time and ends the session with `auth_failed` if it fails.
5. Both show `H(digits-domain ‖ binding ‖ device key ‖ Np ‖ Nc)` mod
10^6.
6. `PAIR_CONFIRM` (phone) + owner approval (`POST /api/relay/pairing`),
both required before `AUTH_OK`.
The phone is bound to `Np` before it sees `Nc`, and the PC picks `Nc`
before it sees `Np`, so no single party, the relay included, can choose
the digits. A relay that passes the frames through unchanged is refused
at step 4. One that runs its own comparison with each end gets matching
digits once in 10^6 tries.
Limits: 90 s from `PAIR_REQUEST` (under the relay's 120 s idle close),
then `timeout` and the prompt is withdrawn. At most 3 phones wait at
once. Every comparison that ends without a device counts toward the
existing 20-failures-per-10-minutes relay pairing limit. The relay
learns nothing new: everything travels inside the E2EE channel, so it
sees only frame sizes and timing.
## UI
- PC: a notice line in every window that may approve one (driving,
direct connection, not a Portal visitor, not through the relay):
*"{name} wants to pair through the relay. Approve it only if its screen
shows 123 456"* with Approve / Decline. It is delivered as
`relay-pairings` on `/api/machines/events`.
- Phone (hosted shell): *"Does {name} show this number?"* with the
digits, **The numbers match**, then a wait for approval. A decline or
timeout shows *"{name} did not approve this phone"*, and no key is
saved.
- Follows Bluetooth numeric comparison (iOS/macOS "Does this code
match?"): the same digits on both screens, a positive confirmation on
each, nothing typed.
Screenshots (before/after, 390 + 1280, dark + light) are linked in the
lane report for the owner's review.
## Tests
- `server/relay/pair-sas.contract.test.ts` (real `RelaySession` over
real Noise channels): happy path, PC approval alone or phone
confirmation alone does not pair, feature not offered, decline, approval
of unknown or already decided ids (incl. HTTP 404/400), broken
commitment, replayed and out-of-order frames, a commitment and reveal
replayed into another connection, deadline (TestClock) withdraws the
prompt, phone leaving withdraws it, cap of 3, and **two MITM relays**: a
pass-through one is refused, and a rewriting one shows the owner
different digits.
- `src/lib/relay/channel.test.ts`: the phone's real `openRelayChannel`
against the real PC session, approve → device saved; decline → no key
saved, `pair-declined` screen.
- `shared/relay-pair-sas.test.ts`: digits and commitment properties,
frame codec (round-trip, stream 0, malformed refused).
## Validation
At head 67d56b7 (origin/main merged, including the typecheck fix
#2693):
- `bun run check fast`: all green (workflow syntax, generated
types/themes, typecheck, build, unit tests 10901 pass / 0 fail).
- Relay suites: `bun test src/lib/relay shared/relay-mux*.test.ts
shared/relay-e2ee.spike.test.ts shared/relay-pair-sas.test.ts
server/relay relay server/devices.test.ts`: 799 pass.
- Manual: a local Bun relay + `createServer` + `dist-shell` in Chromium,
with no production relay: a codeless link → compare → PC prompt (also
with Settings open: Approve is the topmost element) → match → decline,
at 390/1280 in dark and light.
- Skipped: `check integration browser` /
`scripts/relay-e2e-regression.ts` (needs the full browser lane).
- Reviews: omo + Codex Terra (xhigh); findings and resolutions are in
the PR comments.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
origin/main (12a7c63) fails
bun run typecheckwith four errors inserver/herdr-core/adapter.ts. #2671 typedHcConnection.callandopenby hc method name, generated from hc's own schema. #2674, merged right after it, added ahc.open<{ channel: number; terminal?: HcTerminal }>("terminal.attach", ...)and ahc.call<{ text: string }>("terminal.read", ...)written for the old untyped signature, where the type argument was the result type.This drops those two type arguments. The method name now types both calls:
terminal.attachgivesAttachResult(whoseterminal.rowsthe code reads) andterminal.readgivesReadResult(whosetextit reads). Nothing changes at runtime.No changelog fragment: users notice nothing.
Validation
bun run typecheck: clean (four errors on main before).HERDR_TEST_MODE=unit bun test server/herdr-core: 90 pass, 0 fail.bun run check fast: not run locally. The heavy-run slots were all held by other lanes, so the lead chose to let CI's Fast checks gate this PR.