Uh oh!
There was an error while loading. Please reload this page.
🤖 feat: scroll-fixed terminal badge overlay (iTerm2-style workspace/tab watermark) - #3937
Conversation
ibetitsmike
commented
Aug 23, 2026
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:d8ce8d0630
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
ibetitsmike
commented
Aug 23, 2026
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:50b4907458
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
ibetitsmike
commented
Aug 23, 2026
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:e67545a630
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
ibetitsmike
commented
Aug 23, 2026
@codex review Round-3 findings handled in 136413f: badge config is now reset across story boundaries (both reset helpers + unmount restore). The keybind finding is answered inline: palette-only access matches the peer toggle command precedent in sources.ts; a dedicated global chord for a default-off decorative overlay would be disproportionate. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:136413fb4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
ibetitsmike
commented
Aug 23, 2026
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This comment has been minimized.
This comment has been minimized.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
iTerm2-style badge for issue #3607: per-workspace/per-tab identity that stays fixed while terminal output scrolls. Template supports {workspace}, {tab}, {project}; corner position presets, opacity, font size; default off.
UAT found interactive shells overwrite the OSC title every prompt, so
{tab} alone can't distinguish same-cwd tabs; {index} is the stable
1-based tab position (empty in pop-out windows).- number terminal badges/labels across the whole sidebar layout, not per tabset, so split panes don't both render 'Terminal'/#1 - let the badge template settings row wrap and the input shrink at narrow widths - fall back to the workspace ID in pop-out badges when workspace resolution fails
Storybook shares one browser origin, so a badge story's enabled config would leak into later stories and make snapshots order-dependent.
…op-outs - GeneralSection badge config subscribes with listener: true so the command palette toggle updates a mounted settings page - pop-out handoff passes the last known OSC title through openWindow/query params so the badge doesn't reset to 'Terminal' until the shell emits another title
026d896 to
433a223CompareThere was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:433a22328c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // "Terminal"/#1 in every split, defeating the badge's per-tab identity. | ||
| const terminalTabOrder = new Map<TabType, number>(); | ||
| collectAllTabs(layout.root) | ||
| .filter(isTerminalTab) |
There was a problem hiding this comment.
When a single-tab tabset is split, splitFocusedTabset deliberately creates a bare "terminal" placeholder, and isTerminalTab returns true for that placeholder even though it has no session. This filter therefore counts it in the global ordering; adding the first real terminal to that split gives it {index}=2 and the fallback label Terminal 2. Fresh evidence beyond the prior numbering fix is that the layout utilities actively create these sessionless placeholders, so build the order only from tabs for which getTerminalSessionId(tab) is defined.
Useful? React with 👍 / 👎.
Uh oh!
There was an error while loading. Please reload this page.
The five terminal-badge handlers added by #3937 each repeated the same `setTerminalBadgeConfig((prev) => ({ ...normalizeTerminalBadgeConfig(prev), <field> }))` dance. Funnel them through one patchTerminalBadgeConfig helper so the normalize-then-spread step is stated once and cannot drift between fields.
…ration oversubscription) (coder#3939) ## Summary Fixes the three mechanisms that have been kicking fully validated PRs out of the merge queue: a unit-job Bun segfault caused by a desynced test-file exclusion list, integration-job cancellation caused by the job timeout colliding with jest's own test timeout, and integration test flakes caused by jest worker memory oversubscription on CI runners. ## Background Recent merge-queue evidence (all on green PRs whose diffs never touched the failing areas): 1. **Test / Unit segfault** (mq runs 32666269009, 32667158072, both kicked PR coder#3937): every test passes, then Bun 1.3.5 panics with `Segmentation fault` (exit 132). The logs show the crash is not teardown noise: it happens mid-`SandboxHostService` suite inside the monolithic coverage run. `src/node/services/sandbox/sandboxHostService.test.ts` was added to the `isolated_unit_tests` list in coder#3865 (QuickJS-heavy suites are known to crash Bun under coverage in a shared process) but was never added to the hand-maintained duplicate `find` exclusion list, so the suite ran a **second time** inside the shared process, where it segfaults intermittently. 2. **Test / Integration cancelled at the job cap** (mq runs 32632804824, 32639270365, 32640298486, kicking PRs coder#3931/coder#3930): the check-run annotation reads "The job has exceeded the maximum execution time of 10m0s". Healthy runs finish in ~4-5 min, but jest's integration `testTimeout` is 10 minutes (`jest.config.js`), so any single hung test makes jest wait right through the 10-minute job cap; the runner then kills the job and destroys all diagnostics. 3. **Recurring integration flakes** (`focus.test.ts` in run 32715740782 kicking PR coder#3932, `sendModeDropdown.test.ts` in run 32666269009, `anthropicCacheStrategy.test.ts` in run 32665306160, plus `analyticsEscape.test.ts` at 66s of its 60s budget in run 32719185534 kicking PR coder#3932's attempt 5, plus earlier `undo.test.ts`/`analyticsHeader.test.ts` kicks): all share one mechanism. CI runs jest with `--maxWorkers=100%`: 16 forked workers on the 16-core/64GB runner at ~4.7GiB peak each (per `scripts/lib/worker_budget.js`) oversubscribes memory (~75GiB peak) and thrashes. In the failing logs, suites that normally finish in seconds *pass* at 100-135s wall time while the only tests that *fail* are the ones with tight explicit timeouts (30s/45s), timing out in setup. The `--maxWorkers=100%` flag predates the repo's memory-aware worker budget (coder#1326 vs coder#3760), so CI never got the sizing fix that local runs already use. ## Implementation - **Unit job**: derive the `find` exclusions from the `isolated_unit_tests` array instead of maintaining a duplicate list, so the two can never drift again. This removes the double-run of the crashing suite (and fixes the whole desync class rather than one instance). No exit-code masking anywhere: a genuinely failing test still fails the job exactly as before. - **Integration job**: dropped the CLI `--maxWorkers` override so `jest.config.js`'s memory- and cgroup-aware `workerBudgetFor("jest")` sizes the pool (per Codex review; CLI flags take precedence over config), and `timeout-minutes: 10` → `20` so a hung test hits jest's 10-minute `testTimeout` and produces a real failure with logs instead of a log-destroying job cancellation. - **Worker recycling**: the first PR CI run under the bounded budget surfaced the complementary failure mode: integration suites leak memory across test files, so with fewer workers each one accumulates heap until it dies at V8's ~4GB cap (“Jest worker ran out of memory and crashed”, run 32720490232). `workerIdleMemoryLimit: "2GB"` recycles leaky workers between files. - **Tightest observed-killed test timeouts**: `focus.test.ts` 30s → 120s (aligned with its sibling `undo.test.ts`) `anthropicCacheStrategy.test.ts` 45s → 120s (peer provider tests already budget 45-150s per live call), and `analyticsEscape.test.ts` 60s → 120s after it died at 66s in run 32719185534. Other flagged tests already use the harness's 30s windows and should recover via the worker fix; they were left untouched to avoid churn. Live-API tests (e.g. `anthropicCacheStrategy`) are deliberately **not** excluded from `merge_group`: the mq run is the last gate that executes tests for the merged combination (main-push runs skip test jobs for the merge-queue bot), so quarantining them there would remove real coverage. The single observed kick was timeout-tuning, not provider outage. ## Validation - Red-green proof of the file-selection change: reproduced the old `find` invocation and the new derived one side by side; the old list contains `sandboxHostService.test.ts` (the double-run), the new list differs by exactly that one file (759 → 758, no other adds/removes). - `TEST_INTEGRATION=1 bun x jest tests/ui/review/focus.test.ts tests/ipc/providers/anthropicCacheStrategy.test.ts` passes locally (2/2, including the live Anthropic call). - `make lint-actions` (actionlint + zizmor) and `make static-check` pass. ## Risks CI-config only plus two test-timeout constants; no production code. The main tradeoff is failure-detection latency: a genuinely hung integration run is now killed at 20 min instead of 10 (but now with jest diagnostics at the 10-minute testTimeout), and the two touched tests take up to 120s before reporting a real hang. Suite wall time at 8 workers may shift slightly in either direction (less thrash, fewer lanes); healthy runs have ample headroom against the 20-minute cap either way. --- _Generated with `xum` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- xum-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
The five terminal-badge handlers added by #3937 each repeated the same `setTerminalBadgeConfig((prev) => ({ ...normalizeTerminalBadgeConfig(prev), <field> }))` dance. Funnel them through one patchTerminalBadgeConfig helper so the normalize-then-spread step is stated once and cannot drift between fields.
Fixes#3607
Summary
Adds an iTerm2-style "Terminal Badge": a configurable, scroll-fixed watermark overlay above every ghostty-web terminal surface (right-sidebar tabs and pop-out windows) that shows templated workspace/tab identity. Default OFF.
Background
When working across multiple workspaces and multiple terminal tabs, nothing inside the terminal viewport identifies which workspace/tab you are in: prompt markers scroll away, the sidebar tab label is not visible while scrolling output, and ghostty's
background-imageis global. The badge is a DOM overlay (not scrollback content), so it stays fixed regardless of output volume or scroll position.Implementation
TerminalBadgeOverlayrenders as a sibling of the ghostty container inside the already-position: relative.terminal-view, withpointer-events: none+user-select: noneso clicks, drags, and selection pass through to the canvas. Color comes fromvar(--color-terminal-fg); opacity/position/size from config.terminalBadgeConfig) follows theterminalFontConfigpattern end to end: storage key + normalizer insrc/common/constants/storage.ts, zod schema inuserPreferences.ts, preference sync inuserPreferencesStorage.ts, backup projection inpayload.ts.{workspace},{tab},{project},{index}.{tab}mirrors the sidebar tab label (OSC 0/1/2 title, falling back to "Terminal N");{index}is the stable 1-based tab position because interactive shells overwrite the OSC title on every prompt (dogfood UAT finding). Unknown{tokens}pass through so typos stay visible.api.workspace.list()document-title path and track the OSC title locally ({index}is empty there: the sidebar tab order is unknown to a pop-out).Validation
make static-checkgreen; targeted suites pass (badge template, storage normalizer, TerminalView badge rendering with mocked ghostty-web, commands sources, GeneralSection, preference storage, backup payload).elementFromPointhit-testing returning the terminal canvas under the badge, live restyle without reload, per-tab OSC title isolation, pop-out rendering, and 375px truncation. The{index}token addresses the UAT finding that OSC titles make same-cwd tabs indistinguishable.tests/ui/storybook/budget.test.ts(local-only, not in CI) was already failing on clean main (378 estimated snapshots vs cap 305, 88 files vs cap 79); this PR adds 1 story file / 4 snapshots on top of that pre-existing drift.Risks
Low. The overlay renders
nullunless explicitly enabled, so default behavior is unchanged. The riskiest touchpoint isTerminalView(shared by all terminal surfaces); changes there are additive props, a state-captured workspace-name resolution on the existing pop-out fetch, and an OSC-title state update that piggybacks on the existingonTitleChangesubscription.Generated with
xum• Model:anthropic:claude-fable-5• Thinking:xhigh