Comments authored while the cumulative stack range is the active diff scope
carry line numbers that don't correspond to the PR's head diff, so refusing
the entire push at the top of runPush keeps layer-scoped comments from
silently posting alongside them. Wording is asserted by test-diff.sh
Instance 6.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Confirms isCrossRepository round-trips and HeadRepoURL is populated from
headRepository.url so ensureSHAFetched's fork-URL fallback fires for
cross-repo PRs. Guards prJSONFields against accidental field drops.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Sapling-on-git can fall back to git fetch <forkURL> <sha>, matching the
git-only path for cross-repo PRs. Pure sapling has no clean by-URL fetch
primitive, so surface a clear error telling the user how to proceed
instead of leaving the silent failure path that returned the same generic
'sl pull manually' message regardless of whether the PR was a fork.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Mirrors picker.go's topicChainSHAs gate. Without this, a local branch
left pointing at a commit in the default branch's history (e.g. an old
merged feature) would be picked up as a fake stack base whenever the
current branch traverses that commit during its first-parent walk —
turning every fresh feature branch off main into a spurious 'stacked'
detection.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Instead of stamping crit comment with whatever sessions[0] happens to be,
query every daemon in the cwd and:

- Return the unique Range focus when exactly one daemon exposes one
- Return nil (let disk ActiveDiffScope take over) on Range-vs-Range
  ambiguity rather than guessing
- Fall back to a unique working-tree focus only when no Range focus
  exists at all

The previous behavior could silently stamp comments with the wrong
scope when a user had both a PR daemon and a working-tree daemon
running in the same directory.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Hold writeMu across the swap+persist critical section and stop any
pending writeTimer up front. Without this, a debounce timer armed
just before SetFocus could fire between the in-memory swap and
persistActiveDiffScope, producing a torn on-disk state where the
new focus's empty Files coexist with the old ActiveDiffScope label
on comments authored under the new view.

The timer callback already takes writeMu, so blocking it here is
sufficient — the cancel + flush at the top of SetFocus stops any
in-flight debounced write before the swap happens.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The PR-scoped and commit-range review feature added two new launch
modes (--pr <num|url> and --range <baseSHA>..<headSHA>) but the agent
skill files still listed only the original three review modes. Agents
invoking /crit on behalf of users wouldn't know to reach for the new
flags when the user asked for a PR or commit-range review.

Adds the two modes to every workflow skill (claude-code, codex, cursor,
github-copilot, opencode, cline, windsurf, aider). Bumps the claude-code
plugin to 1.4.0 since this is a user-visible feature surface change.
Regenerates integration_hashes_gen.go to match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…s correctly

Two related stack-detection bugs surfaced when running crit against a
long-lived parent branch (e.g. staging that hasn't been merged to the
default branch in weeks):

1. detectStack walked back 20 commits and surfaced every ancestor
   intersecting mergeBase(default, HEAD)..HEAD — including dozens of
   commits unique to the parent branch's history. Tier-3 (naked-commit
   subject fallback) emitted these as separate popover rows, drowning
   the user's actual stack in unrelated ticket commits.

   Fix: split emission into branch-tier and naked-tier; drop any naked
   entry whose distance from HEAD exceeds the closest branch-tier
   entry. Naked commits between HEAD and the nearest branch tip stay
   (they're the user's exposed WIP); older naked commits are subsumed
   by the branch and dropped.

2. detectLocalStackFocus (the no-PR auto-detect path) populated only
   Focus.Label, leaving HeadRefName and BaseRefName blank. The frontend
   Resume pill falls back to base..head SHAs when those fields are
   empty, so users got "Resume 8ba38fe..67b709b" instead of "Resume
   stack: <branch-name>". The PR-driven path (focusFromPR) already
   populated these fields; the local-stack path was the gap.

   Fix: populate HeadRefName from vcs.CurrentBranch() and BaseRefName
   from the matched stack tip's label.

Adds two regression tests covering the topology — naked commits behind
a branch tip get dropped, naked commits ahead of one are kept.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Repositions the header chip cluster (base picker, branch chip, stack
chip, resume pill) as a single .compare-rail group with explicit roles
and reduced visual competition.

Working-tree mode reads as base ← head: the base picker is a neutral
chip (new --crit-pill-* tokens), the head/branch chip is the only
brand-tinted element so the eye lands on "the thing being reviewed".

Stack mode hides the base picker (changing it would lie — the diff
base is pinned to a SHA in this mode) and merges the branch chip with
the stack chip into a segmented composite via .compare-rail.is-stack
adjacent-sibling rules. Adjacent corners flatten and the inter-chip
gap collapses so a single 1px seam separates the segments. The exit ✕
gets its own segment with a separator border on the left.

Resume affordance becomes a quiet dashed annotation that solidifies
and tints brand on hover — reads as "you-were-here" without competing
with the live diff target chips.

Drops the (reviewing) marker text from the popover current row; the
brand-tinted background plus aria-current="page" already convey
current state. Stack chip glyphs (icon, chevron, exit) move to muted
neutral so brand color carries on the head label only.

Theme tokens added to all four blocks in theme.css.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Frees the diff-area-header toolbar row (one fewer chrome bar above
the diff) and answers the most common first-time question — "what
does Layer mean?" — inline via one-line subcopy on each radio:

  Layer       Only changes in this layer
  Full stack  All changes from <default> to here

The legacy .diff-scope-toggle bar is hidden via JS rather than removed
from the DOM so any external CSS overrides or hook references still
resolve. Click handler in renderStackChip's popover delegates the
same /api/focus call as the legacy toggle did — visual surface
moved, behavior identical.

