Skip to content

Bug hunt: per-file analysis and individual fixes - #1698

Merged
brillout merged 30 commits into
mainfrom
claude/bug-analysis-fixes-8dap4v
Aug 25, 2026
Merged

Bug hunt: per-file analysis and individual fixes#1698
brillout merged 30 commits into
mainfrom
claude/bug-analysis-fixes-8dap4v

Conversation

@brillout

@brillout brillout commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Systematic bug hunt over every source file in the repository.

Process

  • Each of the 550 source files got a sibling *.BUG-ANALYSIS.md documenting its business logic (high level), every function (low level), a thorough edge-case analysis of each, and the bugs found. The full notes live in this branch's history; the last commit reduces each doc to a single line recording when its file was last analysed and linking here.
  • Each bug was verified against the code, its SPEC and its tests before being fixed, and each fix is its own commit with a regression test where the seam allowed one. Every fix commit's subject carries its own diff size as [+added,-removed], and its message states the bug twice — a one-line TL;DR and a full description.
  • Fixes stay simple. Bugs whose fix would be complex and which are not critical were left alone.

Validation: pnpm typecheck clean; pnpm test green — 1588 node tests and 837 dashboard tests, 0 failures (7 tests added).

Scope note: "source file" here meant the JS/TS tree (.ts .tsx .mts .js .mjs .cjs). 34 files that could fairly be called source were not analysed: the 25 prompt markdown files under packages/framework/prompts/ (which compile into src/prompts.generated.ts), 3 GitHub Actions workflows, 3 CSS files, 2 HTML files and the Chrome extension's manifest.json.

Crashes and data-loss

  • Claude driver / Codex driver: a stdout line of null is noise, not a crash. JSON.parse('null') succeeds, so the non-JSON catch never fired and the next field read threw — inside a readline handler, where nothing catches it. The daemon died of the uncaught exception and its exit hook SIGKILLed every live agent.
  • RPC mount: parse the request path against a fixed base. The parse base was built from the request's own Host header, so Host: (empty — the fallback only covers absent) or Host: foo bar threw outside the handler's try, on a void-dispatched call. Any local process could end the daemon with one unanswered request.
  • CI watch: an unreadable status is not an absence of checks. ghPrCiStatus answers none both for a repo with no CI and for a gh read that failed — its own contract says acting on an unreadable status must never merge anything. A timed-out or rate-limited read merged a watched PR with its real checks unseen.
  • CI watch: a PR whose state is still being read is not an open PR. A warming cache answers pending behind a synthetic OPEN; the 150ms cold budget against a gh call meant the daemon's start-up tick saw that guess for every watched agent, re-merging landed PRs and able to start a fix session on a branch a human had closed.
  • Archived meta patches go through the atomic write like every other. patchArchivedAgent truncated a meta in place while the history list polled it — the exact failure the rename-based write exists to prevent, and which its doc claims every write in the module uses.
  • Closing keywords: defuse the colon form GitHub also accepts. GitHub's docs: "The keywords can be followed by colons... Closes: #10". Only whitespace was defused, so a plan agent writing Fixes: #1164 still closed the ticket its plan says is yet to be implemented — the incident this module exists to prevent.

