Release 5.5.0 - #54
Closed
serialexperimentslainnnn wants to merge 144 commits into
Closed
serialexperimentslainnnn wants to merge 144 commits into
serialexperimentslainnnn wants to merge 144 commits into
Conversation
release: v5.1.1 — plan limits refresh off-screen, reset countdown in chat
ci(release): drop setup-gradle from the publish job
The largest UI change since 4.0.0, requested after a session running agents under agents plus many background tasks made the single transcript unusable: consecutive "Thought process" rows from different agents, interleaved, unfollowable. Records what the protocol gives (task_started carries tool_use_id and skip_transcript, whose doc comment explicitly anticipates a tasks panel; every assistant/user message carries parent_tool_use_id, and TranscriptModel already keeps parentOf/isDescendantOf) and, more importantly, what it does not: task_started has no parent field, so the chain is our reconstruction, and background_tasks_changed carries no parent or tool_use_id at all, so the owning agent is genuinely unknown for a task never seen in a task_started. Also marks entry 1 done, with the two things the build found that the entry had not anticipated.
Step 0 of the 5.5.0 plan, and what stops every session re-deriving the same repo with the same greps. Built per the project-map skill: the "I want to change -> go to" table is the load-bearing part, directories and entry points are indexed rather than dumped, and no code content is copied, so it cannot drift into fiction. Commands are recorded as verified because they were run today, including the two local quirks that cost time to rediscover: node needs OPENSSL_CONF=/dev/null here, and claude is a system install at /usr/bin/claude while checkDrift defaults to ~/.local/bin/claude. The minefields section carries what has actually bitten: declaration order in ClaudeSession (which the compiler does not catch and InitOrderContractTest scans for), SensitiveGuard walking every string leaf as a path candidate, the hash-pinned CSP in JcefHost, and release.yml publishing on a merge to main. Doctrine stays in CLAUDE.md: this file says where things are, not how work is done.
Backend for the per-agent tabs. The binary already writes everything, one pair of files per subagent under <sessionId>/subagents/: agent-<id>.jsonl plus agent-<id>.meta.json carrying agentType, description, toolUseId, parentAgentId and spawnDepth. So the parent chain, the depth to indent at, the tab title and the link back to the spawning card are DATA, not inference -- system/task_started carries no parent at all, and the alternative was reconstructing the tree by joining events. The agent transcript is parsed by SessionTranscriptReader.parseEntries, the reader the session restore already uses, because that file is in the same format. One code path for live and restored: two paths for the same thing is what produced the duplicated thought-process bug in 4.0.4. Admission is the load-bearing rule. A session id can be resumed from the terminal, so that directory mixes agents this plugin spawned with agents it never saw -- 84 in one real session. An agent is ours if we observed its Task call, if a previous plugin run recorded it, or if its parent is ours. The last rule is not a convenience: a nested agent is spawned inside another agent's turn, so its task_started never reaches the main stream, and without inheritance everything below depth 1 is invisible. It is applied as a fixpoint because depth is not bounded. PluginAgentIndex persists admissions and tab state in workspace.xml, so a tab the user closed stays closed and nothing is lost by closing it -- the transcript is the binary's file and the card reopens the tab. It stores IDS AND TWO BOOLEANS only: no prompt, description or transcript goes into .idea, which is shared and effectively published. AgentIndexPrivacyTest pins that.
The index was a PersistentStateComponent in workspace.xml. The project directory is shared, gets committed by accident and is routinely synced, so anything written there is effectively published -- and which agents a session spawned already hints at what the user is working on. It now lives in ~/.claude/ide/claude-code-native/agent-index.json: private to the user, and the same place this conversation's data already is, so there is no second location to reason about. Its own namespaced directory keeps it from ever being mistaken for one of the binary's files. Contents are unchanged and stay minimal: ids and two booleans. Titles and transcripts are read from the binary's files on demand, so copying them would only create a second thing to leak or go stale. homeOverride follows the rule CredentialsVault set after a test JVM harvested and deleted real credentials: a test must be able to point this away from the developer's home.
The main transcript stops carrying other agents' work: a subagent's text, its tool calls and their results no longer land here. That interleaving is what made a session running agents under agents unreadable -- consecutive blocks from different agents with no way to follow any single one. Nothing is lost. Each agent's transcript is the binary's own per-agent file, read by AgentRegistry, and the Task card stays in the main transcript as the link to it. What still happens for a subagent's call is everything about the filesystem: the pre-write snapshot is captured and the VFS refreshed, because the binary writes files for whoever asked. Task events feed the admission rule -- task_started seeds it, task_progress too (a resumed session can reattach mid-flight and miss the start), and task_notification records how the agent ended so its tab can say so while keeping the transcript. The scan is coalesced: on a heavy session dozens of agents spawn at once, and one directory walk per event is a walk per event. Admissions are persisted after each scan rather than at task_started, because the tool_use_id to agent-id mapping only exists once the binary has written the sidecar. On init, a restored session pre-admits what a previous plugin run recorded, which is the whole of "restore the agent tabs". TranscriptReconciler.addSubagentText is deleted rather than left warm: a helper that anchors agent output under an Agent card is how the interleaving would come back. Its coverage moved to AgentRegistryTest. Named runningAgents because `agents` already means the catalog of agent types the initialize reply offers -- what you can spawn, not what is running.
Two rows under the chat tabs, following one rule: each row shows the children of the row above's selection. Agents shows the selected chat's agents, so switching chat swaps the row; Subagents shows what the selected agent spawned. Both stay visible when empty, so the transcript below does not jump as agents come and go -- on a session that spawns them constantly a row that appears and disappears is worse than an empty one. The strips are header-only: their tabs own no content, because the transcript is painted by the chat's single JCEF browser, which switches which transcript it shows. That is what keeps eighty agents affordable -- one Chromium per chat, not one per agent. Labels carry the tree the user asked for: "|_ description", one connector per level within the strip, capped so a deep chain stops indenting rather than losing its label. The full description, the agentType and how the agent ended go in the tooltip -- a kept tab has to say whether its agent failed, since reading why is the point of keeping it. That logic is a pure object (AgentTabLabels) so it is testable without Swing, and it is tested. A newly-spawned agent's tab pulses orange twice and returns to normal: peripheral vision without noise, since a permanent colour on a session spawning agents constantly is just noise. A rebuild mutes the selection listener and restores the selection: a scan can add, remove and re-parent agents at once, and without that every scan would look like the user had clicked a tab and would repaint underneath. Closing a tab reports it so the caller can remember it; the agent is not deleted -- its transcript is the binary's file and its card is the way back.
Wires the strips to the chat and gives every agent a place to be read. The rows live INSIDE each chat panel, which is both what puts them below that chat's tab (JBTabs owns the space between its header and its content, so a wrapper would have put them above) and what makes "only the selected chat's agents" free: each chat carries its own rows, so there is no shared state that could ever show another chat's agents. One browser paints every transcript. showTranscript(agentId) clears and re-sends; while an agent is shown the chat's live rows keep accumulating in the model but are not pushed, because the frontend upserts by row id and letting both streams write would interleave a live chat row into an agent's transcript -- the exact mixing this release removes. Switching back re-sends the chat whole. The Agent/Task card is now a link, not an expander: its work is not in this transcript any more, so expanding would open an empty box. It sends its tool_use_id and the host resolves which agent that spawned, because the pairing lives in the binary's sidecar and the card never sees an agent id. Clicking a card whose tab was closed REOPENS it -- closing hides a view, it never destroys anything, and this is the documented way back. Spawns blink the new tab twice in orange and raise ONE grouped notification per burst, suppressed when the chat is already on screen: this session spawns agents in waves, and a popup per agent is a storm that teaches you to ignore the tool window. The dashboard gains the Agents / Subagents / Background tasks windows under Session, each row carrying the ownership chain (Chat |_ Agent A |_ Agent B) and linking to its tab. For a background task the chat is always known but the owning agent often is not -- background_tasks_changed carries no parent and no tool_use_id -- so the row says "no known agent" rather than inventing a chain. The chain walk is cycle-guarded: a malformed parent link must degrade to a shorter chain, never hang.
The agent tabs and windows are a feature release, and the artifact should say so while it is being validated: a zip still called 5.1.1 is indistinguishable from the version already on the Marketplace, which is exactly the confusion an install-from-disk test does not need.
Four defects from the first look at it on screen. Rows appeared on a chat that had never spawned anything: two empty bars above every fresh conversation, asking a question nobody had asked. A row is now drawn only when it has something in it. That reverses the original "always visible" (which was mine to keep the layout still) because seeing it settled the argument. The rows are drawn the way tree(1) draws a directory -- fork, last child, and a trunk continuing past each opened level -- recomputed on every render because it depends on which rows are visible right now. A fork pointing at a row that is no longer drawn is worse than no tree at all. They are now a STACK OF LEVELS, not a fixed Agents+Subagents pair. An agent spawns agents, and so does each of those, so selecting at any level opens the level below it and selecting elsewhere collapses it. Two fixed rows could only ever show two levels of a tree the protocol does not bound. Background tasks were missing entirely, and they belong at EVERY level: the chat has its own, and so does each agent. A task's tab is a pointer rather than a transcript -- it shows the transcript of whoever runs it -- and it is not closable, because the plugin does not own a background task's lifetime; stopping one is a deliberate act with its own button. Finally the dashboard: Session was the only view button. There are now four stacked, one per window, each scrolling the panel to its own card and lighting up to say where you are. They are views of ONE panel rather than four panels: the data is a single payload, and splitting it would mean four things to keep in sync.
The Session view no longer carries agents at all. Its `subagents` list was built from the task event stream, while the new Agents / Subagents windows read the real tree from the binary's per-agent sidecars -- two views of the same agents, from different sources, which is precisely how they end up disagreeing on screen. Session keeps what is genuinely about the session: usage, context, cost, account, environment and MCP. Agents, subagents and background tasks live in their own windows, each row carrying the chain it hangs off. ClaudeSession.subagentTasks stays in use -- it is what resolves a background task's owning agent, since background_tasks_changed carries no parent -- but it is no longer something the dashboard draws.
A restored transcript put `<task-notification>` blocks, caveats and command output in the
user's own voice: `parseUser` mapped every `text` block of a `user` line to an
`EntryDTO("USER", …)`, and the binary writes all of that on `user` lines.
The wire already distinguishes them — `isMeta`, `isCompactSummary`,
`isVisibleInTranscriptOnly`, `isSidechain` and the `toolUseResult` object — and none of it
was read. `SyntheticUserText` is that one predicate, tested on its own, so the rule lives in
a single place instead of being re-derived per call site.
`background_tasks_changed` is a LEVEL signal: a task that finishes stops being listed, so its row, its tab and everything it had printed vanished at the exact moment there was something to read. `BackgroundTaskRegistry` keeps every task this session has seen and uses the level for what it actually says — presence means running, absence means finished. The level NEVER creates an entry. Its ids do not always match the `backgroundTaskId` a `tool_result` reports, and adopting the strangers produced a second, contentless copy of every task sitting next to the real one. The structured tool output is what makes a task ours and gives it its owner, its card and its command. The output is a real file — the binary names it in `system/task_notification.output_file`, modelled since 3.0.0 and never read — so `LiveOutputTail` tails it by offset (each poll costs only what is new) and `BackgroundTaskReplay` rebuilds it from the session JSONL after a restart, since nothing of it survives in memory. A task with nothing reported says so rather than showing a plausible blank.
They lived in `.idea/claude-code.xml`: per project, plaintext, and committable. The env block
belongs to the settings, and an env block is where an API key, a credentialed proxy URL or a
registry token ends up — so the configuration people commit contained secrets. The safe is
the same store the OAuth credential already uses, and application-wide is also the scope
these settings actually have.
Three things learned the hard way and pinned by tests:
- A failed READ is not an empty configuration. One unreachable keyring at startup used to
become a permanent overwrite on the next save, which reads as "the settings broke on their
own"; `readFailed` refuses that write.
- The legacy file is deleted only after the safe accepts the copy. It was adopted and removed
while the write failed a millisecond later (`secret_password_store_sync error code 36`),
which `set` reports to nobody, and the file was the only copy.
- Every mutation goes through `update {}`. Nothing persists for us any more, so a bare
`state.x = y` is a change that silently does not survive a restart — which is exactly what
the three "Always allow" mutators were doing.
The document IS the `@Serializable` state, so a field cannot be forgotten by the serialiser;
a test asserts that against the class rather than against a hand-kept list.
The strips were built twice in Swing — `JBTabs`, then a Java2D `PillTabsStrip` — and thrown away both times. A strip above the page cannot share the page's accent, its type scale, its transitions or its SVG, so every attempt ends up approximating one in the other by hand, which is exactly what it looked like. The chat UI has been a JCEF page since 4.0.0 and the bar belongs to it. One browser per chat, transcripts SWITCHED rather than one JCEF per agent: the session this feature came from runs dozens at once, and a Chromium process each is not a design. The host sends what EXISTS (the chat list, the agent tree flat, the tasks with their owner) and the page owns what is SHOWN — which single subtab is open. Round-tripping a click through the host would cost a repaint of the host's model to change a selection. Two things about identity, both of which cost a day. The bare agent id is the identity: the file is `agent-<id>.jsonl` but the sidecar says `parentAgentId: "<id>"` unprefixed, and taking the filename as the id collapsed the whole tree into one level. And admission is what separates our agents from the ones a terminal `--resume` leaves in the same directory — the plugin saw the Task call, a previous run recorded it, or its parent is already ours, applied as a fixpoint because a nested agent's `task_started` never reaches the main stream.
The session stops folding subagent output into the chat's transcript and hands it to the registry instead; the panel paints whichever transcript is selected. What was one stream of interleaved "Thought process" rows belonging to different agents is now one readable transcript per agent. The dashboard's three lists (Agents, Subagents, Background tasks) become ONE Workloads diagram rooted at the chats. They were three views of the same tree: to find out whether an agent had spawned anything you switched view, lost the parent, and read a breadcrumb to work out where you were. Two defects in that diagram, both fixed here with tests: - A chat was drawn TWICE. Pinning a subtab adds a second tab over the same panel — a view of one agent's transcript, not another workload — so the strip listed one session twice and the diagram drew it, its agents and its tasks twice over. - A nested subagent never stopped RUNNING. It has no `toolUseId` of its own (it was spawned inside another agent's turn, so no Task call of ours ever named it), so nothing could ever settle it and it pulsed for ever. It inherits its parent's ending: it cannot outlive the turn that spawned it. And ONE state vocabulary for everything the page colours (`JcefStatus`: running/completed/failed/stopped). The tab bar said `done` where the dashboard said `completed` for the same task, each with its own block of CSS, so the same thing was two different colours depending on which view you read it in. The host names the state; the page only paints it.
3.6k lines in one file. It is now seven, cut at the section banners it already had and concatenated IN CASCADE ORDER by `JcefHost.CSS_PARTS` — order is semantics in CSS, so a split that reorders is a rewrite. `scripts/split-css.py` refuses to write unless the parts reproduce the original byte for byte, which is the only guarantee that matters here. The tests read that same list out of `JcefHost.kt` rather than keeping their own copy or sorting the directory: a harness that concatenates in a different order tests a stylesheet the product never serves, and a part added to the host but not to the tests would go unchecked. ~200 lines of unreachable rules go with it: the Agents / Subagents / Background-tasks list views the Workloads diagram replaced, `.plan-body` (the plan card renders a `.perm-body`), `.q-body`, `.ro-bar`, `.ghost-text`/`.ghost-key` (the composer writes the suggestion as plain text), `.spacer` and `#empty .star`. `scripts/css-usage.py` reports the candidates; it is a reporting aid, not a gate — the gate stays `css-contract.test.js`, which checks the other direction.
`#boot` was `position: absolute; inset: 0` over the whole of `#app`, and `#tabsbar` is a child of `#app` — so a chat that was still starting hid the chat tabs with it, and switching to another chat while one boots is precisely what you do while you wait. Same for the sign-in card, which sits in the same place for the same reason. Both now live in a `#work` wrapper holding the transcript and the composer, which is the containing block their `inset: 0` resolves against. Blocking input while the binary comes up is deliberate; blocking navigation never was.
`JcefChatPanel` is an assembler and had grown past it — detekt's LargeClass, and the kind of file where behaviour lands because it is already open. Three things with their own subject move out, mechanically: - `LinkNavigator` — where a clicked link goes (browser, editor, Project view). It needs nothing from the session, the bridge or the transcript. - `SessionFeed` — the per-PROCESS data the dashboard draws (plan limits, MCP, version) and the poll behind it, with its cache and its burst throttle. - `AttachmentTray` — the chips pinned to the next turn, wherever they came from. No behaviour change and no new baseline entry: the two `ClaudeSession` findings are still the only ones in `config/detekt/baseline.xml`.
`checkDrift` green at the new versions: they advanced, the protocol surface did not, and `KNOWN_*` is unchanged.
`CLAUDE.md` §Status carried 5.0.1 while the build declares 5.5.0. The entry says what was built and, more usefully, the things that cost a day each: the bare agent id is the identity, the level signal never creates a task, a failed settings read is not an empty configuration. `PROJECTMAP.md` gains the routes that moved — `css/*.css` and its cascade order, the tab bar being drawn by the page, `JcefStatus` as the one state vocabulary, the settings living in the safe — and two minefields: the overlays must stay inside `#work`, and the CSP hash-pins the stylesheet too.
Since build 262 the platform ships the embedded browser as a bundled plugin of its own (`plugins/jcef-plugin`, id `com.intellij.modules.jcef`) instead of inside the core, and a plugin that does not declare it gets no `com.intellij.ui.jcef.*` in its classloader. The whole chat UI has been that browser since 4.0.0, so 5.1.1 shipped DEAD on 2026.2: every chat died on `NoClassDefFoundError: com/intellij/ui/jcef/JBCefApp` at `JcefHost.<init>`, reported on IU-262.9437.185. AsciiDoc, Cline and others broke the same way on that line. The dependency is HARD. An optional one that cannot be satisfied is skipped, which would leave 262 exactly as broken and just as silent — and there is no browser-less mode here to degrade to. That raises the floor to 253, and the cost was measured before paying it: on 251/252 the id does not exist at all (`JBCefApp` sits in `lib/app-client.jar`), on 253 and 261 the platform declares `<module value="com.intellij.modules.jcef"/>` in `product-backend.jar`, and on 262 it is the plugin. The compile target moves to the floor with it (253.28294.334, resolved from the Maven repository since that build has no installer on the CDN): building against an IDE that cannot satisfy our own dependencies makes `runIde` a sandbox the plugin refuses to load in. `verifyPlugin` reported Compatible on 262 throughout, and was right to — it resolves against the whole IDE distribution, not against the plugin's CLASSLOADER, which is where the failure lives. So the gate is a source contract: if the sources touch `com.intellij.ui.jcef`, the descriptor must declare it, hard, with a floor that has it. Mutation-checked — degrading the dependency to optional turns the test red. The verifier now also covers PyCharm EAP/RC alongside IDEA, since how a product bundles the platform is where this class of bug hides.
The README still advertised 2025.1, the version badge said 4.4.1 and the test counts were a year old. A user on 2025.1 whose plugin stops updating deserves the reason in the place they look, not in a commit message: the browser dependency 5.5.0 has to declare does not exist before 2025.3, and staying there means staying on 5.1.1. RELEASE_NOTES leads with that, because it is what the Marketplace shows as What's New and it is the only text an affected user is going to read.
The README described what the plugin HAS and never what you DO with it. Someone installing it had nowhere to learn what the first screen means, that an edit opens an editable diff they can change before accepting, that closing an agent's tab destroys nothing, or where the plan limits live. New `User guide` section, written from the code rather than from memory — the shortcuts are the ones `app-composer.js` actually handles, the menu items are the ones `plugin.xml` registers — covering the first run and its three states, the chat and its shortcuts, what happens when Claude wants to change a file, the agent tabs and the Workloads view, the dashboard, sessions, and a symptom-to-cause table for when something goes wrong. The rest of the file caught up too: 5.5.0's features (a tab per agent, Workloads, background tasks that outlive themselves, settings in the keychain) were nowhere in it, and the version badge, the build-output path and the test counts were a year stale.
Three statements were simply wrong, and each was checked against the code this time: - **Settings ▸ Tools ▸ Claude Code** does not exist. The configurable declares no `parentId`, so it sits at the ROOT of Settings — which is where every other mention in the file already sent you. - "The binary is always launched in default mode" is not what `SessionLauncher` does: it translates `acceptEdits`/`bypassPermissions` to `default`, and passes plan mode through. The security claim that rests on it is true for a more precise reason, now stated. - The build commands omitted `koverVerify`, which is a BLOCKING gate in CI, and `checkDrift` was listed with no mention that it needs a real binary and deliberately is not part of `check`. TROUBLESHOOTING gains the failure a 2026.2 user actually hits — the chat never loading, with the exact class name from the log — and says plainly that 2025.1/2025.2 stay on 5.1.1.
Three reports, one subject: the status shown was not the status. An agent LAUNCHED IN A RESTORED CHAT came up red. `restoring` is set when a chat comes back from disk and is never cleared — it is what admits that chat's own subagents — so the rule "an agent nobody watched start belongs to a previous run" also swallowed the ones launched afterwards. Restoring open chats is the default, so this was every agent in a freshly reopened IDE: red, while it was plainly working. Every agent of every PAST session came up red too, and that one does not merely look wrong: red asserts that they failed, and most had finished perfectly. A settled status is per-process memory, so a restart left the plugin knowing nothing — and it answered "I do not know" with "it failed". The binary had already written the answer down: an agent's own transcript ending on `stop_reason: end_turn` said its piece and stopped; `tool_use`, or a user line it never answered, was cut off. Verified against real transcripts before being relied on. A NESTED subagent never stopped running: it has no `toolUseId`, so nothing could ever settle it. It follows its parent now — it cannot outlive the turn that spawned it, and by the same token a working parent means the child is live work. The test found an NPE on the way: `meta.toolUseId in observedToolUse` with a nested agent's null id, and these are concurrent collections, which throw on a null key instead of answering false. The file already warned about exactly that.
Every agent was being registered as one: a row with no description ("Background Task
(background)") whose OUTPUT was pages of raw JSONL. The id gave it away — the task id in the
view was the agent id.
`task_notification` fires for agents too, and `observeOutputFile` was creating an entry from
it. Same lesson as the level signal, which learned it first: only a `tool_result` carrying
`backgroundTaskId` makes a task ours. Everything else may update what exists and must never
conjure it.
Install, sign-in and loading were `position: absolute; inset: 0`, so they covered whatever they were laid over: first the chat tabs — you could not switch chats while one started — and then, once that was fixed, the composer. And because the binary often comes up in a fraction of a second, opening a chat painted a full-window panel and removed it again, which reads as the whole plugin flashing. As content of the transcript none of that is possible: a row cannot cover what it does not own. The composer stays usable throughout, which is not a compromise — `send` already starts the process and queues, and `pump` writes once there is a session, so what you type while it boots is sent when it is ready. A short grace before the loading screen draws means a fast start never shows it at all.
With enough chats open the tabs were, in the user's words, neither scrollable nor reachable. Three faults, stacked: The row never bounded its width. `#tabsbar`, `.tab-rows` and `.tab-row` had no `max-width`, so the capsule GREW instead of overflowing and the extra tabs were painted outside the tool window — where no amount of scrolling reaches them, because there is nothing to scroll. The tabs then got much wider: since chats are named after the first prompt, an untruncated tab is as wide as that sentence. `.pill-label` had `text-overflow: ellipsis` and no `max-width`, so it never truncated. Capped now, full title in the tooltip. And the scrollbar was hidden on the grounds that the wheel would do — which left no way to reach a tab AND no way to see that there were more. Rather than restore a grey slab across a rounded capsule, the row itself became the handle: GRAB IT and drag, with a 4px threshold so a click is still a click, and the click that ends a drag is swallowed so releasing over a tab does not switch chat. Selecting a chat now CENTRES it, which is what makes ordinary use need no dragging at all — the row moves under you and both neighbours stay one click away. Mouse events, not pointer events, deliberately: `setPointerCapture` does not exist in jsdom, and this broke twice in a row without a single test noticing. The CSS contract is pinned too — bounded containers, a capped pill, the grab cursor — because that is what kept regressing.
Reported with a screenshot: a prompt containing a list came out inside a bordered slab, each bullet a blank line apart and the `1.` on a line of its own. Two declarations in one rule, both correct for the body this row USED to have and both wrong since it changed. `buildUser` renders `kind: 'md'` — it moved off verbatim so an attachment folded in as `[@name](jb://open?file=…)` reads as a link — while `.msg.user .body` still carried the card chrome and, fatally, `white-space: pre-wrap`. Over already-rendered markup that turns every newline in the MARKUP into a visible break, which is the whole of the reported spacing. The rule's own comment still said "Verbatim user text (kind 'text' → textContent)": a stale sentence outliving its symbol, in a stylesheet. So it flows like the assistant's row. What stays is the only thing the container was really needed for — `word-break`/`overflow-wrap`, because a pasted path has no break opportunity and would otherwise set the row's width — and the header, which is what marks the row as the user's now that no box does: `You`, and Copy, which still hands back `__rawText` rather than the rendered text. Pinned in both directions, against the stylesheet's text because jsdom lays nothing out: no `white-space`, no chrome, the wrapping kept, and the header and Copy still there. Verified by reverting the CSS with the tests in place — two fail, on exactly the two declarations.
`Chat <n>` came from a monotonic counter, so closing the last chat — which opens a replacement, `ChatTabsPanel.replaceLastChat` — walked the number up forever: a session spent closing and reopening one conversation reached `Chat 47` while never holding more than one. The count of chats ever created is not a fact about the chat in front of the user, and it is the number they are shown. It is the lowest number no OPEN chat is using now. Lowest-free rather than `size + 1`, because the size says nothing about which names are taken: with `Chat 1` and `Chat 2` open, closing the first and adding one would produce a SECOND `Chat 2`, and two identical pills in the tab row are indistinguishable — which is the confusion this area is already being debugged for. It reads the live titles, so a rename or a model-generated title frees a number as well as a close does. That is the intended reading: the number exists only to tell unnamed chats apart, so a chat with a real name is not holding one. `create()` is `@Synchronized` for the reason `gitChatOrCreate` already is — it is now a check-then-act over `sessions`, and a thread-safe list does not make a pair of operations one.
Two ways a click on a chat pill produced nothing at all, with no exception
anywhere and nothing in the log — which is how the ghost-tab report reads from
the user's chair, and it is indistinguishable from twenty other causes.
A pill carries the id the page was drawn with, so one whose tab is gone sends an
id that resolves to nothing, and `firstOrNull { }?.let { }` then does exactly
nothing. `selectById`/`closeById` now say so, naming the id asked for and the ids
that exist: a page drawing a list this strip no longer has is a statement the log
can make on its own, instead of one that needs a reproduction.
And `select` reaches a browser twice — `showTranscript` and `focusInput`. A throw
from either took the rest of the method with it, so `onSelected` never ran and
the selection was half applied: the card had moved, the listener that settles
`active` and the dashboard had not, and the only visible result was a tab that
did not seem to respond. An exception inside a bridge callback fires no `error`
event and reaches no log, so it was invisible twice over. Each is recorded and
the switch completes.
Neither is the cause of the reported randomness — the page was measured drawing
exactly the list it is pushed and returning the right id — and that is the point:
these are the two places where the cause, whatever it is, stops being visible.
They are an orientation index for AI-assisted sessions, not product: already excluded from the artifact, deliberately not gated, and regenerated by hand with scripts/gen-projectmap.py. Keeping them in history meant every session that refreshed one produced a diff nobody reviews, over a file whose only reader is the next session. The .gitignore rule alone would not have done it: an already-tracked file is tracked regardless of what .gitignore says, so this also removes them from the index. They stay on disk, and the map for this repository is now local.
The guard stops being a confirmation and becomes a lock with one documented key. An enforced SecurityRule now DENIES every caller — the agent's own tools exactly like an MCP server or a Skill, under bypassPermissions exactly like under default — and a rule the user switched off in Settings ASKS, every time, never silently allows. Why: an ASK on an enforced rule was an Accept button on a call the guard had just classified as dangerous, put in front of someone mid-task, which is precisely when it gets clicked. And the caller-trust matrix that produced it made a rule's meaning depend on which tool carried the call — a name that arrives on the wire from the binary, i.e. the one input a guard against prompt injection must not take policy from. AGENT_TOOLS, isTrustedCaller and SecurityRule.deniesEveryCaller are deleted rather than kept: they decided nothing once the verdict stopped asking who was calling, and evaluate() no longer takes a tool name at all. Six rule decisions come with it, each replacing something that read as reasonable: - CREDENTIALS is no longer exempt inside the project. "A file the user brought into their own repo is theirs" is the wrong premise here: we are stopping a model that may be acting on an attacker's instructions, and the repository is what the agent is allowed to write to. What remains exempt is a PLACE (the project sits under /tmp, or on a share) and never a THREAT. - SYSTEM_DEVICE is ^/dev(/|$) — the whole tree, one pattern. The enumeration it replaced was a blacklist, and it covered no GPU, no /dev/kvm, no bus, and not /dev/tcp/<host>/<port>, which is a network socket spelled as a file. A BENIGN_DEVICES allowlist went with it: it was dead code whose doc claimed otherwise — none of its twelve names matched any pattern. - No exempt redirect target either. 2>/dev/null is a write as far as review is concerned, and hiding output is what a problem does. - curl -o and wget are file writes. The one-liner curl | sh was caught, so the two-step version is what an injected instruction reaches for, and landing the payload inside the project tripped nothing at all. - isUnc asks for the FORM of a network resource: //<host>/<resource>, host an IP literal, a dotted name, or a bare label that RESOLVES (a bounded, cached lookup). Both earlier narrowings asked instead whether the input looked innocent, which is a rule the attacker-supplied string decides. This is also what makes a Python a // b not a share. - OUTSIDE_PROJECT resolves before it decides. Folding . and .. is textual and cannot see a symlink, so proj/link -> /etc made proj/link/passwd look like one of the project's own files to what was a string comparison. A binary is no longer "readable" for analysis: readText on an ELF returns mojibake, matches no rule, and a downloaded payload then executed was cleared precisely because there was nothing analysable in it. The one card the guard still raises says so. PendingPermission.guard carries the SecurityRule and the guard's own sentence to the page, which draws a red pulsing "Guard alert" naming the rule twice — the exact id, and the row label with its category so the switch is findable. The badge is text, because colour and motion are not information and neither survives forced-colors or a screenshot. And "Always allow" works on that card now: it did nothing before, which is worse than absent. It cannot open a lock — the ask branch is only reachable for a rule already switched off. GuardPolicyContractTest pins both halves as a table over every SecurityRule, so a rule added later is covered the moment it exists, and answers the question this change started from: the working directory is not a whitelist.
The code-execution, destructive-command, dev-tool-script and version-control families were inline in the guard; each now gets its own file, with the trusted dev-tool checksums as a resource rather than a literal, so adding a tool is a data change. SensitiveGuardPentestTest exercises them from the outside. CLAUDE.md stated the guard's contract for agents only. That contract is what a user needs before trusting the plugin with their machine, so it moves to docs/SECURITY-GUARD.md, with the trust chain published alongside it.
…sks" This reverts commit 2dc05c6.
The guard becomes a full defensive detector across the attacker kill chain: it recognises an attacker's own methods so a prompt injection acting through the agent is stopped at the technique, not only at the file it reaches for. It reads a command and refuses it; it never runs one, the same shape a Yara signature or an EDR rule has. New category INTRUSION_TECHNIQUE, deepest in the set, one toggle so an authorised engagement disables the whole family while every confidentiality and destructive wall stays up: HACKING_TOOL (~90 curated tools, anchored, cross-platform), REVERSE_SHELL (recognised by SHAPE across a dozen languages), PRIVESC_EXEC (GTFOBins escapes, dual-use floor kept). Curated lists are paired with shape rules because a name list is a blacklist and a blacklist is what you miss the next tool with. Analysis over refusal: a variable the command binds itself is no longer a false unresolvable; a command substitution is expanded and its inner command judged, not blanket-refused for being one; a script's own file writes no longer card at depth over zero. The shell-write rule is now LOCATION-AWARE, not global. Writing or deleting inside the open project is ordinary development and passes; only a write that lands off the workspace, where server data, configs and access data live, is a card. This was the noisiest rule in the set and the wrong criterion: what matters is not that a write has no diff, it is where it lands. The other rules cover the rest of the safety. The dev-tool checksum baseline is removed entirely: it could block a developer's own gradlew on any version its shipped hashes did not pin, and its keychain lookup ran on the reader thread with a first-run unlock dialog at IDE startup. The exemption is name-only; creating one of those files is itself a wall. Verified live against the rebuilt artifact across the whole battery: every intrusion vector, destructive op, reverse shell and GTFOBins escape blocks; every ordinary dev command and in-project write runs. Known residual, documented: reading a system-enumeration file outside the project through a command token still reaches the shell (the read-verb rule is not built yet).
The guard is the reason this plugin can be trusted with a machine, and it has been damaged more than once by well-meant edits nobody asked for. It is now off limits: no change to the security code or, even more strictly, to the tests bound to it, except under an explicit and unambiguous order. Breaking it means stopping, reverting and apologising. Neither party removes this directive.
Agents were stuck green: the colour comes from each agent's own transcript, but only a scan reads it, and the poll that scans when the stream is quiet had a gate that could not see the case. It was turnActive() AND anySettledAgent(), so a subagent finishing while the main session sat idle had no live turn (the timer had already stopped) and was not settled — it was the thing needing to be settled — and nothing ever re-read it. The gate now asks whether state could move in EITHER direction: a RUNNING agent may finish, which needs no live turn because a backgrounded agent outlives the turn that spawned it; a settled one may revive, which still does. With every agent settled and no turn running the timer retires exactly as before, so an idle chat still polls zero times. The stream cannot be the witness here: task_notification carries an optional tool_use_id that several of the binary's call sites omit. AgentStatePollGateTest drives the schedule through its own closures and asserts the gate rather than waiting on a timer to tick.
The reported "constantly running" agents: an agent's colour is decided by observedStateOf, and its first rule returned the status the binary's stream had written (statusByToolUse, filled by observeSettled from task_notification) BEFORE the agent's own transcript was ever read. task_notification sends started/running/in_progress for work in flight, each landing there as RUNNING, so a live progress notification pinned the agent RUNNING and the scan that could have seen it finish on disk was short-circuited — for the rest of the session, because the stream does not re-say "done" for an ending it delivered without a tool_use_id (several of the binary's call sites omit it) or delivered while the main session sat idle. The transcript is now authoritative. observedStateOf reads AgentEnding.of first: a closed turn is COMPLETED whatever the stream last said, a turn grown past its own close is RUNNING, and only when the file has NOT closed a turn is the stream's word consulted — a terminal status it delivered is an ending the file has not caught up to yet, honoured with its sealed instant, while a RUNNING from the stream is ignored because the file is the thing that says whether it is still going. The unfinished-file branch moved to its own function to stay within the complexity budget. Two AgentRegistry tests encoded the old stream-first resolution and were corrected to the authoritative-file model: a FAILED agent is tested with an unfinished transcript (a file that closed a turn cannot be a failure), and the instant-seal test uses a completed transcript the stream agrees with (a resumed one now reads RUNNING off the file, which its own test already covers). AgentStatePollGateTest keeps its four cases.
An agent's colour comes from its own transcript, and the rule that reads it knew only two endings: `stop_reason: end_turn`, and a final assistant record carrying no `stop_reason`. The binary writes two more, and both mean the work STOPPED without finishing: a `user` record whose text is `[Request interrupted by user]` (or its `...for tool use]` variant), and an `assistant` record under the reserved model name `<synthetic>` — in every observed case a session limit. Neither closes a turn, so both fell through to UNFINISHED and read as work still in flight. Unlike a genuinely open turn, nothing further is ever appended to such a file, so the wrong answer was permanent: the agent kept the running animation for the rest of the session. The resumed-then-cancelled case was worse, since RESUMED answers RUNNING unconditionally — no parent and no restore flag can soften it — so re-reading the file could never help. Measured over the 672 agent transcripts on this machine: 155 end on one of the two markers (78 cut-offs, 77 cancellations), split 41 RESUMED / 114 UNFINISHED, i.e. every one of them shown as live. They now read ABORTED -> STOPPED, which the page paints red, and the COMPLETED count is unchanged: no finished agent moved. The marker counts only as the LAST record, and only as the leading text: an interruption the agent worked past is not an ending, and prose mentioning one is not one either. The cut-off is matched on the structured `model` field, not on the message, which carries a clock and a locale. On the same corpus no marker has ever followed a closed turn directly, so this cannot bury an ending already reached.
A refusal was a dead end. The block named the rule that stopped the call and the only way to act on it was Settings, where the only choice is to turn that rule off PERMANENTLY — so the sole unblock the plugin offered was the most dangerous one, and the part nobody does is turning it back on. The block now carries a `Disable rule` link with seven durations: 5/15/30 minutes, 4/8 hours, until the IDE closes, forever. Five of the seven expire on their own, so the lock spends less time open than it did before this existed. There is no pre-selected option and no confirm button: every entry IS the action, so opening the menu commits to nothing and no reflex click can accept a default. Opening a rule stays an explicit choice, taken knowingly. Three lifetimes, three storages, because collapsing them loses information: the timed ones persist as `RULE=<epochMillis>` (an 8-hour suspension an IDE restart silently cancelled would be a control that lies about its own duration); "until the IDE closes" is process state and is never written, since the restart IS the expiry; "forever" is the existing permanent CSV, which is what no expiry means. The open set is recomputed on every can_use_tool, so a suspension that runs out is enforced again on the very next call, with nothing to run or remember. Enforcing a rule again from a switch cancels its suspension in both stores. DISABLED STILL MEANS ASK, NEVER BYPASS, and the one implicit pass left is gone. A tool marked "Always allow" used to skip a guard card, so one click on a Bash card opened every command Bash can run — including every other one the rule exists to stop. The unit of that answer was wrong, not its existence: on a guard card "Always allow" now approves THIS command, filed under the rule that stopped it, matched whole. It is honoured only while that rule is open, so re-enabling the rule revokes every command approved under it without a write — the check is a conjunction, not a stored expiry. The command is read from the pending request on the host and never from the page, so a compromised renderer cannot widen an approval past what the card showed. `SensitiveGuard` and the detection rules are untouched: the guard already takes the open set as an input, so only who computes that input changed. The broker's two branches and the transcript block are the only security-path edits, both under an explicit instruction. The block also confirms itself in the conversation. Suspending a rule has no other visible effect until the next matching call, so without that row the link writes a security setting and looks like it did nothing — this repository's most-shipped defect. Also brings the 5.5.0 notes up to date: the date, the catalogue's real size (28 rules in nine groups, not "the six rules of the security lock"), and the cancelled/cut-off agent fix. `SECURITY-GUARD.md` had stated that pre-approving a command belongs in Settings and NEVER on a card; that position is reversed here, so the document says so, states what bounds the new path, and keeps the reasoning rather than quietly dropping it. Tests are new, not adjusted: 25 over the two stores, the broker's policy in both directions, and the block's control. Two contract tests pin what review would otherwise have to catch — that the menu offers exactly the durations the host understands, and that every choice reads as English in the confirming sentence. The settings-form ownership gate classifies both new fields as off the form, which is what it exists to force.
`CC.markdown` is the one path in the page that turns model text into HTML: `marked` first, which passes raw HTML straight through, then DOMPurify. So `marked`'s output is attacker-influenced markup that has not been sanitised yet — and both fallbacks around DOMPurify returned exactly that string. A page served without `purify.min.js`, or a DOMPurify that raised on one pathological input, therefore stopped being sanitised at all. Nothing would look wrong: everything still rendered, and the only untrusted channel in the plugin was feeding unsanitised markup to every caller's `innerHTML`. That is CWE-636 / A10 on the surface that matters most here. Both fallbacks now return the escaped source, which is what the `marked` branch above already did when it was absent or threw. The two paths differ only in what firing costs — losing formatting versus losing the whole control — and only one of them was fail-closed. Five tests remove one leg of the sanitiser at a time. Each asserts the same two properties: the payload does not survive as live markup, and the text is still SHOWN, escaped, rather than silently dropped. The baseline case also pins that `<img>` itself survives with its handler stripped — an image is legitimate output, the config allows `data:image/` for inline ones on purpose, so asserting "no img at all" would have been the wrong property and would have passed just as well with the sanitiser missing entirely.
`scan()` claimed "Read once, parsed once, and each result serves every purpose that needs it". Read once was true. Parsed once was not: `parseEntries` ran a JSON pass over the lines to build the tab's rows, and `AgentEnding.of` then ran its own, identical pass over the same lines to judge how the agent ended. That is every admitted agent's WHOLE transcript, parsed twice, every five seconds (`AGENT_REVIVAL_POLL_MS`), for as long as the chat is open — and the session this feature exists for is precisely the one running dozens of agents. `SessionTranscriptReader.parseRecords` is now the single pass, and the two readings are taken from its output: `entriesOf` for the rows, `AgentEnding.of` for the verdict. `parseEntries(lines)` stays as it was for its three other callers and simply composes the two. Behaviour-preserving, and the reason is checkable rather than assumed: both parsers were configured identically (`ignoreUnknownKeys`, `isLenient`), so they accepted exactly the same set of lines. Blank and malformed lines are dropped by `parseRecords`, which is where both had dropped them before, so "nothing parseable" still reaches the verdict as an empty list and still answers null. The growth baseline deliberately still counts `entries.size`, not records: those are different numbers, and changing which one is compared would change when an agent reopens. That is a semantic change, not a performance one, and it has no place in this commit. `AgentEndingTest` keeps its fixtures as JSONL text and pushes them through the real parser, so the blank-line and malformed-line cases now assert that `parseRecords` drops them before a verdict is ever asked for. Hand-building `JsonObject`s instead would let the suite pass over shapes the binary never writes.
Every five-second pass read and parsed every admitted agent's WHOLE transcript, for the life of the chat. Most of those files are finished and will never be written to again, so the work grew with exactly the case this feature exists for: the session running dozens of agents. A pass now stats the file and skips the read when the stamp is unchanged. One `stat` in place of a whole read and parse. `size` is the load-bearing half of the stamp. These files are append-only — the property `reopenIfGrown` already rests on — so any new record makes the file longer. `mtime` covers the pathological in-place rewrite, and a change this can miss would have to rewrite the file at exactly the same length within one filesystem timestamp tick. That limitation is not hidden: the test that proves the skip works is built out of it, since `end_turn` and `tool_use` are both eight characters and carry opposite verdicts. WHAT IS CACHED IS THE VERDICT, NOT THE CONTENT. The file contributes exactly one thing to an agent's state — how it ended — so that is what is kept, and the rows come back from the previous snapshot node, which already holds them. Caching the parsed records would keep a second copy of every transcript in memory for the life of the chat, which is a worse trade than the reads it saves. `settledStateOf`/`observedStateOf` now take that verdict instead of the records, which is also what makes the skip provably safe to reason about: the rest of their input is the parent, the stream and this run's own bookkeeping. The state is therefore still recomputed on every pass even when nothing was read. A cache over the STATE would freeze an agent whose parent had since stopped, which is the bug this file spent two commits fixing. A missing file gets a sentinel stamp no real file can produce, so an agent whose sidecar is written later is read the moment it appears — without it, "absent" and "unchanged" would be the same observation. The cache is pruned to the snapshot's keys on every pass, like the other session-lifetime maps here. Three tests, one per property: it skips, a moved stamp forces the read, and a transcript that appears later is picked up.
`ClaudeSession` fronted `LoginCoordinator` with five functions — `startLogin`, `attachLoginUi`, `detachLoginUi`, `submitLoginCode`, `cancelLogin` — that forwarded the same names with the same arguments and the same defaults. The coordinator is now public and callers reach it directly: `session.login.start`, `session.login.submitCode`, and so on. Why this is worth a commit rather than being left alone: detekt counts this class's NON-PRIVATE functions, because a public member is one more thing that can call into a live session, and that count is 47 against a threshold of 20. Five of those forty-seven carried nothing at all. Extracting private helpers does not move that number — which is the trap in "just split the file" — so the only thing that reduces it is removing a way to call in. Seven call sites, all in the UI layer. Nothing about the flow moved: the card still lives in the panel, the flow still lives in the coordinator; the middleman neither of them needed is gone. 47 -> 42.
The seven verbs that change a RUNNING session's options — model, permission mode, effort, provider, extended thinking, the launch flags and the mode cycle — are now `SessionLiveSettings`, reached as `session.settings.changeModel(…)`. They are one subject rather than seven neighbours. Each answers the same question in a different place, and the three possible answers are the whole content of the new file: a control request the binary may refuse (model), one it cannot refuse (permission mode), or a launch flag that needs a restart (everything else). Spread across a 2,600-line class, that distinction is invisible. WHAT THIS COST, because it is not the mechanical move it looked like. The properties these verbs write were `var … private set`: read by the UI, written only by this class, enforced by the compiler. A collaborator cannot write them, so moving the verbs required widening eighteen setters to `internal` — the guarantee stops being structural and becomes a gate. `SessionStateOwnershipTest` is that gate: it pins that only `ClaudeSession` and `SessionLiveSettings` assign them, and a second test pins that the guarded list still matches what the class declares, so the first one cannot quietly cover less over time. One of those eighteen is `permissionMode`, which is where `bypassPermissions` lives, and that was raised before doing it rather than after. It never opens the deterministic guard — `PermissionBroker` refuses to let the mode answer a guard card, and nothing here changes that — but everything outside the guard is downstream of that field. Lain authorised it explicitly, with the test as the compensating control. State stays on the session rather than moving with the verbs: the UI reads those properties directly and they are `@Volatile` for that reason, so a second copy would be a thing to keep in step. What the new class takes is exactly the three effects it cannot reach — dispatching to the EDT, telling the listeners, writing a line to the binary — as function references, which is the shape `LoginCoordinator`, `SessionQueries` and `PollSchedule` already use here. Nothing about behaviour moved: same bodies, same order, same restarts. 47 -> 35 non-private functions, against a threshold of 20; 2,670 -> 2,549 lines. `LargeClass` still fires until the event router comes out.
Presenting a card and answering one are now `SessionCards`, reached as `session.cards.resolvePermission(…)`. One subject with a clean boundary: it covers everything between "the binary asked" and "the control response is written", and nothing else. It cannot widen what the guard decided, and that is structural rather than a promise: what reaches this class is only what `PermissionBroker` chose to surface, because a denied call never becomes a card at all. The review-edit override moved with it. That path is where the user's own edit of a proposed change becomes the input the binary is told to write, so it belongs beside the approval it modifies rather than in the diff layer, and its fail-safe shape — no edit yields null and the binary writes its own version — is easier to keep true in one readable place. TWO DELEGATES SURVIVE ON `ClaudeSession`, deliberately: `pendingPermissions` and `resolvePermission`. The security suite is immutable — those tests are the verifiers of the plugin's guardrails, so a refactor does not get to edit them, and a refactor that needs to is a refactor that is wrong. Two of them drive a session through those exact names. So the logic lives in `SessionCards` and these forward: one implementation, one behaviour, a spelling a frozen test can still reach. That is the difference from the five sign-in delegates deleted a commit ago, which forwarded for no reason at all — a delegate with a stated constraint behind it is a decision, one without is clutter. `cards` was the card STORE and is now the verbs, so the store is `cardManager`. Giving them the same name is how a reader calls the wrong one. 35 -> 33 non-private functions, against a threshold of 20; 2,549 -> 2,492 lines.
OUTSIDE_PROJECT asks where a call ACTS. A Kotlin doc-comment opener begins with
a slash, so an `Edit` whose replaced text was a doc block reached that rule as
an absolute path at the filesystem root — a match made of pure syntax, with
nothing to do with where the call was writing. It is the same class of false
match the neighbouring exclusions already exist for: a command's tokens are
code, and a `/pattern/` is absolute-looking by coincidence.
BOUNDED ON BOTH SIDES, and neither bound alone would be safe:
- the KEY must be a payload (`CONTENT_KEY`), so `file_path` — the real
destination — is still judged in full, and so is every other key. The shape
test alone would exempt a glob written as a destination;
- the SHAPE must be a whole block comment and nothing else: a bare opener, or
an opener that closes. The key alone would exempt every path ever quoted in
a document, which is the cost `CONTENT_KEY` documents as deliberately
accepted.
So `/**/id_rsa` is still a candidate — it opens like a comment and closes like
nothing — and a line comment is untouched, which matters because on Windows two
leading slashes are a UNC share.
`pathCandidates` is not changed at all. Every string leaf, payload included, is
still judged by the walls: a credential path inside a comment still trips
CREDENTIALS. The carve-out is scoped to the one rule whose claim a comment
cannot meaningfully make.
VERIFIED AGAINST THE FROZEN SUITE, which is the only acceptable evidence here.
The whole guard family is green — 211 tests across thirteen classes, plus the
two integration tests that drive a live session — and the four payloads in it
that begin with a slash all still fire their rule: two credential/foreign paths,
and two LINE comments, neither of which is a block comment. An earlier and much
broader attempt at this (excluding every payload from that rule, on the strength
of a stale generated map) broke one of those four; the code was reverted, not
the test.
Nine tests of its own pin the boundary from both sides, because an exclusion
nobody bounds is a hole nobody notices.
48% of the bytes under src/ were comment: 11116 of them across 441 files, gone in one
mechanical pass with the formatter run afterwards. Two kinds were kept because they are not
prose — tooling pragmas and licence lines.
Two consequences the gates caught, recorded here rather than papered over:
* `no-empty` counted a comment as a catch block's content, so the ~22 best-effort catches
in the page satisfied it with a bare "ignore" comment and nothing else. The rule now
allows an empty catch, and what it no longer asks, review has to.
* the CSS contract read the `attach-body` class out of a comment in composer.css and
counted it as a rule. It is a query hook with no styling of its own, so it joins the
grandfathered set instead.
The rewrite is `.claudetools/strip-comments.py`, which is ignored and not part of the build:
a character scanner rather than a regex, because Kotlin raw strings, nesting block comments,
JS template literals and regex literals all contain comment syntax that a line regex would
mangle silently. Nothing recursive, and no line was merged — no block comment sat between
code on the same statement.
normalize() ended by trimming every trailing separator, so a candidate that legitimately carries one reached the rules one character short of what the caller actually wrote — a directory named with its separator, or a payload whose last character is the closing slash of a block comment. The canonical form keeps it now, and the UNC shape test fixes that form for a regex literal, which is the case where the trim was doing the work unseen. OUTSIDE_PROJECT is unaffected in either direction: it folds its own candidates before the containment test, and fold() discards empty segments, so a trailing separator never survived that far anyway.
isUnc accepted any first segment without whitespace, so a directive-style line comment, a build tag, and the fragments a tokenised Python integer division leaves behind were all NETWORK_MOUNT — the hardest verdict in the set, DENY for every caller with no override — on an ordinary source edit or a one-liner. That segment must now be a name a network could resolve: a DNS or NetBIOS name, an IPv4 literal, or an IPv6 literal in either spelling, bracketed or bare, with an embedded IPv4 tail or a scope suffix. IPv6 is parsed by hand on purpose: InetAddress resolves a NAME through DNS, which is a blocking network call on the thread that reads the binary's entire stdout, made over a string the model chose. Two spellings are kept deliberately. The Win32 long-path form of a remote path names the host in the segment AFTER its prefix, not in the prefix itself, so the prefix is dropped before the host is judged. And a trailing dollar sign is stripped before the labels are checked, because that is how Windows itself names the share for the WSL filesystem — while a lone dollar sign stays false, since nothing is left once it is removed. Three assertions in SensitiveGuardTest recorded the old permissiveness as an accepted cost and now fail. They are the tests to change, and changing them is not this commit's business.
Three assertions recorded what the shapeless predicate cost rather than what it should answer: a directive-style comment, a lone dollar sign and the fragments a tokenised integer division leaves behind all read as a network share. The predicate now asks for a host a network could resolve, so those read false, and the two test names asserted the opposite of what the bodies do. Two of the five fragments still read as a share, and that is not an oversight: a bare digit and a bare word are syntactically single-label host names, exactly like the one-word share the test above this one requires to be recognised. The names now say "while it spells a valid host", so the next reader meets that on purpose instead of as a surprise. The orphan half-sentence above the first test went with it: the comment sweep kept that line because it contained the word nolint and read it as a tooling pragma.
The test image drops /usr/share/locale to stay small, so the JVM inside it inherits an ASCII sun.jnu.encoding and cannot create a file whose name is not ASCII. DiffPresenterIsWithinRootTest writes an accented filename to assert that containment holds for one, and it died there with FileNotFoundException while passing on every developer machine — 1584 tests, one failure, and the only difference was the locale. C.UTF-8 is built into glibc instead of living under /usr/share/locale, so the fix needs no image rebuild. That matters: the image is the only caching mechanism this pipeline has, and rebuilding it to change one environment variable would be the expensive way to say the same thing. Fixed here rather than skipped in the test on purpose. The plugin resolves paths the user chose, and a UTF-8 filesystem is what it ships into, so an accented path is a case the suite should keep asserting rather than assume away.
The menu was an absolutely positioned panel inside #conversation, which is a scroll container, a z-index:0 stacking context and smooth-scrolling: it was clipped by the scroller, dragged off its anchor by any scroll, and could never paint above the sticky #dock whatever z-index it carried. It had no way out either — the Escape handler sat on the menu, an element focus never reached — and it coloured its options with var(--fg), a custom property the stylesheet never defines, so they inherited the block's danger red. It is now positioned against the link in viewport coordinates, flips above when there is no room below, and closes on an outside click, on Escape, on a scroll and on a resize. Focus enters the menu on opening and the arrow keys move through it, which is what role=menu promises. "4 hour" and "8 hour" read as English again. The host's twin of those labels in SecuritySuspensions.Duration is untouched: it is security code and nothing reads it.
ShellFileWrites already exempts the inert sinks, but its redirect pattern captured every non-space character, so a trailing shell separator came along with the target. A redirect written as 2> /dev/null immediately before a semicolon reached the exemption test with that semicolon glued to it, matched none of the six benign sinks and became a SHELL_FILE_WRITE deny — on the most ordinary redirect there is, and it only needed some other absolute path in the same command to sit outside the project. The target now stops where a shell ends it: at whitespace or at a metacharacter. This cannot weaken the rule. A shorter target is still not one of the benign sinks, and a real write in the same command is still found, because the mutator verbs are asked before the redirect.
Fixed positioning was not enough. The panel still lived inside the conversation element, so it stayed in that element's stacking context — below the sticky dock — and the placement maths I wrote for it put the panel at the bottom edge of the viewport instead of against its anchor, cut off and barely readable. It now does what the composer menus have always done: on opening it is appended to the body and placed by the same helper, and on closing it goes back into its row, so the row still owns it. That helper moved to app-core as CC.placeMenu — one implementation for every menu in the page instead of two that drift apart. A row can be dropped while its menu is open — switching tab clears the conversation — which would leave a popup floating over the whole UI, so the menu watches the conversation for its own row disappearing and shuts itself. The frontend tests that reach for the menu while it is open now look it up from the document rather than from the row, and two more pin the handover both ways. Same assertions otherwise: nothing the control does was relaxed.
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
1 participant
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.
Release 5.5.0. Four changes on top of what
developalready has, and the last three are all in thepermission package.
The comment sweep. 48% of the bytes under
src/were comment — 11116 of them across 441 files — andthey are gone in one mechanical pass, with the formatter run afterwards. Tooling pragmas and licence lines
were kept, because those are not prose. Two consequences the gates caught rather than hid:
no-emptycounted a comment as a catch block's content, so the page's best-effort catches satisfied it with an
"ignore" comment and now need the rule to allow an empty catch; and the JS↔CSS contract had been reading
the
attach-bodyclass out of a comment and calling it a rule, so that class joins the grandfathered setas what it is — a query hook with no styling.
A trailing separator belongs to the path.
GuardPaths.normalizeended by trimming every trailingseparator, so a candidate that legitimately carries one reached the rules one character short of what the
caller wrote.
OUTSIDE_PROJECTis unaffected in either direction — it folds its own candidates andfold()discards empty segments — so this changes the canonical form, not that rule's verdict.A network share needs a host, not two slashes.
ForeignTerritory.isUncaccepted any first segmentwithout whitespace, so a directive-style line comment, a build tag and the fragments a tokenised Python
integer division leaves behind were all
NETWORK_MOUNT— the hardest verdict in the set, denied for everycaller with no override — on an ordinary source edit or a one-liner. That segment must now be a name a
network could resolve: a DNS or NetBIOS name, an IPv4 literal, or an IPv6 literal in either spelling, with
an embedded IPv4 tail or a scope suffix. IPv6 is parsed by hand because
InetAddressresolves a namethrough DNS, which is a blocking network call on the thread that reads the binary's entire stdout, made
over a string the model chose. Two spellings are kept deliberately: the Win32 long-path form of a remote
path, whose host is the segment after the prefix rather than the prefix itself, and a trailing dollar sign,
because that is how Windows names the share for the WSL filesystem.
The tests that pinned the old cost now state the rule. Three assertions recorded what the shapeless
predicate cost instead of what it should answer, and two test names asserted the opposite of their bodies.
Two of the five fragments still read as a share on purpose: a bare digit and a bare word are syntactically
single-label host names, exactly like the one-word share the neighbouring test requires to be recognised.
Verification
Run locally, in full, and the JVM suite on a forced pass (
--no-build-cache --rerun-tasks,:testprinting with no cache label):
test·detekt·spotlessCheck·koverVerify·verifyPlugin·buildPlugin·npm test(549tests, 26 files) ·
npm run lint·npm run format:check·npm audit --omit=dev --audit-level=low(0 vulnerabilities).
The guard family is green on its own too, including the two integration tests that drive a live session.