Repository navigation
fix(work-panel): keep the active tab's close button inside the strip - #1769
Merged
Merged
Conversation
Contributor
Author
|
Note for the reviewer on the checks: if That merge added a bare Electron step to |
vastsa
requested changes
Oct 20, 2026
vastsa
left a comment
vastsa
left a comment
Owner
There was a problem hiding this comment.
Thanks for the focused fix. The bug is real: the reported measurements match the current structure—the close control is a 24px sibling, while the reveal effect scrolls only the tab button. Scrolling the wrapper is the right direction.
This head does not cover the full reported path yet:
- The effect still depends only on
activeTabIdandtabs.length. If the panel is narrowed while the same tab remains active, neither dependency changes, so React does not rerun the effect and the close button can remain clipped after resize. - The issue also expects a hovered tab's close button to be reachable. A partially visible inactive tab can expose a clipped close button on hover, but this effect only reveals the active tab.
- The new test checks source text and CSS declarations; it does not prove the close button is inside the strip viewport or receives pointer hits after narrowing/resizing.
Please cover resize and hover reveal, and add a renderer-level regression check for the actual geometry/hit target. Also, this batch is limited to fixes without visible UI behavior changes, so this PR stays out of scope for this batch even after the code gap is addressed. Thanks for investigating it.
The AskTool card kept its question index and draft answers in component state, so switching sessions unmounted the card and returning to the same pending ask reset every answer. Store the draft by requestId instead and let the card read and write that entry, dropping it when the ask is resolved, dismissed, or removed. Amends the asktool card spec's request-transition rule and adds E2E-ASKTOOL-draft-survives-session-switch.
The transcript guard only dropped empty messages whose status was error or aborted. An empty complete assistant message therefore reached the outbox on every provider retry, growing the request body and feeding the EMPTY loop. Drop empty assistant rows whatever their status, while keeping messages that carry an error payload, a tool row, or a steering reservation. Adds empty-assistant-persistence tests for each of those cases.
An isolated session's files live in its worktree, but chat references can still carry a path from the parent checkout: project instructions, MCP tools, or a reference captured before isolation. The resolver rejected that path as outside its roots, and the renderer then handed the file view a relative path that it resolved against the parent checkout, so the click either failed or opened the main-branch copy. Rebase an absolute parent-checkout reference onto the same relative path in the worktree when that file exists, and keep the worktree's absolute path in the renderer so the file view opens the branch copy. Paths that exist only in the parent still report outside-allowed-roots. fixes vastsa#1747
Follow mode can still read pinned when a send or retry hid the pane. On the next reveal the scroll hook then bottomed the transcript before the reading-position hook could restore the recorded bookmark, so a reader who had scrolled up returned to the newest turn. Route the reveal through one shared ownership rule: an unpinned bookmark or a pending restore keeps the edge, a genuinely pinned pane bottoms, and an unpinned pane without a bookmark reapplies its retained offset. Both hooks now agree on which one acts. fixes vastsa#1771
The file index keeps only the shallowest 8000 entries, so in a large workspace — a Unity project, where every asset has a .meta sibling — a script several folders down matches no query at all, even though the truncation note invites the reader to refine it (vastsa#1781). While the snapshot is truncated, a non-empty query is now also searched over the workspace's full listing through the read-only fs/search channel, after a short typing pause, and the best rows merge with the index's own matches. The listing is the git list the index already builds, or a deeper ignore-set walk when the index came from the walk. A complete index never searches, and a remote session keeps its snapshot because the remote file protocol has no search operation. fixes vastsa#1781
muzimu217
force-pushed
the
fix/work-panel-tab-close-visible
branch
from
October 20, 2026 16:41
78d5748 to
ed76271
Compare
The close button is a sibling of the tab button inside the tab wrapper, but scroll-into-view targeted only the button. With `inline: "nearest"` a button that is already visible scrolls zero pixels, so the sibling close button stayed clipped outside the strip's overflow edge — the tab looked like it had no close control at all (issue vastsa#1762). Scroll the wrapper instead (falling back to the button), and reserve the close button's width at the inline end via `scroll-padding-inline` so sub-pixel rounding cannot reintroduce the clip.
vastsa#1782: when discovery returns no registration endpoint, the login flow fell back to a literal client id: } else { clientId = "pi-desktop"; } No authorization server has ever registered that id, so the app opened `<authorize>?client_id=pi-desktop` and the user landed on a bare 404 — with no explanation and no way forward. It is not a GitHub-specific failure: any OAuth MCP server without Dynamic Client Registration is affected. Throw before opening anything, naming what is missing and the two real ways out. A settings field for a client id does not exist yet, so the message does not promise one. The test drives the real startLogin through a mocked discovery that omits registration_endpoint, and asserts both the message and that openExternal was never called. Verified against the old code: the test does not merely fail, it hangs until the 300s timeout, because the previous flow waited for a callback that could never arrive — the same experience the report describes. Verification: mcp-oauth.test.mjs 20/20, mcp-oauth-discovery.test.mjs pass, tsc --noEmit clean.
muzimu217
force-pushed
the
fix/work-panel-tab-close-visible
branch
from
October 20, 2026 17:04
ed76271 to
399dd26
Compare
A failed catalog query used to drop the list back to built-in picks, discarding what the user was just looking at (issue vastsa#1692 follow-up). Both markets now keep the last successful result for the same query on screen — under the failure with a timestamped note, and during the retry that follows — until a retry succeeds or the query changes. The snapshot never answers a different query, an empty successful answer is not a failure to cover, and Load more refreshes it.
…astsa#1785) * feat(plugins): offer one line of composer suggestion from a plugin A plugin that reacts to a finished turn often knows what the user plausibly types next, but every surface it has today interrupts: a toast, a notification, or a rewrite of a draft the user already wrote. Add `pi.composer.suggest` and `pi.composer.clearSuggestion`, gated by the new `composer.suggest` permission, which put one dismissible line into the placeholder layer of the Composer for the session on screen. The decision is split along the ownership the host already has. The main process refuses a session that is not the one on screen and a turn that a newer turn overtook, because it owns both facts; the renderer refuses a draft that is not empty, because only it owns the draft, and answers the waiting caller. A plugin therefore learns `shown: false` with a reason instead of guessing, which is what lets it stop spending completions on a user who is not looking. Main never writes renderer state: it sends a message, like `pi.ui.revealView` does. Nothing a plugin offers becomes draft text on its own. `Tab` writes the line into the draft with the caret at the end, `Esc` drops it, and typing, sending, a session switch, a new turn, a plugin stop, a reload or a window close all end it; the `@` / `/` completion list keeps `Tab` and the first `Escape`. See ADR 0332. * refactor(plugins): keep only the facts the suggestion slot reads The suggestion bridge kept the composer handle inside its live-draft value, but it already reaches the newest handle through its own registry. Drop the duplicate so the value describes only what the decision needs: the session, the text and how many references the draft holds. * fix(plugins): give the suggestion slot a server snapshot React requires a third snapshot when a component is rendered to a string, and the app-shell snapshot fixtures render the Composer that way. Without it React threw "Missing getServerSnapshot" and the three shell-render tests failed on CI even though the browser path was fine. A suggestion is a live-window fact, so the server snapshot is simply "none". --------- Co-authored-by: v_wyxgwang <v_wyxgwang@tencent.com>
Keep each assistant turn behind one process disclosure while allowing shell calls to remain independent or join their surrounding activity group. Read the presentation preference during rendering so cached turns update immediately, and preserve disclosure choices across parent toggles. Add regression coverage for settings persistence, outcome states, visibility, alignment, hit targets, and rendered scroll stability.
vastsa#1751's Electron E2E job ran 30m18s and was cancelled by the job budget with no failing step to read. The cause is this runner's only timeout: const timer = setTimeout(() => child.kill(), 60_000); SIGTERM is a request. An Electron child still initialising its renderer can ignore it, and the 'close' event this script awaits then never arrives, so the step holds the job until GitHub cancels it. The runner's own 60s budget is never enforced. Escalate: SIGTERM at 60s, SIGKILL at 65s, and report which one fired. A probe that cannot start now fails this step in about a minute with its output attached, instead of burning the job budget. Verified: normal path still passes ("provider client certificate E2E passed"); with a child that ignores SIGTERM the escalation force-kills it at ~800ms in an isolated harness, so the timeout path terminates rather than hanging.
Long sessions accumulate one system-transcript snapshot per instruction change. Unless a model declares mid-conversation system support, every adapter folds them into a single leading message, but the output-cap estimate charged each superseded snapshot again. The clamp collapsed to 1 and the provider floor (16) went on the wire while the request was far from the window, so reasoning models were cut off and the turn ended as EMPTY_MODEL_RESPONSE without compaction ever triggering. The parent and subagent stream paths now fold the transcript the way the adapter will before clamping, and hand that same transcript to pi-ai so its own streamSimple clamp agrees. Context calibration and the compaction budget keep their existing basis.
Add a persisted ranscriptDisplayMode setting with flat, command-first, and fully-folded layouts. Fully-folded keeps the v0.20.0 command grouping, command-first keeps run rows standalone, and flat removes nested activity groups while preserving one outer process. Make Thinking rows default expanded in flat and command-first, collapse to a whole-run TextShimmer indicator, and use the same shimmer for the hidden reasoning live indicator. Keep fully-folded's existing Thinking defaults. Deprecate keepCommandsExpanded; it remains readable for compatibility. Cover mode segmentation, settings normalization and sync, and renderer wiring; update transcript and settings specs and ADRs.
Open the completed process and Thinking disclosure before asserting the retained reasoning text now that a collapsed Thinking row shows only its label. Also assert flat mode keeps one outer process without nested activity groups or standalone run wrappers.
… set
The sidecar merges the operating system's trust store into Node's default CA
set at startup. Those stores are a cache and keep entries that already
expired: Windows still lists Let's Encrypt's `ISRG Root X2` and `R3` with a
2025-09-15 expiry long after the fact.
Merging one of those makes OpenSSL accept an expired certificate as the anchor
for any chain that names it as issuer. A chain like
leaf -> YE2 -> Root YE -> ISRG Root X2 -> ISRG Root X1
stopped at the expired `ISRG Root X2` instead of walking up to the valid
`ISRG Root X1`, so the handshake failed with `CERT_HAS_EXPIRED` even though
the provider's own chain was valid. The failure is reported as a TLS category
error and is correctly not retried, so the provider is unusable until the
expired entry leaves the store.
Every provider behind such a chain failed, which is why the report looked
provider-specific (issue vastsa#1393). Reproduced on Windows 11 with Electron 43.6.0:
`tls.getCACertificates("system")` returned 146 certificates, 17 of them
expired, and the same host handshakes fine with the bundled roots alone.
An anchor that has already expired can verify nothing, so it is never merged.
The bundled and `NODE_EXTRA_CA_CERTS` sets are untouched, and chain,
expiration and hostname validation stay enabled - this narrows the merge, it
never disables verification.
- `system-ca.ts`: filter the system store by `validTo` before merging.
- `system-ca.test.ts`: assert no expired system anchor reaches the defaults.
- `e2e-provider-certificates.mjs`: the trust probe now measures the usable
system roots, because the previous "every system certificate is present"
expectation cannot hold once expired entries are excluded.
The shared transcript module now imports the Thinking shimmer. Keep the session-reference renderer probe's dependency table complete so it can continue exercising message references without loading the app store.
feat(transcript): add display modes and thinking shimmer
Bring the contributor branch onto the current integration base before validating and merging the OAuth fix.
Bring the transcript reading-position fix onto the current integration base before validation and merge.
The PR reused D665 after current main assigned that decision to the macOS window surface. Integrate the latest base and keep the deep-search decision uniquely traceable as D669.
The shallow workspace index could hide deep files from the @ menu. Search the full bounded listing while the snapshot is truncated, and discard stale query results.
Current main adds a second search handler in the same workspace IPC module. Keep that handler and the contributor's worktree reference mapping together in the integration candidate.
Absolute references from the parent checkout missed the isolated session's worktree. Resolve the matching file in the worktree and pass that exact path to the file view.
Integrate the reviewed transcript reveal change with the latest main before candidate validation.
Restoring a session could let stale pinned state send a reader to the latest turn. Let the saved unpinned reading bookmark own the pane reveal.
The pull request was based on an earlier main revision. Merge the current production tree into the shared branch so review and landing use the latest main without rewriting the contributor's commits.
…t-tab-stacking fix(window): 修复窗口恢复后残留置顶导致的窗口遮挡问题
The pull request was based on an earlier main revision. Merge the current production tree into the shared branch so review and landing use the latest main without rewriting the contributor's commits.
…y-disclosure fix(transcript): keep activity details visible after turns settle
The pull request was based on an earlier main revision. Merge the current production tree into the shared branch so review and landing include the latest changes without rewriting the contributor's commits.
…ferences fix(chat): parse positioned and ambiguous file references safely
Keep a revisited transcript near the reader's last position when virtualization omits the anchor row instead of leaving it at the initial viewport.
A bookmarked anchor can be outside the mounted window while its bounded history lookup is pending. Start viewport alignment only after the lookup succeeds so the fallback deadline cannot cancel it. Cover delayed lookup and document the approximate offset behavior.
Restore the saved absolute scroll offset when a bookmarked row is loaded but outside the mounted window. Wait for bounded history lookup to finish before starting alignment, so slow reads are not canceled by the fallback deadline.
Windows system stores can retain expired certificates, and merging one as a trusted anchor can make otherwise valid provider chains fail validation. Filter only expired system entries while preserving bundled and extra certificates, and keep the behavior covered by a deterministic regression case.
…nchors fix(agent-runtime): skip expired OS trust anchors when merging the CA set
Long sessions retain superseded system snapshots even though adapters send only the current instructions. Measuring the folded request keeps the configured output budget from collapsing to the provider floor, while model-declared mid-conversation support preserves its existing transcript.
…sed-system fix(runtime): size the output cap from the folded request transcript
…codex/review-pr1776
…ives-session-switch fix(asktool): keep card drafts across session switches
…codex/review-pr1803
…ed-test-suite fix(tests): keep the launch-resolver and sessionDir suites green on unprivileged Windows
…codex/review-pr1778
Drop ordinary empty assistant rows to stop provider retries growing the transcript, while preserving the one-shot silent reply allowed by a Host-verified completion notice. Spend or revoke that exception on the first completed response, accepted steering, and turn settlement. fixes vastsa#1766
…sist-guard fix(persistence): drop empty assistant rows regardless of status
…codex/review-pr1751
The retained list is rendered after the current remote entries are cleared, so source badges must follow the displayed snapshot. Keep the existing provenance visible until a retry replaces it.
…sults fix(settings): keep the last successful market list when a query fails
…codex/review-pr1769
Make the geometry fixture shrink the actual scrollport and reset it between stages so resize and hover are independently exercised. Update the bilingual E2E scenario to document both paths.
vastsa
approved these changes
Oct 22, 2026
vastsa
left a comment
vastsa
left a comment
Owner
There was a problem hiding this comment.
The remaining paths are now covered: a ResizeObserver reveals the active close control when the strip actually narrows, and pointer entry reveals a partially visible inactive tab. The updated Electron probe verifies the close rectangle and elementFromPoint hit target for narrow-active, same-tab resize, and hover-inactive states. I corrected the fixture so the resize stage really changes clientWidth from 205px to 80px; the current-main baseline fails there, while this candidate passes all three. The 29 WorkPanel tests, desktop typecheck, Biome, style-token check, docs/locale checks, and latest-main base check pass. GitHub's integration tree matches the tested tree. Thanks for extending the fix and adding the renderer-level coverage.
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.
What this fixes
Fixes #1762.
When the work panel is narrow and a tab title is long, the active tab's close button sits outside the tab strip's viewport — invisible and unclickable. The cause is a structural mismatch between what scrolls and what is measured:
WorkPanel.tsx:890-899).tabButtonRefs(:313-319).scrollIntoView({ inline: "nearest" })returns 0px of movement for a button that is already fully visible, so the sibling close button stayed clipped — the strip keepsoverflow-x: autowith a hidden scrollbar (work-panel.css:190-210).Change
WorkPanel.tsx— addtabItemRefspointing at the wrapper that contains both the tab button and its close button, and scroll that:The button refs are kept for focus/activation paths (
:416/:605/:640) — those need the focusable element, while revealing needs the full item box. The fallback keeps behaviour intact if the wrapper is ever absent.work-panel.css—scroll-padding-inline: 2px→2px 24px. The inline-end padding reserves the close button's width so the strip's own scrollport does not clip the button by sub-pixel rounding once it is scrolled to.test/work-panel.test.mjs— a regression assertion pinning the wrapper ref, the reveal expression, and the two-valuescroll-padding-inline, so a future refactor back to button-only scrolling fails loudly.Verification
node --test test/work-panel.test.mjs— 28/28 pass, including the new case "the active tab scrolls its wrapper so the sibling close button stays visible".tsc -p tsconfig.json --noEmit— clean (exit 0).node scripts/check-style-tokens.mjs—style tokens OK.main(8a8d56413), 0 behind,git merge-treereports 0 conflicts.Scope note, stated plainly: this is verified at the source-assertion level (the repo's established pattern for this component, whose suites read and match the source rather than rendering it). I did not run a real renderer and measure the button's bounding box against the strip's scrollport. The fix is mechanical — scroll the box that contains the button instead of the button alone — and the CSS padding is additive, so the risk surface is that one effect.
Duplicate check
No open PR or commit on
maintouchesWorkPanel.tsx/work-panel.csstab-strip scrolling; #1762 has no linked fix.