Wrong behaviour

  • Choice panel: a delivered pick counts as delivered. Both deliverers resolve void and the action hook reports failure as undefined, so the panel's success test never held in production: it re-enabled its buttons instead of parking, and the questions hub never collapsed an answered card.
  • Unattended agents keep their messages, not just their Stop. The chat queue was gated on the gate switch, which unattended agents deliberately leave unset — so every message typed at a preset, routine or drain agent was shown as queued and then died unread, against both the SPEC and the comment above it.
  • Open PR: decide against the PR that last saw the branch. The moved-past check ran against the first PR, so a session whose PR merged mid-run and kept committing read as unlanded and the button opened a third PR for work already in.
  • Interventions: a parked agent is identified by which agent it is. Gate ids are unique only within an agent (every agent's first is await-choices), so two agents parked in one project shared an identity and only one was announced.
  • Auto PM: a click that names a routine leaves the sweep's calendar alone. A named Run-now stamped the maintenance schedule although the sweep never ran, postponing it a whole interval.
  • Tickets: a capitalised topic badge filters by its topic. Matching lowercases a ticket's topics and the URL parser lowercases what it reads, but click-to-filter added the badge verbatim — clicking UX emptied the page.
  • Markdown tables / file diff counts / untracked file size / device hop draft cap / resume command quoting / scheme-less device pastes — see the individual commits.

Dashboard state

Per-agent latches that never reset on selection change (AgentView, AgentComposer, HandoffArm), a merged branch reporting "no changes", two sidebar destinations highlighted at once, and a missing dot on cloud-waiting rows.

Security

  • File read: the .git guard matches the case the filesystem does. The guard matched .git exactly, but macOS and Windows resolve .GIT/config to the same file — and the path comes from the browser, on a config that holds a token after a gh clone.

Generated by Claude Code

claude added 29 commits August 25, 2026 11:57
Each source file gets a sibling BUG-ANALYSIS.md documenting its business
logic, functions, edge-case analysis, and bugs found. These docs are
working artifacts: a final commit will remove them all once the bug
fixes are in.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
…nly three dashes

TL;DR: A GFM table written with `:-:` or `--` separators rendered as raw pipes instead of a table.

isTableSeparator required 3+ dashes per cell, so valid GFM separators
like `:-:` or `--` left the whole table rendering as prose pipes — the
exact regression table support (#869) exists to prevent. GFM needs one
dash; the doc comment's own example `| :-: |` now passes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: The copyable resume command was broken for any repo whose path contains a space.

The one-liner embedded the recorded workspace path unquoted, so a repo
checked out under a path with a space (routine on macOS: /Users/John
Doe/…) produced a command whose mkdir/cd hit the wrong paths and never
reached `claude --resume`. Single-quote the directory in both places.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
…ll-origin device

TL;DR: Pasting a device address without `http://` stored a device pointing at `/null`.

new URL('localhost:4200/?token=abc') parses with `localhost:` as the
scheme and an opaque 'null' origin, so parseDeviceUrl returned
{url:'null'} — the dialog's name-field placeholder then crashed on
new URL('null').host, and saving would store a device whose connect
navigates to /null. Only http(s) pastes are device addresses.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: A non-Latin composer draft could break the whole device hop instead of being dropped from it.

The 7000 cap counted pre-encoding characters, but percent-encoding
grows multibyte text up to 9x — a CJK draft under the cap could encode
past the destination daemon's request-header budget and fail the whole
hop with the token, instead of degrading to a plain connect with the
draft dropped. Encode first, cap what the URL actually spends.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: Removing one agent's worktree hid the Remove button for every agent opened after it.

The view is mounted un-keyed, so switching agents only swaps props —
pressing Remove worktree on agent A left `removed` latched true and
hid agent B's Remove offer until a reload; a stale `archiveBehind`
could likewise swallow the next agent's archive catch-up re-read on an
equal-length collision.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
…session end

TL;DR: The 'Queued' note followed you to other agents, and to resumed sessions that never queued it.

The 'Queued — the session reads it between turns' echo latched forever:
switching to another agent, or resuming after a stop, re-showed a note
about a message this leg never queued. Reset it the way the sibling
stop/resume latches already do — on the agent id and when live drops.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: A merged branch's handoff reported 'no changes', and the 'merged' marker could never appear at all.

merged implies empty — a merged branch's commits are all on the base,
so `base..branch` lists nothing — which made the empty early-return
swallow every merged branch as "no changes" and left the `· merged`
marker unreachable. Say "merged" in the empty branch and drop the dead
span.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: Switching agents mid-arm painted the next agent's handoff checkbox with the previous one's pick.

The box is mounted un-keyed inside AgentView, so a pick still waiting
for agent A's event echo painted agent B's checkbox with A's pending
level — and the clearing effect (pending === armed) may never come true
on B, leaving the wrong arming state indefinitely.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
… and a still dot for cloud-waiting rows

TL;DR: Ticket pages lit two sidebar destinations at once, and a cloud session waiting on a question showed no dot.

A ticket's own page routes with a project and no picked run, so the New
item lit up beside Tickets — the SPEC's 'exactly one destination is
highlighted' broken by every ticket detail. And a cloud session parked
on a question reads 'waiting' but got no dot, because the dot was gated
on the local status being running; the SPEC gives every 'waiting' a
still dot.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: Answering a question left the panel live instead of parked, and the questions hub never collapsed the answered card.

Both deliverers resolve void, and useAction's run() reports failure as
undefined — so the panel's result !== undefined success test never held in
production: the panel re-enabled its buttons instead of parking, and the
questions hub never collapsed an answered card. Map delivery to a value and
latch on that. The hub tests' mockResolvedValue(null) workaround is gone:
the production-faithful undefined now collapses too.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: A CI status `gh` could not read looked identical to 'this repo has no CI', so the sweep merged the PR anyway.

`ghPrCiStatus` answers `none` both when a repo has no CI and when `gh`
itself could not say — its own contract states that acting on an unreadable
status must never merge anything. The sweep could not tell the two apart, so a
timed-out or rate-limited read merged a watched PR past the grace period with
its real checks unseen: the #1406 stale-check hazard by another route.

A read that succeeded always carries the head commit, so require it before
believing 'no checks'. The two no-checks fixtures now carry a head, which is
what a real check-less read looks like.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
…n PR

TL;DR: On start-up the daemon acted on guessed PR state, re-merging PRs that had landed and able to touch ones a human had closed.

A warming PR-lookup cache answers `pending`, and the value it hands over
meanwhile is the recorded PR with a synthetic OPEN state. The cold-read budget
is 150ms against a `gh` call, so the daemon's start-up tick saw that guess for
every watched agent: PRs that had already merged drew a doomed merge write and
a 'could not merge' line on every restart, and a PR a human had closed could
draw an unattended fix session pushing onto the branch they rejected.

Honour what `pending` means — the answer is on its way — and let the next tick
act on the state it actually read.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: `Fixes: #123` — the colon form GitHub documents — still closed the ticket.

GitHub's own documentation says the keywords can be followed by colons —
`Closes: #10` closes the issue exactly like `Closes #10` — but the phrase
pattern only allowed whitespace in the gap. A plan agent writing the very
common 'Fixes: #1164' still closed the ticket its plan says is yet to be
implemented, which is the incident (#1560) this module exists to prevent.

The gap is captured and re-emitted verbatim, so the sentence keeps its own
punctuation: 'Fixes: the ticket #1164'.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: Messages typed at an unattended agent were shown as queued and then silently thrown away.

The live-chat queue was handed to the agent only when `requestChoice` was
set, and an unattended agent deliberately leaves that unset so its gates take
the recommended option instead of parking. So every message typed at a preset,
routine or drain agent was pushed into the queue by the control watcher, shown
as queued in the composer, and then died with the process unread — while both
the SPEC ('An unattended agent keeps its control channel — Stop and messages
still work') and the comment right above say otherwise.

The queue now follows the control channel, which is what carries a message in
the first place; the gates stay tied to `unattended`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
…endar alone

TL;DR: Running a routine by hand postponed the maintenance sweep a whole interval without ever running it.

The routine a click names outranks a due maintenance sweep for the tick — the
job selection already says so — but the sweep flag stayed true regardless, and
the tick ended by stamping the maintenance schedule. So a Run-now on planning
or a triage, with the queue empty and the sweep due, postponed the sweep a
whole interval although its agent never ran.

The flag now asks first whether a routine was named, which also spares the
schedule read on those clicks, the way a switched-off sweep already does.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: Clicking a topic badge written in capitals emptied the tickets page instead of filtering it.

Matching lowercases a ticket's own topics, and the query-string parser
lowercases what it reads, but click-to-filter added the badge's text verbatim.
So clicking a badge reading `UX` filtered by a topic no ticket has and emptied
the page — the tags that are written lowercase worked, which is why it went
unnoticed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: One stdout line reading `null` killed the daemon and every agent it was running.

`JSON.parse('null')` succeeds, so the parser's non-JSON catch does not fire
and the next field read throws on it. That read happens in the readline 'line'
handler, where nothing catches it: the daemon dies of the uncaught exception
and its exit hook SIGKILLs every live agent, for one odd line of driver stdout.

Guard the parsed value the way the rest of this file already guards nested
ones, and the line is ignored like any other noise.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: The same hole as the Claude driver: a `null` line from Codex took the daemon down with it.

The same hole as the Claude driver's parser: `null` parses, so the banner
catch lets it through and `obj['type']` throws inside the readline handler,
killing the daemon and every agent it is running.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: Any local process could kill the daemon with a single request carrying a malformed `Host` header.

The mount built its parse base from the request's own Host header, so a
request carrying `Host:` (empty — the `?? 'localhost'` fallback only covers
absent) or `Host: foo bar` made `new URL` throw. The throw was outside the
handler's try and the server dispatches with a bare `void`, so any local
process could end the daemon with one request, unanswered.

Only the path and query are ever read, so the header bought nothing: a fixed
base is what every other request parse in the daemon already uses.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
…other

TL;DR: Patching an archived agent could make it vanish from the history list mid-write.

`writeMetaFile` exists because a plain write truncates a meta before it
refills, so a reader polling from another process sees an empty file and
reports the agent gone — and its own doc says every meta write in this module
goes through it. `patchArchivedAgent` did not: adoption and the Open-PR RPC
truncated an archived meta in place while the history list polled it.

Using it also restores the pretty-print and trailing newline the in-place
rewrite flattened out of the data-branch diff.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: The landing page shipped 'critcal' where it meant 'critical'.

The solution line of the Lazy-AI-plans row, as its SPEC spells it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: Open PR could open a third pull request for work a second one had already landed.

The Open-PR button's moved-past check compares the branch tip against a PR's
head, which only the latest PR on the branch can answer — the automatic
handoff already picks that way, and `pickAgentPr`'s own doc says the first
one would call work a second PR already landed unlanded.

The button asked for the first, so a session whose PR merged mid-run, kept
committing, and had a second PR opened for it read as still-unlanded and the
click opened a third PR for work that was already in.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
…not a prompt

TL;DR: Every prompt's SPEC document was compiled into the shipped bundle as if it were a prompt.

The filter excluded the literal names README.md and SPEC.md, so the 25
per-prompt `<name>.SPEC.md` docs were compiled in beside the prompts they
document: half the generated module's 50 constants were spec prose, ~26KB of
it, shipped in the published dist and rebuilt on every SPEC edit.

The script's own comment already said what it meant to exclude.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
…t is

TL;DR: Two agents parked on questions in one project produced a single notification between them.

A gate id is only unique inside its own agent — every agent's first gate is
`await-choices` — but the awaiting key was project + gate. Two agents running
in one project and both parked produced one identity, so the dedupe kept one
and the queue, the Discord message and the browser notice announced a single
'needs you' while the other agent waited with nothing pointing at it.

Concurrency is the normal case here (one agent per queue entry, three by
default), and the item already carries its agent — the unpushed key next to
it is keyed the same way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
…l counts

TL;DR: Removing a `---` separator or a `--` comment did not show up in a file's diff counts.

The +/- counter skipped every line starting `---` or `+++`, wherever it sat.
Those prefixes only mean 'file header' before the first hunk: inside one, a
removed `---` separator reads `----` and a removed `-- comment` reads
`--- comment`, and both were dropped, so the file's card under-reported what
the diff plainly showed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
…eview's

TL;DR: Every new file over 500 lines was listed as exactly +500 changes.

The patch is cut to the 500-line preview before the counts were taken off it,
and the Changes list reads an untracked file's added count from exactly there
— so every new file over 500 lines was listed as +500, while the SPEC says
that count is the file's line count.

Counted from the whole file, and by counting rather than re-parsing: every
line of an untracked file is an addition by construction.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
TL;DR: On macOS and Windows, `.GIT/config` slipped past the guard that refuses `.git`.

The guard refused a `.git` segment by exact match, but macOS and Windows
resolve `.GIT/config` to the same file — and the path comes from the browser,
so on those platforms the read this exists to refuse was one capital letter
away, on a config that holds a token after a `gh` clone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
The remaining batches of the sweep: business logic and every function of each
source file, with an edge-case analysis and the bugs found. Removed again in
the last commit of this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
The 550 per-file analyses were the sweep's working notes; what is worth
keeping beside a source file is when it was last gone over and where the
result went. Each doc is now that one line, and the notes themselves stay in
this branch's history.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WSWSBVmtSH4RgA4w1tuKGq
@brillout
brillout force-pushed the claude/bug-analysis-fixes-8dap4v branch from eed7a1b to 9cffdd2 Compare August 25, 2026 12:01
@brillout
brillout merged commit f1c7e8a into main Aug 25, 2026
2 checks passed
@brillout
brillout deleted the claude/bug-analysis-fixes-8dap4v branch August 25, 2026 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants