Skip to content

fix(server): share one pty when clients attach the same pane concurrently - #2

Merged
devswha merged 1 commit into
mainfrom
fix/concurrent-attach
Sep 19, 2026
Merged

fix(server): share one pty when clients attach the same pane concurrently#2
devswha merged 1 commit into
mainfrom
fix/concurrent-attach

Conversation

@devswha

@devswha devswha commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Problem

ensureAttachment checked the attachment map, then awaited the terminal lookup (session.snapshot) before storing the new record. When two attaches to the same pane landed in that window, each one spawned its own herdr terminal attach. That can happen when a PC and a phone reconnect together after a server restart.

  • The later record replaced the earlier one in the map, and the earlier pty was orphaned.
  • The orphan's onData still looked the pane up by id, so its output went into the surviving attachment.
  • One client went silent (no pty-data, no pane-geometry), and the other got output from two ptys.
  • When the orphan exited, closeAttachment(paneId) closed the surviving attachment too. stop() never killed the orphan.

Fix

  • A concurrent attach now joins the in-flight creation through a pendingAttachments map. The entry is cleared in finally, so a failed lookup (pane_not_found) doesn't leave the pane stuck.
  • The creation body moved into spawnAttachment unchanged. attachments.set still runs synchronously right before new PtySession(...).
  • Mixed roles work as before: whichever attach joins second takes its own branch after the shared creation. An interact client resizes; an observer receives pane-geometry.

Test

WebSocket concurrent attach runs on its own herdr-web-ui-test-race workspace, so it doesn't depend on another test's pane. It sends two attaches in the same tick, waits for pty-data on both sockets, then checks that a resize from one reaches both.

Check Result
Mutation: server/index.ts fix reverted new test fails 3/3 (second hears resize ×2, first pty-data ×1)
bun test server/api.contract.test.ts ×4 22 pass / 0 fail each run
bun test 63 pass / 0 fail, 9 files
bun run typecheck / bun run build clean
Leftovers after runs no herdr terminal attach / pty-host.mjs processes, no herdr-web-ui-test* workspaces

The README test count (63) matches the suite again. The coverage sentence now lists concurrent attaches.

🤖 Generated with Claude Code

@devswha @claude
fix(server): share one pty when clients attach the same pane concurre…
77fdd28 
…ntly

ensureAttachment checked the attachment map, then awaited the terminal
lookup before storing the new record. Two attaches to the same pane in
that window each spawned their own `herdr terminal attach`. The later
record replaced the earlier one in the map, but the orphaned pty's onData
still looked the pane up by id, so it streamed into the surviving
attachment: one client went silent, the other got output from two ptys,
and the orphan's exit closed the surviving attachment.

A concurrent attach now joins the in-flight creation through a pending
map that is cleared on success and failure alike. A new contract test
races two attaches on a fresh pane and checks a resize reaches both
clients; with the fix reverted it fails 3/3.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@devswha
devswha merged commit 7f4c4a3 into main Sep 19, 2026
@devswha
devswha deleted the fix/concurrent-attach branch September 19, 2026 05:09
@devswha devswha mentioned this pull request Sep 19, 2026
Merged
devswha added a commit to nahwan-kim/herdr-web-ui that referenced this pull request Sep 29, 2026
@devswha @claude
fix(usage): keep accounts apart across sources, switches and 429s
7939ba3 
Review fixes (CodeRabbit, and an adversarial review with Codex):
- gh auth status asks only github.com, so enabling usage no longer
  contacts Enterprise hosts with their credentials or waits on them.
- A GH_TOKEN/GITHUB_TOKEN sign-in is read with the plain token lookup;
  the per-user lookup has no stored token for it.
- Copilot tokens of no known login are separate sign-ins, so one that
  answers first no longer hides the others.
- A place that now holds another account's failing sign-in no longer
  shows the previous account's name, plan and numbers.
- A 429 waits per account (or per private token fingerprint), so a
  sign-in moved or copied elsewhere does not ask again early.