Full-stack option is disabled with an explanatory title when the
session has no resolved default-branch SHA (consistent with the
legacy toggle's gating).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…t branch

For workflows where the repo's default branch isn't the user's effective
stack root — e.g. teams that stack feature work on top of a long-lived
parent branch like `staging` — full-stack scope diffing against the
literal default branch (`master`) drags in dozens of unrelated commits
from the parent branch's own history that the user already merged via
the parent PR chain. The cumulative diff becomes noise instead of
"everything in MY stack".

Redefines Focus.DefaultSHA as the *topmost non-default branch tip*
in the user's stack — the effective stack root — rather than the
literal default-branch tip. For non-stacked branches (single PR off
main) the two coincide and behavior is unchanged. For multi-layer
stacks, full-stack scope now diffs against the meaningful root.

Two paths updated:
  * autodetect.go:detectLocalStackFocus — walks ancestors after picking
    BaseSHA and remembers the *furthest* non-default tip seen before
    reaching the default branch.
  * picker.go:assignStackBases — stamps every entry with the topmost
    entry's HeadSHA when there are 2+ entries; falls back to the
    literal default branch SHA for single-entry stacks.

Adds three regression tests: a 3-layer local stack pins DefaultSHA to
the stack root, a 2-layer stack falls back to default, and the picker
endpoint stamps stack-root on every entry of a multi-layer stack.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The All / Branch / Staged / Unstaged toggle filters by working-tree
state vs baseRef — meaningful only when reviewing the live working
tree. In stack mode the diff is pinned to BaseSHA..HeadSHA, so the
toggle's filtering produces a half-baked state where the file list
gets working-tree-filtered but file diffs stay pinned to the range.

The fix is a one-line gate in applyFocusToHeader: when focus.kind
is 'range', set #scopeToggle to display:none. Same shape as the
existing base-branch-picker hide. Working-tree state remains
reachable — users can ✕ out of stack mode any time.

E2E coverage:
  * Adds an assertion that #scopeToggle is hidden in range mode.
  * Updates the scope-toggle.rangemode spec to assert the new
    layer/full-stack location (popover) instead of the legacy
    diff-area-header bar.
  * Adds an explicit assertion that the legacy #diffScopeToggle
    bar stays hidden so we don't end up with two competing
    controls for the same setting.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Two follow-on bugs from the smart-stack-root change:

1. Popover dropped the stack-root entry from the visible tree.
   The render code filtered any entry whose head_sha matched
   focus.default_sha — originally a defensive duplicate-drop because
   the old default_sha was the literal default branch (which picker
   already excluded from stack entries). With the new semantics
   default_sha is the stack root (e.g. `staging`), and that filter
   silently removed staging from the popover.
   Fix: drop the filter — picker.go's stackTipLabels already
   excludes the default branch, so duplicates can't enter the list.

2. Every popover entry was stamped with the same DefaultSHA (the
   topmost entry's HeadSHA). Clicking the topmost entry to navigate
   to it produced focus.default_sha = self, so full-stack scope on
   that entry would diff entry..entry — an empty range.
   Fix: in assignStackBases, the topmost entry falls back to the
   literal default branch tip; non-topmost entries point at the
   topmost. Single-entry stacks fall back unchanged.

Also updates the popover's "Compare against / Full stack" subcopy to
say "All changes from <stack-root> to here" instead of hardcoding
the default branch name. Resolves the stack root branch name by
looking up the entry whose head_sha matches focus.default_sha.

Updates TestHandlePicker_DefaultSHAIsStackRootForMultiLayerStacks to
assert the per-entry semantics (non-topmost → stack root, topmost →
literal default).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The popover root marker fell back to the literal string 'main' whenever
the topmost stack entry's base_ref_name was empty (always true for
branch-tier entries that aren't PRs). For repos whose default branch is
'master' (or anything else), the marker silently misled by claiming
'main' is the stack's root.

Adds default_branch_name to the /api/picker response, sourced from
vcs.DefaultBranch(). Frontend caches it alongside stackCache and uses
it as the highest-priority source for the root-marker label in the
stack popover. Falls back to entry-derived name and then 'main' for
defensive parity with prior behavior.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…d-trip

Page-load in stack mode showed a visible 2s shift: chip cluster painted
without the stack chip, then ~2s later (when /api/picker resolved) the
chip appeared and other elements re-flowed. Two compounding causes:

1. Cold prListCache. /api/picker fans out to `gh pr list`, which can
   take 2-5s on large orgs. Daemon boot left the cache empty until the
   first /api/picker call paid the round-trip in foreground.
   Fix: spawn a goroutine right after signalReadiness that calls
   prList.get() to prime the cache during boot. By the time the user
   opens the browser, the cache is warm. Best-effort — failures (no
   gh, no remote, file mode) are silently dropped.

2. Frontend deferred showing the stack chip until stackCache populated
   (renderStackChip required stack.length > 1 to display anything).
   Fix: render the chip immediately whenever focus.kind === 'range',
   using fields already on Focus. chipLabelForFocus now falls back to
   focus.head_ref_name (populated for both PR-driven and local-stack
   autodetected focuses) so the chip's label is correct on first paint
   without needing the picker. The popover starts as a "Loading stack…"
   placeholder and refreshes when stackCache arrives.

Together: chip cluster paints with the right label on first frame, and
even on a cold cache the picker fetch is happening in parallel rather
than blocking visible state.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Combined hot-fix batch responding to two independent reviews of this
branch (Go expert + frontend expert).

**Stack semantics (Go)**

- assignStackBases: when the VCS is Sapling, force every entry's
  DefaultSHA to the literal default branch instead of using the
  topmost entry. Sapling's localBranchTips falls back to "every draft
  commit is a tip" when no bookmarks exist, so the topmost entry is
  always a real user commit — anchoring full-stack to it would exclude
  the bottom commit's changes from any non-topmost entry's full-stack
  diff. Git stacks require explicit branch tips so the smart
  staging-style anchoring stays correct there.
- detectLocalStackFocus: same Sapling caveat applies. Skip the
  findFurthestStackTip walk for Sapling and pin DefaultSHA to the
  literal default branch.
- detectSaplingDefaultBranch: return "" instead of "main" when neither
  bookmark resolves. Avoids running `sl log -r main` against a phantom
  ref in downstream merge-base lookups; full-stack scope correctly
  disables itself when there's no resolvable default.
- detectLocalStackFocus: when Sapling lacks a bookmark on the
  matched ancestor, fall back to a short-SHA label instead of
  feeding the commit subject into BaseRefName as a pseudo-branch.

**Header behavior (frontend)**

- applyFocusToHeader: restore the working-tree scope toggle when
  leaving range mode. Previous code only had a hide-when-inRange
  branch; clicking the ✕ exit left the toggle stuck at display:none.
- loadStackFromPicker: queue a refetch when called during an in-flight
  fetch, instead of silently sharing the previous promise. Avoids the
  A→B→C focus-change race where C's data was never loaded if A's
  fetch hadn't resolved yet. /api/picker is server-cached so the
  follow-up call is cheap.
- doFinishReview: check resp.ok before parsing JSON; derive
  hasComments from `!data.approved && !!data.prompt` so an "all
  resolved — proceed" approval prompt doesn't get mis-routed into
  the agent-waiting UX.
- Cache compareRail / baseBranchArrow / scopeToggle DOM refs at
  module init alongside the other header chip refs; drop the
  per-call getElementById lookups in applyFocusToHeader.

**Process hygiene**

- gh subprocess on warm-prime now respects the daemon's shutdown
  context. Ctrl+C during boot kills the in-flight `gh pr list`
  process instead of orphaning it. New fetchOpenPRsCtx /
  prListCache.getCtx pair; existing call sites delegate via
  context.Background() so behavior is unchanged elsewhere.

**Cleanup**

- Drop dead CSS selectors .stack-popover-marker and
  .stack-popover-disabled (no longer emitted; replaced by :disabled
  on the popover items).
- Drop the .stack-popover-default class from the rendered root
  marker — only .stack-popover-root has rules.
- Restore aria-hidden="true" on the waiting-review spinner so screen
  readers don't announce three empty dots.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The Loading-stack placeholder added in d594e7c gated on
`stack.length > 1` — but for sessions where /api/picker resolves with
an empty/single-entry stack (e.g. `crit --range A..B` with no
detectable surrounding stack, or an unstacked PR), the placeholder
never transitioned to a loaded state and users saw a forever-spinning
"Loading stack…" message.

Three popover states now, gated on the stack array's identity rather
than its length:

  stack === null/undefined  → fetch in flight, show "Loading stack…"
  stack length ≤ 1          → loaded, no surrounding stack, show
                              "No surrounding stack — this is a
                              standalone range." so the user
                              understands the chip's role
  stack length > 1          → render the full tree

Also stamp an empty stackCache when /api/picker fails (HTTP error or
network exception) so a transient picker failure doesn't leave the
popover stuck on the loading placeholder. The chip remains usable
(✕ exits, label is rendered from focus data).

Reproduces in `make test-diff` instances 5 (--range A..B) and 6
(stacked-PR layer/full-stack).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ches

Instances 5 and 6 ran against single-branch fixtures (main → feat-c with
three commits) which doesn't surface anything in the stack popover —
the picker walks HEAD's ancestors looking for branch tips and only
finds feat-c. Manual smoke testing showed "No surrounding stack" in
the popover instead of the layered tree the instances are supposed
to demonstrate.

Updates both fixtures to mirror the E2E setup at
e2e/setup-fixtures-range-mode.sh: separate feat-a / feat-b / feat-c
branches stacked on each other, so the picker walks
feat-c → feat-b → feat-a → main and the popover surfaces all three
layers.

SHA values flow through unchanged downstream (--range A..B,
review-file stamping checks, /api/focus stacked metadata payload).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…-root)

Earlier in this branch we made Focus.DefaultSHA point at the topmost
non-default stack tip rather than the repo's literal default branch.
The motivation was a workflow where a long-lived parent branch absorbs
unrelated history and full-stack-against-default becomes noise.

That decision changed the semantic of "Full stack" in a way that
surprises every other workflow: in a typical stacked-PR setup
(default → feat-a → feat-b → feat-c), navigating to feat-b and picking
Full stack now diffs feat-a..feat-b instead of default..feat-b — the
topmost layer's own changes are excluded from any non-topmost entry's
cumulative view, even though the user considers those changes part of
their stack.

Reverts to the simpler invariant: Focus.DefaultSHA = literal default
branch tip, always. Full-stack scope on any entry diffs default..entry,
including the topmost layer's content.

For long-lived-parent-branch workflows the workaround stays: exit
range mode and use the working-tree base picker to set the parent
branch as the diff base. A future commit can add an opt-in
.crit.config.json field (e.g. stack_root_branch) to restore the
smart anchoring behavior for teams that want it.

Changes:
- autodetect.go: detectLocalStackFocus stamps DefaultSHA = defaultSHA
  unconditionally; drop the findFurthestStackTip walk and the
  Sapling-specific bypass (no longer needed since the literal default
  is correct for both backends).
- picker.go: assignStackBases stamps every entry's DefaultSHA with the
  literal default branch, drop the per-entry topmost-vs-not branching
  and the Sapling-aware fallback.
- autodetect.go: drop the dead findFurthestStackTip helper.
- frontend/app.js: drop the now-dead stackRootName lookup; the popover
  always uses defaultBranchNameCache for the "Compare against / Full
  stack" subcopy.
- Update tests: TestAutoDetect_LocalStack_DefaultSHAIsLiteralDefaultBranch
  (replaces _IsStackRoot and _FallsBackToDefaultBranch) and
  TestHandlePicker_DefaultSHAIsLiteralDefaultBranch (replaces
  _IsStackRootForMultiLayerStacks).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Orphaned-file entries are phantom FileEntry records that appendOrphanedFiles
adds for paths with comments in the review file but no matching entry in the
session's file list. Their purpose is to preserve user comments across
review-round + focus transitions — without them, a file that was commented on
in round N but no longer changes in round N+1 (or in the new focus's diff)
would silently drop its comments from the UI.

The implementation didn't account for FocusKey-scoped comments. When the user
navigates between range-mode focuses (e.g. clicking through stack-popover
entries), comments are tagged with the source focus's HeadSHA + DiffScope.
visibleInFocus filters them out for unrelated focuses — but the orphan entry
itself is still emitted by GetSessionInfo, leaving a "This file is no longer
part of the review" placeholder for files whose only comments belong to a
different focus the user already switched away from.

Filter: in GetSessionInfo, drop orphaned entries whose visible comment count
in the current focus is zero. The placeholder still renders for orphans with
at least one focus-visible comment (the original UX), and the file list stays
clean across focus navigation.

Reproduces in `make test-diff` instance 5: the harness seeds layer-scoped
comments on b.txt, then navigating from the seeded focus to a different
range (e.g. via the stack popover) leaves b.txt rendering as an orphan
placeholder even though its comments are scoped to the original focus.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Range-mode E2E suite had eight failures on CI flagged against changes
made earlier in this branch. All are mismatches between the test
assertions (written for the original PR's design) and the redesigned
header chip + stack popover. Behavior is intentional; tests updated
to match.

  * Restore the .stack-popover-default class on the root marker.
    The class was dropped as "dead CSS" since nothing styled it, but
    several tests rely on it as a stable selector for the
    non-interactive default-branch row. Adding it back is a no-op
    visually and gives the tests a documented anchor again.

  * Update "current entry has reviewing marker" → "current entry is
    aria-current". The (reviewing) text marker was deliberately
    removed earlier; the brand-tinted background + aria-current="page"
    convey current state without the extra text. Tests now assert the
    role/aria signal instead.

  * Rewrite "chip is hidden when stack 0 or 1 entries" →
    "chip stays visible; popover shows no-stack placeholder". Page-load
    UX prefers immediate chip render (label paints from focus data
    without waiting for /api/picker), so the chip is always visible in
    range mode. Empty stacks now render a "No surrounding stack"
    placeholder inside the popover.

  * Migrate scope-toggle-files test from #diffScopeToggle (legacy
    diff-area-header bar) to the in-popover scope rows
    (#stackPopover [data-action="scope"]). The legacy bar is
    intentionally hidden — toggle moved into the popover with
    one-line subcopy.

  * showScope = true unconditionally in renderStackChip's "Compare
    against" section. Previous condition (focus.is_stacked ||
    !!focus.default_sha) hid the rows entirely when neither held —
    confusing for users of unstacked range mode. Now Layer is always
    rendered as the canonical default, full-stack is rendered but
    disabled (with an explanatory title) when default_sha is missing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…isible)

…uited

…ound

… doesn't trip applyFocusToHeader

`/api/branches` returns JSON `null` when the repo has no remote branches
(server marshals a nil Go slice as null). Previously fetchBranches did
`baseBranches = await res.json()`, which made baseBranches null. The next
applyFocusToHeader call read `baseBranches.length` unconditionally and
threw a TypeError, which propagated through the init promise chain and
silently skipped the trailing `.then(connectSSE)` step — leaving the
page without any SSE event listeners.

In CI this was deterministic because the fetch resolved before init's
.then ran; locally on macOS the .then often won the race, so the page
got SSE listeners and tests passed. Outcome on CI: 14 SSE-driven tests
saw waitingOverlay never deactivate after round-complete (cli-comment
SSE updates, round-complete UI refresh, tab-badge transitions, viewed
state reset).

Two-part fix:
- fetchBranches coerces non-array responses to []. The endpoint can
  legitimately return null and the JS contract should be array-shaped.
- applyFocusToHeader guards baseBranches.length with Array.isArray as
  defense-in-depth — a future regression that nullifies the variable
  again will degrade gracefully (chip stays hidden) rather than nuking
  the SSE pipeline.