fix: file-link and session-sidebar review follow-ups from 1.19.0 - #310
Merged
Conversation
Five post-merge review items from PRs #306 (clickable file paths) and #307 (session sidebar): - constants.js FILE_PREVIEW_EXTENSIONS gains the media extensions it was missing vs the single-source sets in attachment-registry.ts (m4v ogv ogg oga m4a aac flac opus), so an in-workspace .m4a opens the preview player instead of the log viewer; new test/media-extension-parity.test.ts pins all three copies (constants.js, panels-ui.js, attachment-registry.ts) against each other. - FILE_PATH_LINK_PATTERN drops `etc` from its root alternation: /etc is unconditionally in DEFAULT_BLOCKED_TREES, so every /etc link 403'd. Negative cases added to the link-provider and response-viewer tests. - updateSidebarCount() counts the rows actually on the sidebar list (session rows + web-tab rows, minus filtered-out ones) instead of this.sessions.size, and applySidebarFilter() refreshes it so the count follows the filter box per keystroke. - The incremental-render connection-line gate now also fires in sidebar layout (this._lineageEdgeCount is permanently 0 there), matching the strip-scroll listener widened in #307, so a badge changing row heights redraws subagent/ultracode connectors. - isSensitivePath() blocks ~/.claude.json, ~/.claude/settings.json and ~/.claude/settings.local.json (credential-bearing by schema), anchored to homedir() read at check time so case-level .claude/settings*.json files stay servable in the File Viewer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Five follow-ups left by the post-merge reviews of PR #306 (clickable file paths) and PR #307 (session sidebar), all shipped in 1.19.0.
1. Media extension lists had a third, diverging copy
VIDEO_ATTACHMENT_EXTENSIONS/AUDIO_ATTACHMENT_EXTENSIONSinsrc/attachment-registry.tsare the single source for what plays in the preview overlay, andpanels-ui.jsmirrors them for the<video>/<audio>markup. The third copy,FILE_PREVIEW_EXTENSIONSinconstants.js(which decides whether a clicked terminal/chat path opens the preview overlay or the tail/log viewer), was missingm4v ogv ogg oga m4a aac flac opus. Result: clicking an in-workspace.m4aan agent had just produced opened the log viewer and rendered binary noise, while the same file in/tmpplayed fine through the attachment path.Fix: the missing extensions are added, and a new static guard,
test/media-extension-parity.test.ts(same technique astest/sse-registry-parity.test.ts), pins all three copies: every registry media extension must appear inFILE_PREVIEW_EXTENSIONS, and thepanels-ui.jsVIDEO_EXTS/AUDIO_EXTSsets must exactly equal the registry sets.2.
/etclinks were guaranteed deadFILE_PATH_LINK_PATTERN(the ONE pattern feeding both the xterm link provider and the response viewer's linkifier) listedetcin its root alternation, but/etcis unconditionally inDEFAULT_BLOCKED_TREESserver-side, so every/etc/...link rendered clickable and then 403'd on click.etcis removed from the alternation, in its single source only; both consumers inherit the change.test/link-provider-regex.test.tsandtest/response-viewer-file-links.test.tsgain negative cases (/etc/hosts, plus the extension-bearing/etc/app/config.jsonthat only the root removal keeps out).3. Sidebar count disagreed with the list under it
updateSidebarCount()renderedthis.sessions.size, but the sidebar list also shows web-tab rows and is filterable: 3 sessions + 2 dashboards read "3" above 5 rows, and a filter matching one row still read "3". It now counts the rows actually on the list (.session-tabrows, web tabs included, minus.tab-filtered-out), falling back to the session map before the first render.applySidebarFilter()refreshes the count at its tail, so it also follows the filter box per keystroke (the box callsapplySidebarFilterdirectly viaoninput, with no render in between); the previously separateupdateSidebarCount()calls after eachapplySidebarFiltersite are folded in.test/session-list-layout.test.tspins the new behavior.4. Stale connectors on incremental renders in sidebar layout
In
_renderSessionTabsImmediate()the connection-line redraw was gated onthis._lineageEdgeCount > 0. That count is permanently 0 in sidebar layout (lineage arcs are skipped there), yet subagent/ultracode connectors still anchor to tab rects, and a badge appearing changes ROW HEIGHTS in the vertical list, sliding every connector below it off its anchor. PR #307 already widened the strip-scroll listener's gate tothis._lineageEdgeCount > 0 || this.isSessionSidebarActive?.()for exactly this reason; the incremental-render gate now gets the same widening.5.
~/.claude.jsonand~/.claude/settings.jsonwere servable but credential-bearingisSensitivePath()names secret members individually so the surrounding trees stay attachable, but these two escaped:~/.claude/settings.jsoncan holdenv: {ANTHROPIC_API_KEY: ...}andapiKeyHelperby schema (settings.local.jsonshares that schema), and~/.claude.jsonholds account/OAuth-adjacent state. A blanket/\.claude\/settings\.json$/would wrongly block the CASE-level.claude/settings.jsonfiles users legitimately view and edit in the File Viewer (model override, hooks), so the new entries are HOME-anchored:homedir()is read INSIDEisSensitivePathat check time, per the file's own rule about module-load captures being wrong when HOME changes later (per-file test fixtures). Blocked exactly:<home>/.claude.json,<home>/.claude/settings.json,<home>/.claude/settings.local.json. Case-level copies, transcripts and team state stay attachable;test/sensitive-path.test.tscovers both directions, including a.claude/settings.jsonunder a different home.Verification
npm run test:ci: 261 files passed, 5202 tests passed (12 skipped, pre-existing)media-extension-parity,link-provider-regex,response-viewer-file-links,session-list-layout,sensitive-path,routes/file-routes-attachment-path-guard,routes/file-routes,attachment-magic,file-browser-hidden,file-preview-media: all greentsc --noEmit,npm run lint,npm run format:check,npm run check:frontend-syntax,npm run check:public-assets: all clean