- Sign-ins of one provider that name no account are numbered (devswha#1, devswha#2),
  and a fallback key hashes where the sign-in was found instead of
  sending its path to the browser.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@devswha devswha mentioned this pull request Oct 22, 2026
Merged
devswha added a commit that referenced this pull request Oct 22, 2026
@devswha
refactor(i18n): move terminal-only strings into src/lib/i18n/terminal…
601eb3a 
….*.ts (#1674)

Second PR of the i18n split (#2 in the split plan, after the Settings
pilot #1673). The strings that only the terminal's files use move out of
the three base dictionaries into `src/lib/i18n/terminal.{ko,ja,zh}.ts`.
`t()`, the English keys, every translation, placeholders, the English
fallback and all call sites stay the same.

## What changes

- **Moved:** 113 keys in each language, byte for byte, in the order each
language's file had them. A key moves only when every file that uses it
is a terminal file: `components/PaneTerminal.tsx`,
`components/PaneCanvas.tsx`, `components/KeyBar.tsx` (with its `ARROWS`
and `KEY_SPOKEN_NAMES` label maps), `components/FindBar.tsx`,
`components/TerminalViewportTools.tsx`,
`components/SplitDropPreview.tsx`, `components/TabStrip.tsx`,
`lib/terminalLabel.ts` and `lib/tabName.ts`. Keys these files share with
another feature stay in the base files.
- **Composition:** each base file gains one import and one `KO_PARTS` /
`JA_PARTS` / `ZH_PARTS` entry (`terminal: TERMINAL_KO`, …). Nothing else
in the base files changes.
- No test, docs or CONTRIBUTING change: the pilot's tests already cover
every part, and CONTRIBUTING already says where a feature's strings go.

## Key list

I regenerated the list with the split plan's ownership script on this
branch's base, `a717621f`. That script collects keys the same way as
`i18n.test.ts`: `t()`/`tt()` literals plus the 13 label maps, recorded
per file. It finds 113 terminal keys, identical to the plan's list from
`8b5aaba2`, with no key added or removed. Since then main gained only
#1672 (no i18n change) and the pilot #1673 (which moved only
Settings-only keys).

## Checks

- **Same dictionaries:** the composed `KO`, `JA` and `ZH` (every key and
value) dumped as sorted JSON on `a717621f` and on this branch are
byte-identical (`EQUIVALENT`).
- **`--color-moved` summary:** `git diff --color-moved=plain
origin/main...HEAD` over the base and part files shows all 339 removed
entry lines (113 × 3) as moved. The only non-moved lines are 33 added
lines, 11 per language:
  - the import;
  - the parts entry;
- the part file's 7-line header, its `export const` line and its closing
`};`.

  No line is removed other than moved entries.
- **Same keys moved:** the keys of `TERMINAL_KO`, `TERMINAL_JA` and
`TERMINAL_ZH` each equal the 113-key list (`SAME-KEYS` for ko, ja and
zh). The scan still finds all 1,389 keys, the same count as main.
- **Part sizes:** each part file is 122 lines, under the 400-line cap.
`i18n.ko.ts` goes from 1,180 to 1,069 lines, `i18n.ja.ts` from 1,182 to
1,071 and `i18n.zh.ts` from 1,184 to 1,073.
- `HERDR_TEST_MODE=unit bun test src/lib/i18n.test.ts`: 21 pass, 0 fail.
This covers missing, unused, placeholder and untranslated entries, a key
in two parts, and different placement across languages.
- `bun run typecheck`: passes.
- `bun run check fast`: passes (unit 6127 pass, 0 fail, 12 skip;
workflow syntax, generated types/themes, typecheck and build ok).

This PR contains only moved lines, one import and one parts entry per
language, and the part file header. It should qualify for the move-only
merge path if the parts entry and header are counted as part of the
move.

## Note

After this move the `// ---- key bar ----` section comment in each base
file has no entries left: its keys went to Settings (#1673) and here.
The comment is left in place to keep this diff move-only; the last split
PR can drop empty section comments.

No changelog fragment: users see no change.
This was referenced Oct 27, 2026
Merged
Merged
devswha added a commit that referenced this pull request Oct 28, 2026
@devswha @sisyphus-dev-ai
perf(check): reduce native typecheck GC CPU with caller-overridable d…
4b07d0c 
…efaults (#2276)

## Change

Default native TypeScript 7 checks to `GOGC=400`, retaining garbage
collection
while spending modestly more RAM to reduce CPU. Typecheck's Go 1.26.4
`tsc`
was the #2 CPU consumer on the shared P620 CI host.

The original proposal was `GOGC=off GOMEMLIMIT=6GiB`. The maintainer's
concurrency
steer requires sizing for 24 simultaneous typechecks with approximately
40 GB free;
the measurements below rule out a GC-off limit with the requested 1.5x
headroom,
so this PR uses the explicitly requested `GOGC=400` fallback instead.

- `package.json` routes `typecheck` through a small Bun wrapper. Both
CI's
`check fast`/`check concurrent` and the remote-bundle jobs already
invoke
`bun run typecheck`; neither `scripts/check.ts` nor any workflow needs
editing.
- The wrapper retains the existing native compiler launcher, both
tsconfig
invocations, sequential execution, inherited stdio, and failure exit
codes.
- Caller `GOGC` wins, including an explicitly empty value. `GOMEMLIMIT`
is
never assigned or removed by the wrapper, so caller limits remain in
force.
- The wrapper passes a child environment and argument array, with no
POSIX
environment-assignment syntax or hard-coded Linux compiler path. This
preserves
  TypeScript's platform package selection on Linux, macOS, and Windows.
- Seven subprocess regression cases cover both compiler environments,
default
and independent/combined/empty overrides, argument forwarding, inherited
  unrelated environment, and stopping on either compiler's failure.
- The explicit tools tsconfig now includes the wrapper and its tests so
the normal
  typecheck validates both new files.
- Includes an English maintenance changelog fragment. No workflow or
dependency
  changes.

## Concurrency sizing

Direct GNU `/usr/bin/time -v` measurements of
`node_modules/@typescript/typescript-linux-x64/lib/tsc`, **not Bun's
RSS**.
Three runs per project and setting, each through `dori heavy ram-gogc-pr
-- ...`;
the table reports the maximum of those three runs. Both projects run
sequentially.
Baseline and unlimited-off measurements explicitly remove `GOMEMLIMIT`.

| Native compiler setting | Main peak RSS MiB | Tools peak RSS MiB |
| --- | ---: | ---: |
| `GOGC=100`, no limit | 1205.29 | 606.29 |
| `GOGC=off`, no limit | 1400.33 | 695.16 |
| `GOGC=off GOMEMLIMIT=6GiB` | 1399.34 | 698.23 |
| **Chosen: `GOGC=400`, no limit** | **1381.34** | **684.29** |

Separate baseline `GODEBUG=gctrace=1` traces show largest post-GC live
heaps
(`X->Y->Z MB`, the `Z`) of **752 MiB main / 394 MiB tools**. These are
observed
live heaps, not a universal upper bound.

The no-GC main peak times 1.5 is 2100.50 MiB, rounded up to **2112MiB**.
That limit alone times 24 is **49.5 GiB**, before RSS overhead,
exceeding both
the approximately 30 GB target and approximately 40 GB free. Even
observed
unlimited-off RSS times 24 is 32.82 GiB / 35.24 decimal GB without
growth headroom.
There is no qualifying GC-off limit under the requested sizing rule.

Baseline direct RSS times 24 is **28.25 GiB / 30.33 decimal GB**. The
chosen
direct peak times 24 is **32.38 GiB / 34.76 decimal GB**. The larger
observed
full-command high-water RSS gives 32.49 GiB / 34.89 decimal GB.
The decision-relevant increment for these direct peaks is **176.05 MiB
per
compiler**, or **4.13 GiB / 4.43 decimal GB at 24**: the existing
baseline memory
is already consumed without this change. In the prior seven-round
CI-image
experiment below, the median increment is **157.00 MiB per compiler**,
or
**3.68 GiB / 3.95 decimal GB at 24**.
The fallback is below approximately 40 GB in this measurement but **does
not
meet the preferred approximately 30 GB envelope**; it leaves about 5 GB
for other
memory. `GOGC=400` retains collection, not an RSS cap or a guarantee
that 24
runners plus their overhead fit. `GOMEMLIMIT` remains available to the
caller.

Twenty-four simultaneous typechecks are a conservative planning case,
not the
usual workload: `CHECK_HOST_MAX_RUNS=4` bounds the herdr-backed lanes,
whereas
typecheck runs in Fast checks on up to 20 runners plus any applicable
lanes.
That lane bound is not a global semaphore for every Fast-check compiler.

## Performance evidence

### Prior CI-image experiment: seven rotating rounds

Source: `/home/devswha/.cache/ram-offload-tmp/gogc/results.md`, snapshot
`adfdef6e49767ec09323b9b2e1f0f151c6b84394`, Bun 1.4.2, TypeScript 7.0.2
/
Go 1.26.4, `herdr-ci-runner:local` on the 128-CPU P620. Medians:

| Setting | CPU user+sys s | Wall s | Max RSS MiB |
| --- | ---: | ---: | ---: |
| Default | 52.31 | 6.47 | 1188.54 |
| **Chosen fallback: `GOGC=400`** | **38.74 (-25.94%)** | **6.14
(-5.10%)** | **1345.54 (+157.00)** |
| Rejected: `GOGC=off GOMEMLIMIT=6GiB` | 23.49 (-55.09%) | 4.76
(-26.43%) | 1390.78 (+202.25) |

Every one of the seven GOGC=400 runs used less CPU than the baseline
median;
its worst was 47.16 CPU-s versus 52.31 CPU-s.

### This worktree: five alternating baseline/chosen comparisons

Rebased onto `792dbb98be8a2cd88b1123930a6aae8b131ebe23`; Bun 1.4.3,
native
TypeScript 7.0.2 / Go 1.26.4. GNU time wraps the real `bun run
typecheck`.
Baseline supplies `GOGC=100`; chosen leaves it unset and exercises the
wrapper
default. Both remove `GOMEMLIMIT`. Odd rounds run baseline then chosen,
even
rounds reverse that order. All runs use `dori heavy ram-gogc-pr -- ...`.

| Round | Baseline CPU s | Chosen CPU s | Baseline wall s | Chosen wall
s | Baseline RSS MiB | Chosen RSS MiB |
| --- | ---: | ---: | ---: | ---: | ---: | ---: |
| 1 | 59.58 | 43.57 | 6.34 | 7.37 | 1191.29 | 1310.29 |
| 2 | 48.88 | 36.36 | 8.39 | 7.83 | 1174.24 | 1382.29 |
| 3 | 66.91 | 42.98 | 7.38 | 6.25 | 1132.29 | 1386.23 |
| 4 | 58.33 | 41.24 | 6.30 | 6.23 | 1132.29 | 1195.29 |
| 5 | 64.05 | 61.80 | 9.07 | 10.97 | 1194.29 | 1191.29 |
| **Median** | **59.58** | **42.98 (-27.86%)** | **7.38** | **7.37
(-0.14%)** | **1174.24** | **1310.29 (+136.05)** |

Every chosen run beats its paired baseline on CPU; four of five also
beat the
baseline median. Round 5's 61.80 CPU-s exceeds the 59.58 CPU-s baseline
median
and is retained. Wall-time improvement is negligible on this busy shared
host.
Full-command RSS is GNU time's process-tree high-water statistic, not a
sum of
simultaneously resident processes.

For the originally requested chronological three-run before/after
comparison:
the unchanged script was measured before tracked edits, so no stash was
needed.
Before CPU was 35.10 / 36.17 / 28.64 s, wall 4.32 / 4.95 / 5.94 s, and
RSS 1202.23 / 1206.29 / 1207.29 MiB. The first three after runs are
rounds 1-3
above. Their medians are **35.10 -> 42.98 CPU-s, 4.95 -> 7.37 wall-s,
1206.29 -> 1382.29 MiB**. That chronology-only comparison is slower;
shared-host
conditions differed between blocks. The alternating comparison above is
the
contemporaneous evidence for the win. No result was discarded.

Full-command `GODEBUG=gctrace=1` counts: **24 before, 6 after** (3 per
compiler).
The 0-2 expectation was for GC off; the selected fallback deliberately
retains GC.

Raw logs, scripts, and the complete local report are retained at
`/home/devswha/.cache/ram-offload-tmp/pr-tmp/` (`sizing-results.md`,
`baseline-*.time`, `size-*.time`, `compare-*.time`, and `*.trace`).
The table above uses `final/compare-*.time` and
`final/size-chosen-*.time`.
An earlier five-round comparison before adding the module marker and
tools
typechecking coverage is retained too: medians 55.60 -> 39.74 CPU-s
(-28.53%),
7.03 -> 6.74 wall-s, 1193.21 -> 1250.16 MiB. The final tree was
remeasured rather
than substituting those earlier results.

## Validation

- `dori heavy ram-gogc-pr -- bun test ./scripts/typecheck.test.ts`: 7
pass, 0 fail.
- Native TS7 checks the new wrapper and tests through
`tsconfig.tools.json`.
- Required local `dori heavy ram-gogc-pr -- bun run check fast`: **exit
0**;
**8202 pass, 0 fail, 33 skip in 530 files**. Workflow syntax, both
generated-file
freshness checks, both TypeScript projects, build, and unit tests all
passed.
  Report `e39ee9c2-d32d-4758-89a6-7432c7db8945` has `status=success`,
`verification.status=verified`, `source.unchanged=true`, and no errors;
  log: `check-fast-final.log`.
- Final diff reviewed locally. An independent gate reviewer could not be
started
  because this task is already at the harness's maximum subagent depth.
- Native macOS and Windows runs were not executed locally; portability
follows
the existing launcher and Bun child-environment API, without shell
assignments.
- No live runner service/containers, panes, other worktrees, sysctl,
swap, or sudo
operations. Temporary evidence uses the requested `TMPDIR`; no shared
cleanup.

Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
This was referenced Nov 3, 2026
Merged
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@devswha