From 79c2de128e506da7cd70d950c6c1adeeb0069cce Mon Sep 17 00:00:00 2001 From: Kyle Seaman Date: Mon, 27 Jul 2026 13:58:22 +0000 Subject: [PATCH] fix(source): resolve PR merge state on first load, not on refresh Both providers compute mergeability lazily. The first read of a pull request they have not evaluated recently answers "not known yet" (GitHub UNKNOWN, GitLab checking/unchecked) and is itself what starts the computation, so a single read reports a conflicting PR as having no merge blocker at all. The panel read once, so its conflict banner only appeared after the user hit refresh -- the second read that got the real answer. Full fetches now re-read the merge fields alone (at most twice, 0.8s apart) until they settle, dispatched inside the existing secondary fanout so the wait overlaps calls the request was already making. An omitted field is never re-read, and a failed or still-unsettled re-read degrades to unknown rather than failing the panel. The short-TTL chip-status cache also carries the settled merge pair -- free on the GitHub call, already present in GitLab's payload -- and the panel folds a fresher poll answer into its pinned payload. That covers the second way a conflict stayed hidden: one that starts after the panel opened, which the staleTime-Infinity query would never notice. Unsettled values are never cached, so "still computing" cannot overwrite a real answer. --- .../modules/learn-cron-dashboard.md | 8 +- .../dashboard/handlers/source_providers.py | 239 ++++++++- test/test_source_providers.py | 455 ++++++++++++++++++ website/src/pages/ChatSidebar.tsx | 4 + .../src/test/pullRequestStatusDelta.test.ts | 39 ++ website/src/types/index.ts | 12 +- website/src/utils/pullRequestStatusDelta.ts | 31 +- 7 files changed, 761 insertions(+), 27 deletions(-) diff --git a/docs/system-specs/modules/learn-cron-dashboard.md b/docs/system-specs/modules/learn-cron-dashboard.md index 93c310afd93..c379ae18c38 100644 --- a/docs/system-specs/modules/learn-cron-dashboard.md +++ b/docs/system-specs/modules/learn-cron-dashboard.md @@ -1,6 +1,6 @@ # Self-Learning, Cron & Dashboard Modules -Last Updated: 2026-07-26 (foreign-agent first-run scan/review/apply flow, authenticated API contract, disabled schedule import, and onboarding order — the import gate runs after the cross-platform Kiro CLI prerequisite gate and before the theme tour. Prior — 2026-07-26 cross-platform Kiro CLI prerequisite status/install/login service, first-run SPA gate, and readiness-resumable post-fan-out synthesis. Prior — 2026-07-23 source payloads gain normalized `mergeable`/`mergeStateStatus` merge-state fields for GitHub and GitLab; PullRequestPanel surfaces a merge-blocker banner for open PRs — conflicts/behind carry an agent chat handoff, branch-protection blocks do not. Prior: left-nav IA restructure: sidebar toggle moved into the rail's menu row; Sessions label; Apps header with accent Explore link; per-frame Apps scrolling; bottom-pinned Agent Capabilities/Settings/Contact Us; Agents + Capabilities merged into the /capabilities panel — /agents redirects there. Prior: independent source-tabs hardening: provider binaries must be canonical root-owned non-writable paths; command-specific output ceilings and task-lifetime retained-byte reservations bound provider memory; only durable messages contribute sources; backend/frontend/panel retain at most 64 first-seen sources per slot. Prior: pull-request source links, source/check/resolve APIs, bounded sidebar CI refresh, SidePanel Changes view; learn_add session-recovery resolver now probes cron JSONL names; artifact companion-chat updates; silent-cron failure-alert suppression; CHAT_TURN_TIMEOUT remains aligned with ACP) +Last Updated: 2026-07-27 (pull-request merge state resolves on first load: both providers compute mergeability lazily, so full fetches bound-re-read the merge fields until they settle, the chip-status cache carries the settled pair, and the panel folds a fresher poll answer into its pinned payload — the conflict banner no longer waits for a manual refresh. Prior — 2026-07-26 foreign-agent first-run scan/review/apply flow, authenticated API contract, disabled schedule import, and onboarding order — the import gate runs after the cross-platform Kiro CLI prerequisite gate and before the theme tour. Prior — 2026-07-26 cross-platform Kiro CLI prerequisite status/install/login service, first-run SPA gate, and readiness-resumable post-fan-out synthesis. Prior — 2026-07-23 source payloads gain normalized `mergeable`/`mergeStateStatus` merge-state fields for GitHub and GitLab; PullRequestPanel surfaces a merge-blocker banner for open PRs — conflicts/behind carry an agent chat handoff, branch-protection blocks do not. Prior: left-nav IA restructure: sidebar toggle moved into the rail's menu row; Sessions label; Apps header with accent Explore link; per-frame Apps scrolling; bottom-pinned Agent Capabilities/Settings/Contact Us; Agents + Capabilities merged into the /capabilities panel — /agents redirects there. Prior: independent source-tabs hardening: provider binaries must be canonical root-owned non-writable paths; command-specific output ceilings and task-lifetime retained-byte reservations bound provider memory; only durable messages contribute sources; backend/frontend/panel retain at most 64 first-seen sources per slot. Prior: pull-request source links, source/check/resolve APIs, bounded sidebar CI refresh, SidePanel Changes view; learn_add session-recovery resolver now probes cron JSONL names; artifact companion-chat updates; silent-cron failure-alert suppression; CHAT_TURN_TIMEOUT remains aligned with ACP) ## Overview @@ -379,7 +379,7 @@ Modular aiohttp package at `127.0.0.1:5476` (configurable). Split into: `_pending_synthesis` armed and using `_synthesis_inflight` to prevent a duplicate waiter. It consumes the arm only after readiness and sub-agent delivery guards pass, immediately before the synthesis turn begins. -- `handlers/source_providers.py` — validates public GitHub PR and GitLab MR URLs, delegates authentication to `gh`/`glab`, normalizes metadata/files/comments/reviews/checks, recursively redacts provider strings, enforces subprocess and aggregate-payload limits, and maintains separate bounded caches for full source payloads and lightweight sidebar check state. Full source payloads carry a normalized merge-state pair shared by both providers: `mergeable` is `mergeable|conflicting|unknown` (`''` when the provider omitted it) and `mergeStateStatus` uses GitHub's lowercased merge-state vocabulary extended with a GitLab-specific value (`clean|dirty|behind|blocked|unstable|draft|need_rebase|unknown|''`). GitHub maps `mergeable`/`mergeStateStatus` from `gh pr view` directly; GitLab derives `mergeable` from `detailed_merge_status` (falling back to legacy `merge_status`) and maps `detailed_merge_status` onto the shared vocabulary (`conflict`→`dirty`, `need_rebase`→`need_rebase` (kept distinct: on fast-forward-only projects a merge commit cannot unblock the MR, so it must not be conflated with `behind`), approval/CI/discussion/policy/security gates (including `status_checks_must_pass`, `policies_denied`, `security_policy_violations`, `merge_request_blocked`)→`blocked`, `ci_still_running`→`unstable`, unrecognized non-empty values→`unknown`). Provider CLIs run through `sandboxed_spawn_argv(..., mode="standard")` using only fixed absolute candidates or explicit absolute `KIROCREW_GH_BIN` / `KIROCREW_GLAB_BIN` overrides. Resolution fails closed unless the path is already canonical, contains no symlinks, and the executable plus every ancestor through the filesystem root is root-owned and neither mode-writable nor effectively writable by the non-root gateway user. Ordinary same-user Homebrew/Linuxbrew installs are deliberately rejected because a provider child receives provider authentication and a same-UID-replaceable path is not a trust boundary; root-owned ancestry makes validation stable through execution without a path TOCTOU. Operators using those package managers must provision a privileged canonical copy under `/usr/local/libexec/kirocrew/` or `/usr/libexec/kirocrew/` (or another root-owned hierarchy), point `KIROCREW_GH_BIN` / `KIROCREW_GLAB_BIN` at it when it is outside the fixed candidates, and refresh that copy after CLI upgrades. The child receives a fixed system `PATH`, never the gateway/workspace `PATH`; `resource_limit_preexec()`, a minimal provider-specific environment, and pinned public hosts remain enforced, and unrelated gateway/AWS/Slack credentials are not inherited. Provider stdout is section-bounded at 1 MiB for metadata/checks, 2 MiB for discussions, and 4 MiB for diffs/changes; normalized full payloads are capped at 8 MiB. A global four-command semaphore covers full-source, direct-check, resolve, and sidebar work. Unique direct full/check tasks share a 16-task ceiling and a conservative 128 MiB retained-byte budget: full tasks reserve 64 MiB and checks tasks reserve 8 MiB until the underlying task terminates. Same-URL callers coalesce before admission, detached stale full fetches retain their leases, and a stale pre-mutation fetch plus its required fresh successor can coexist at the exact aggregate ceiling. Successful thread resolution advances that URL's cache generation and detaches older shared fetches so pre-mutation results cannot refill the cache or satisfy the post-resolution refresh. Secondary metadata endpoints degrade independently so core source details remain available, but every failed files, commits, discussion/thread, pipeline, or job request is named in `partialSections` before its data falls back to an empty section; provider page limits and overflow evidence use the same deduplicated markers. Native Windows returns a clear 503 without spawning because no supported OS-level provider sandbox exists. Every provider execution emits credential-free SEL `invoked` plus `completed`/`failed` lifecycle events; policy and provenance rejections emit `denied`. The critical `invoked` append is shielded and awaited on a worker thread before spawn, preserving audit-or-deny ordering without blocking the gateway event loop; cancellation waits for that worker, pairs a landed `invoked` event with `failed/request_cancelled`, and never spawns, while other terminal events are best effort. Events contain only the logical provider and coarse reason, never argv, URL, repository, output, environment, credentials, thread id, or exception text. Sidebar refreshes run outside slot serialization with inflight deduplication, a 16-task pending ceiling, one-TTL overflow backoff, and a 512-entry status cache. Scheduling requires an exact dashboard-owner request. Cached check state is otherwise only repopulated at WebSocket-connect and slots-GET time, so each owner WebSocket connection additionally runs a background refresh driver that re-schedules refreshes for its currently-rendered sidebar chip URLs (`DashboardState.source_link_urls()` — the first `_SERIALIZED_SOURCE_LINKS_PER_SLOT` links of every slot) once per cache TTL (`CHECK_STATUS_TTL_SECS`, sleeping exactly one TTL so each round finds the previous round's entries just expired — one provider fetch per URL per TTL, coalesced across tabs by the inflight dedup); without it a PR merged or a CI run completed after page load would keep its stale connect-time chip until a full reload. The driver is owner-gated (never created for non-owner connections), cancelled with the connection, and advances a per-round starting offset by the pending admission cap (`CHECK_STATUS_PENDING_MAX`) each round so that when the number of stale chips exceeds the cap the admitted window rotates across every chip within `ceil(len/cap)` rounds instead of the same slot-order prefix winning every TTL and starving newer slots' chips indefinitely. Each round's work is individually guarded so a transient failure is logged and the loop continues rather than the driver dying silently and reverting to frozen chips. Generic slot serialization and broadcasts omit cached `state`/`ci`; owner HTTP and WebSocket snapshots opt in, and changed statuses trigger a debounced generic update followed by an owner-WebSocket-only overlay. `_ChatSlot` retains only the first 64 unique durable source links and stops scanning at the cap; `to_dict()` exposes up to three sidebar chips as `{url, provider, number}` to all authenticated callers and adds `{state?, ci?}` only at an owner-authorized serialization boundary. `state` is `open|draft|merged|closed` and `ci` is `running|passed|failed` when known. +- `handlers/source_providers.py` — validates public GitHub PR and GitLab MR URLs, delegates authentication to `gh`/`glab`, normalizes metadata/files/comments/reviews/checks, recursively redacts provider strings, enforces subprocess and aggregate-payload limits, and maintains separate bounded caches for full source payloads and lightweight sidebar check state. Full source payloads carry a normalized merge-state pair shared by both providers: `mergeable` is `mergeable|conflicting|unknown` (`''` when the provider omitted it) and `mergeStateStatus` uses GitHub's lowercased merge-state vocabulary extended with a GitLab-specific value (`clean|dirty|behind|blocked|unstable|draft|need_rebase|unknown|''`). GitHub maps `mergeable`/`mergeStateStatus` from `gh pr view` directly; GitLab derives `mergeable` from `detailed_merge_status` (falling back to legacy `merge_status`) and maps `detailed_merge_status` onto the shared vocabulary (`conflict`→`dirty`, `need_rebase`→`need_rebase` (kept distinct: on fast-forward-only projects a merge commit cannot unblock the MR, so it must not be conflated with `behind`), approval/CI/discussion/policy/security gates (including `status_checks_must_pass`, `policies_denied`, `security_policy_violations`, `merge_request_blocked`)→`blocked`, `ci_still_running`→`unstable`, unrecognized non-empty values→`unknown`). Both providers compute mergeability **lazily**: the first read of a pull request they have not evaluated recently answers "not known yet" (GitHub `UNKNOWN`, GitLab `checking`/`unchecked`) *and* is what starts the computation, so a single read reports a conflicting pull request as having no merge blocker at all. Each full fetch therefore re-reads the merge fields alone — `gh pr view --json mergeable,mergeStateStatus` / the GitLab merge-request endpoint, which is the only place `detailed_merge_status` is exposed — at most `_MERGE_STATE_REREADS` (2) times spaced `_MERGE_STATE_REREAD_DELAY_SECS` (0.8s) apart, dispatched inside the existing secondary fanout so the wait overlaps calls the request was already making. A value that is empty rather than unknown is never re-read (the provider omitted the field; re-reading cannot settle it), and an unsettled, failed, or malformed re-read degrades to `unknown` rather than raising — an unknown merge state costs one banner, never the panel. Settledness is judged on the **pair**, not on `mergeable` alone: GitLab settles `need_rebase` and its branch-protection gates in the detail field while `mergeable` stays `unknown`, so keying on `mergeable` would re-read a state the provider had already answered and then discard it. Provider CLIs run through `sandboxed_spawn_argv(..., mode="standard")` using only fixed absolute candidates or explicit absolute `KIROCREW_GH_BIN` / `KIROCREW_GLAB_BIN` overrides. Resolution fails closed unless the path is already canonical, contains no symlinks, and the executable plus every ancestor through the filesystem root is root-owned and neither mode-writable nor effectively writable by the non-root gateway user. Ordinary same-user Homebrew/Linuxbrew installs are deliberately rejected because a provider child receives provider authentication and a same-UID-replaceable path is not a trust boundary; root-owned ancestry makes validation stable through execution without a path TOCTOU. Operators using those package managers must provision a privileged canonical copy under `/usr/local/libexec/kirocrew/` or `/usr/libexec/kirocrew/` (or another root-owned hierarchy), point `KIROCREW_GH_BIN` / `KIROCREW_GLAB_BIN` at it when it is outside the fixed candidates, and refresh that copy after CLI upgrades. The child receives a fixed system `PATH`, never the gateway/workspace `PATH`; `resource_limit_preexec()`, a minimal provider-specific environment, and pinned public hosts remain enforced, and unrelated gateway/AWS/Slack credentials are not inherited. Provider stdout is section-bounded at 1 MiB for metadata/checks, 2 MiB for discussions, and 4 MiB for diffs/changes; normalized full payloads are capped at 8 MiB. A global four-command semaphore covers full-source, direct-check, resolve, and sidebar work. Unique direct full/check tasks share a 16-task ceiling and a conservative 128 MiB retained-byte budget: full tasks reserve 64 MiB and checks tasks reserve 8 MiB until the underlying task terminates. Same-URL callers coalesce before admission, detached stale full fetches retain their leases, and a stale pre-mutation fetch plus its required fresh successor can coexist at the exact aggregate ceiling. Successful thread resolution advances that URL's cache generation and detaches older shared fetches so pre-mutation results cannot refill the cache or satisfy the post-resolution refresh. Secondary metadata endpoints degrade independently so core source details remain available, but every failed files, commits, discussion/thread, pipeline, or job request is named in `partialSections` before its data falls back to an empty section; provider page limits and overflow evidence use the same deduplicated markers. Native Windows returns a clear 503 without spawning because no supported OS-level provider sandbox exists. Every provider execution emits credential-free SEL `invoked` plus `completed`/`failed` lifecycle events; policy and provenance rejections emit `denied`. The critical `invoked` append is shielded and awaited on a worker thread before spawn, preserving audit-or-deny ordering without blocking the gateway event loop; cancellation waits for that worker, pairs a landed `invoked` event with `failed/request_cancelled`, and never spawns, while other terminal events are best effort. Events contain only the logical provider and coarse reason, never argv, URL, repository, output, environment, credentials, thread id, or exception text. Sidebar refreshes run outside slot serialization with inflight deduplication, a 16-task pending ceiling, one-TTL overflow backoff, and a 512-entry status cache. Scheduling requires an exact dashboard-owner request. Cached check state is otherwise only repopulated at WebSocket-connect and slots-GET time, so each owner WebSocket connection additionally runs a background refresh driver that re-schedules refreshes for its currently-rendered sidebar chip URLs (`DashboardState.source_link_urls()` — the first `_SERIALIZED_SOURCE_LINKS_PER_SLOT` links of every slot) once per cache TTL (`CHECK_STATUS_TTL_SECS`, sleeping exactly one TTL so each round finds the previous round's entries just expired — one provider fetch per URL per TTL, coalesced across tabs by the inflight dedup); without it a PR merged or a CI run completed after page load would keep its stale connect-time chip until a full reload. The driver is owner-gated (never created for non-owner connections), cancelled with the connection, and advances a per-round starting offset by the pending admission cap (`CHECK_STATUS_PENDING_MAX`) each round so that when the number of stale chips exceeds the cap the admitted window rotates across every chip within `ceil(len/cap)` rounds instead of the same slot-order prefix winning every TTL and starving newer slots' chips indefinitely. Each round's work is individually guarded so a transient failure is logged and the loop continues rather than the driver dying silently and reverting to frozen chips. Generic slot serialization and broadcasts omit cached `state`/`ci`; owner HTTP and WebSocket snapshots opt in, and changed statuses trigger a debounced generic update followed by an owner-WebSocket-only overlay. `_ChatSlot` retains only the first 64 unique durable source links and stops scanning at the cap; `to_dict()` exposes up to three sidebar chips as `{url, provider, number}` to all authenticated callers and adds the whole cached chip-status entry — `{state?, ci?, mergeable?, mergeStateStatus?}` — only at an owner-authorized serialization boundary. `state` is `open|draft|merged|closed` and `ci` is `running|passed|failed` when known; the merge pair uses the shared merge-state vocabulary above and is present only once settled. Because the chip entry is spread whole, a refresh in which only the merge pair settles also counts as a status change and triggers the same debounced generic update plus owner overlay. - `server.py` — app factory, route registration, startup, SPA fallback middleware for React Router, `/api/ws` WebSocket route, token auth middleware, loopback-only binding (`127.0.0.1`). Fires background MCP probe at startup via `asyncio.create_task()`. Honors `agent.yolo=true` config at startup via `_apply_startup_yolo()` — attempts SEL audit first and only activates dashboard YOLO (6h TTL) if the audit succeeds (fail-closed). ### Security @@ -462,7 +462,7 @@ A pending tool approval has **two** pieces of state that must stay in lockstep: **Agent Config**: GET/PUT `/api/agent/config` (read/write `~/.kiro/agents/kirocrew.json`, PUT auto-restarts sessions) **Chat**: POST `/api/chat` (SSE stream, or JSON with `?ws=1` — chunks via WebSocket), `/api/chat/slots` (CRUD, POST accepts optional `agent` field to set agent at creation; list responses include `source_links` extracted from slot messages with cached provider/number/state/CI metadata), resume from history, POST `/api/chat/slots/{slot}/generate-title`, POST `/api/chat/slots/{slot}/agent` (switch agent for slot), POST `/api/chat/slots/{slot}/fork` (fork session — copies visible messages into new slot, body: `{at_message_index?, prompt?}`, returns `{ok, key, title, messages, prompt}`, new slot has `forked_from` metadata), POST `/api/chat/slots/{slot}/edit-resend` (edit a user message and re-run; in-place truncation of `slot.messages`, body: `{index?, ts?, content}`), POST `/api/chat/slots/{slot}/rewind` (edit any past user message and re-run; fork-and-swap — truncates `slot.messages`, removes the slot's ACP session via `SessionManager.remove`, deletes orphaned kiro-cli session JSONL at `~/.kiro/sessions/cli/.json[l]`, then runs the edited prompt against a fresh ACP session under the same slot key/title/folder. Mirrors kiro-cli `/rewind`. Body: `{at_message_index?, ts?, content}`), PATCH `/api/chat/slots/{slot}/mode` (switch session mode between `""` and `"orchestrator"` — `_VALID_MODES`; 404 missing slot, 400 invalid mode, 409 while the session is running) **Follow-up suggestions** (`suggest_followup` MCP tool → card above the composer): POST `/api/chat/slots/{slot}/followup` with `{items: [{title, description, prompt, branch?}]}` — broadcast-only (nothing persisted server-side), 404 unknown slot, re-validates `SUGGEST_FOLLOWUP_SCHEMA` server-side (≤3 items; title ≤120, description ≤600, prompt ≤8000, branch ≤80 chars full-matching `FOLLOWUP_BRANCH_RE`; hidden-Unicode stripped), redacts every field through `redact_exfiltration_urls` + `redact_credentials` — including `branch`, which is DROPPED rather than mangled when redaction alters it — and emits `followup_card` over WS. The card goes out on the OWNER-only websocket channel (`deliver_ws_owners`), never the all-clients broadcast, because an app caller can open `/api/ws` and would otherwise receive another user's complete handoff prompts. Returns `{ok, delivered}` where `delivered` is the number of owner-socket sends that actually COMPLETED — the send is awaited (`deliver_ws_owners`), not fire-and-forget, because a socket count is taken before any send runs and a window that drops in that gap would be reported as delivered; failed sends are dropped from the owner set. A card with no listener is reported to the model as not shown instead of a false success. Dashboard sessions only, at both layers: the MCP tool rejects non-`dashboard:` session keys, and BOTH endpoints require the OWNER's own identity via `is_owner_dashboard_request` — the same predicate the source-provider mutations use: an `app` claim of `""` is necessary but not sufficient, since a dashboard credential minted for a different subject carries it too and would raise cards in the owner's composer and create branches in the owner's repositories; when no owner is configured only the signed local bootstrap subjects (`local-app`, `local-startup`) are accepted, which is the standalone-local case. One carve-out: the loopback internal-secret grant sets `request["internal_auth"] = True` and NO app claim (that is the path every MCP call arrives on), so it is permitted. A request with neither marker really did skip authentication and is refused, SEL-audited. POST `/api/worktree/create` with `{repo, branch}` creates `/-wt-` on a new branch off `origin/HEAD` (falling back to `HEAD`) and returns `{ok, path, branch, base, reused}`. Security boundary: `repo` must name or sit inside a directory an existing chat slot is scoped to (the submitted path AND the resolved git toplevel are both checked, and only the server-held root is ever used as a path); sensitive paths refused; git is routed through the `sandboxed_spawn_argv` chokepoint in **strict** mode (OS isolation + scrubbed env; strict because the filter probe passes `--includes` and repo-controlled `include.path` could otherwise make git read `~/.aws/credentials` as config); a host with no sandbox backend and no `agent.sandbox_allow_unsandboxed_exec` opt-in gets a 503 instead of an unisolated spawn. On top of that: an argv list with no shell, `resource_limit_preexec`, 120s timeout, and `-c core.hooksPath= -c core.fsmonitor=false` so no repo-supplied program executes (a non-directory device has no hook to find and nowhere to plant one; an in-repo path is repo-writable and a gateway-owned temp dir is still same-uid writable between calls) — a repo declaring a `filter.*.{process,smudge,clean}` driver in EITHER repository config scope (`--local`, and `--worktree` when `extensions.worktreeConfig` is on and a `config.worktree` exists under the per-worktree `$GIT_DIR`) is refused (409) because `-c` cannot disable an arbitrary filter name; the branch must also be a ref git accepts (`foo..bar`, trailing `.`/`.lock`, and `HEAD` are rejected up front rather than after the claim); both probes pass `--includes` (git defaults it OFF for a specific-scope query, so a driver reached via `include.path` was invisible yet still ran on checkout), and an unreadable scope is refused too. The allow-list collection and the sensitive-path/`isdir` screens run on a worker thread, so a slot project on stalled storage cannot block the event loop. Concurrency: the branch is claimed atomically (`update-ref ""`) and the destination by `os.mkdir`, both before anything else, so cleanup after a failure deletes only what the request proved it created, pruning before the branch delete so an `rmtree` fallback cannot leave the claimed branch behind, and skipping the delete entirely when the post-prune listing shows another worktree holding that branch or cannot be read at all (`update-ref -d` lacks `branch -D`'s used-by-worktree guard, so deleting an adopted branch would strand that worktree on a dangling ref); a failed `worktree list` is treated as unknown (503), never as "nothing registered". Reuse requires the destination to be registered on the REQUESTED branch (`_dir_slug` keeps only a branch's last segment, so `feat/foo` and `fix/foo` collide). Both endpoints are SEL-audited. Frontend: both card actions PRE-FILL a composer and never send, appending below an unsent draft rather than replacing it (the pending-input path also persists the draft, so a plain set destroyed in-progress user text); the worktree action creates the tree, opens a session WITHOUT activating it, scopes it, and only then activates it and prefills — so the composer is never live in the default directory (and fails closed rather than prefilling an unrelated one), and deletes the session it just made if scoping fails. Per-slot card state is pruned on slot delete and in stale-slot pruning. User-facing guide: `docs/followup-suggestions.md`. -**Pull-request sources**: POST `/api/source/pull-request` with `{url, refresh?}` returns the normalized full GitHub PR or GitLab MR payload. POST `/api/source/pull-request/checks` with `{url}` returns `{checks}` through a lightweight one-call GitHub or at-most-two-call GitLab path without rewriting the full-source cache. POST `/api/source/pull-request/status` with `{urls: [...]}` (bounded to `STATUS_URLS_MAX` = 64 canonicalized URLs, non-PR/MR URLs dropped, non-list bodies 400) returns `{statuses: {url: {state?, ci?}}, refreshing: [...], ttlSecs}` read straight from the same short-TTL chip-status cache the sidebar uses — it never blocks on a provider call, and schedules the bounded background refresh for stale entries, so unknown URLs are simply absent until a later poll. `refreshing` is the scheduler's own report of URLs whose value is expected to change shortly (started or already in flight; pending-cap deferrals excluded) and `ttlSecs` is the cache TTL, so a client paces its steady state by the server's TTL and re-polls within seconds of a refresh landing instead of up to one extra interval later. The Changes-tab source strip polls it to mark every PR/MR tab with its lifecycle state and CI rollup (bounded fast follow-ups, then TTL pacing; failing polls back off exponentially to a 5-minute ceiling rather than stopping, and re-poll on reconnect), and drives the selected tab from the full payload instead. The strip polls rather than riding the WS slots channel that already carries cached `state`/`ci` for sidebar chips: slot `source_links` are capped at a handful per slot while the strip shows up to 64 sources, so widening every slots push for one panel would cost every client more than one bounded request per TTL costs this one. **One truth for two surfaces**: the full-payload cache and the lightweight chip cache are kept coherent instead of expiring independently (which let the sidebar chip and the detail panel render different lifecycles for the same PR while both were nominally "fresh"). A completed full fetch projects its `state`/`draft`/`checks` onto the chip cache via `status_from_full_payload` (`record_full_payload_status`), and a chip refresh that observes a *changed* status conversely drops the full payload for that URL, so the panel's next read cannot serve a lifecycle the chip has already moved past. A structural loop-breaker guards the two comment-mirrored projections: `_refresh_check_status` records each URL's *changed* transition, and once the identical `(previous → new)` transition repeats past `_CHECK_FLAP_DAMP_THRESHOLD` consecutive refreshes — the signature of a chip↔full vocabulary divergence — it stops invalidating the full payload and emitting deltas for that URL and logs loudly once, degrading a would-be unbounded provider-polling loop to a stale glyph; a genuinely changing PR produces distinct transitions, which resets the counter and clears the damp. **Turn-boundary refresh**: when a chat slot goes idle, `DashboardState.refresh_slot_source_status` re-reads that slot's serialized chip URLs through `request_check_refresh_now`, which bypasses the chip TTL — an agent turn that opened a PR, pushed a revision, or drove a review round is the moment the remote state most likely moved, and TTL rotation alone can lag it by minutes once there are more PR-linked slots than `CHECK_STATUS_PENDING_MAX`. It is gated on at least one owner websocket (status is credential-backed and nobody else can render it, so a headless gateway spawns no provider work), scoped to the one slot that finished, floored to one forced read per URL per `_CHECK_FORCE_MIN_INTERVAL_SECS` (URLs inside the floor fall back to plain TTL pacing), and best-effort — a failure is logged and can never break turn completion. **Status deltas push instead of polling**: whenever a URL's cached `{ci, state}` changes, the owner-only `source_status` WS event carries `{url, origin, ci?, state?}` to owner sockets (`DashboardState.push_source_status`, registered once as a delta sink at app wiring and unregistered on `on_cleanup`). `origin` (`"chip"` = the lightweight path learned it, `"detail"` = a full fetch's write-through produced it) is **diagnostic only** — the client patches its cached status batch AND invalidates `['pull-request-source', url]`/`['pull-request-checks', url]` for *every* changed delta regardless of origin. It must invalidate on `"detail"` too, because that delta is emitted by the single window whose full fetch ran; only that window received the fresh HTTP payload, so the other owner windows (whose detail query is `staleTime: Infinity`) would otherwise keep rendering the pre-change lifecycle. The initiating window's resulting refetch is harmless and cannot loop: `record_full_payload_status` runs only in the *uncached* fetch path, so the refetch hits the warm 30s cache and emits no further delta. The `origin` field is retained on the wire for diagnostics and possible future requester-aware routing; no consumer branches on it today. The client additionally invalidates the mounted pull-request queries on `chat_done` for the active slot, because lifecycle/CI deltas do not cover review comments or mergeability and the detail query is otherwise `staleTime: Infinity` (it would never refetch after mount). Polling remains the safety net for a missed event. POST `/api/source/pull-request/resolve` with `{url, threadId}`, POST `/api/source/pull-request/auto-merge` with `{url, confirmImmediateMerge?}`, and POST `/api/source/pull-request/ready` with `{url}` are the three mutations. All three go through one auth/audit/error wrapper (`_owner_mutation_response`), so a client disconnect, a rejected request, and a provider failure are recorded identically across them. Every credential-backed endpoint requires the explicit dashboard-user claim `request["app"] == ""`. When `state.owner_id` is configured, all six endpoints require an exact `request["user"] == owner_id` match. When no owner is configured, only the three read endpoints accept the signed machine-local bootstrap subjects `local-app` and `local-startup`; every mutation still returns 403. App tokens, non-owners, unrelated or missing subjects, and every unconfigured-owner mutation fail closed. Every direct source request makes a best-effort SEL audit attempt with only the caller, operation, coarse outcome/reason, never URL, thread id, provider output, or credentials. SEL write failure cannot weaken a denial or replace the request's response or exception. Provider CLI calls separately emit coarse `invoked`, `completed`, `denied`, or `failed` tool-invocation events with no argv or provider-controlled text. Cancellation while reading a request or awaiting a provider is recorded as `failed/request_cancelled` when SEL is available, then the original cancellation is re-raised; because a remote mutation may already have landed, mutation cancellation is intentionally an uncertain failure outcome. Cache removal, generation advancement, and stale in-flight detachment complete before provider mutation dispatch, so a cancellation cannot preserve or repopulate pre-mutation data. A mutation invalidates **both** caches: the full-source payload and the separate short-TTL chip-status cache the sidebar and `/api/source/pull-request/status` read, which would otherwise keep serving pre-mutation `draft`/CI state for up to one TTL. The chip cache carries its own per-URL generation counter for the same reason resolve advances the full-fetch generation: a status refresh that started before the mutation captures the generation at entry and discards its result if it changed, so an in-flight fetch cannot restore superseded state. The resolve endpoint validates thread ownership and resolves a supported review thread, returning `{resolved: true}`. The auto-merge endpoint arms provider auto-merge and returns `{autoMerge: true, mergeMethod}`: on GitHub it reads the pull request node plus the repository's allowed merge methods, refuses a draft or an already-armed pull request without dispatching, picks the first repository-allowed method in squash/merge/rebase order, and calls `enablePullRequestAutoMerge`; GitLab has no separate switch, so it reads the merge request first, refuses a draft or an already-armed merge request, and otherwise issues the merge call flagged `merge_when_pipeline_succeeds`; because that call merges immediately when no pipeline is pending, the GitLab path is a merge authorization, so with no pending head pipeline the request is refused unless the body carries `confirmImmediateMerge: true`. The field must be a real JSON boolean -- coercing it would let any truthy value, notably the string `"false"`, read as consent -- and any other value is a 400. That refusal is raised as `ConfirmationRequired` and answers with `{error, confirmationRequired: true}`, which is what makes the guard live rather than a constant the client asserts: the dashboard's confirming click sends `false`, and only the server's own refusal escalates the panel to a third, explicitly-worded `Merge now` step that sends `true` and quotes the server's reason. Confirm and Cancel are separate buttons, with Cancel standing where the arming button was, so the second half of an accidental double-click backs out instead of authorizing a merge. The ready endpoint clears draft state and returns `{ready: true}`: `markPullRequestReadyForReview` on GitHub, and on GitLab the GraphQL `mergeRequestSetDraft(draft: false)` mutation, refusing when the merge request is not a draft (`draft`, or legacy `work_in_progress`). Neither ready path rewrites the title: the draft-prefix grammar stays the provider's concern, a concurrent retitle cannot be lost, and a title that merely begins with a draft-like word (`Drafting widgets`) is never mangled. GraphQL reports refusals in the body with HTTP 200, so every mutation response is inspected for transport-level `errors` and the per-mutation `errors` field and raises rather than reading as success. Both read the current draft/auto-merge state before mutating so an inapplicable action is a 400 instead of a provider error, and both invalidate the cache before dispatch on the same rule as resolve. The full payload carries `autoMerge` (GitHub `autoMergeRequest`, GitLab `merge_when_pipeline_succeeds`) so the panel renders armed auto-merge as state rather than an available action. Invalid URLs/thread IDs return 400; provider CLI, authentication, secure-spawn, audit, or direct-fetch-capacity failures return 503. Sidebar source-link extraction ignores every non-durable message role (`chunk`, `done`, `streaming`, `queued`, and `permission`) and indexes only durable message content, preventing partial output, queue placeholders, and permission prompts from scheduling credential-backed provider work. Sidebar provider refresh and cached `state`/`ci` serialization use the same read-only boundary: an exact configured-owner request, or a signed `local-app`/`local-startup` dashboard request when no owner is configured. Non-owner and app-token slot responses retain the source URL/provider/number but cannot trigger or observe credential-backed status. +**Pull-request sources**: POST `/api/source/pull-request` with `{url, refresh?}` returns the normalized full GitHub PR or GitLab MR payload. POST `/api/source/pull-request/checks` with `{url}` returns `{checks}` through a lightweight one-call GitHub or at-most-two-call GitLab path without rewriting the full-source cache. POST `/api/source/pull-request/status` with `{urls: [...]}` (bounded to `STATUS_URLS_MAX` = 64 canonicalized URLs, non-PR/MR URLs dropped, non-list bodies 400) returns `{statuses: {url: {state?, ci?, mergeable?, mergeStateStatus?}}, refreshing: [...], ttlSecs}` read straight from the same short-TTL chip-status cache the sidebar uses — it never blocks on a provider call, and schedules the bounded background refresh for stale entries, so unknown URLs are simply absent until a later poll. `refreshing` is the scheduler's own report of URLs whose value is expected to change shortly (started or already in flight; pending-cap deferrals excluded) and `ttlSecs` is the cache TTL, so a client paces its steady state by the server's TTL and re-polls within seconds of a refresh landing instead of up to one extra interval later. The Changes-tab source strip polls it to mark every PR/MR tab with its lifecycle state and CI rollup (bounded fast follow-ups, then TTL pacing; failing polls back off exponentially to a 5-minute ceiling rather than stopping, and re-poll on reconnect), and drives the selected tab from the full payload instead. The strip polls rather than riding the WS slots channel that already carries cached `state`/`ci` for sidebar chips: slot `source_links` are capped at a handful per slot while the strip shows up to 64 sources, so widening every slots push for one panel would cost every client more than one bounded request per TTL costs this one. **One truth for two surfaces**: the full-payload cache and the lightweight chip cache are kept coherent instead of expiring independently (which let the sidebar chip and the detail panel render different lifecycles for the same PR while both were nominally "fresh"). A completed full fetch projects its `state`/`draft`/`checks` onto the chip cache via `status_from_full_payload` (`record_full_payload_status`), and a chip refresh that observes a *changed* status conversely drops the full payload for that URL, so the panel's next read cannot serve a lifecycle the chip has already moved past. The chip entry carries the normalized **merge pair** alongside `{state, ci}` — free on the GitHub chip read and already present in GitLab's payload — and `status_from_full_payload` projects it too, because a write-through that dropped it would make every full fetch look like a chip change and drive exactly the mutual-invalidation loop below. Each field is recorded independently and only once it is real, never as `unknown`: independently because GitLab settles `need_rebase` and its branch-protection gates in the detail field while `mergeable` stays `unknown`, and never-as-`unknown` because a still-computing read must not read as a change away from a real answer. Because the pair participates in the chip cache's change detection, a branch that starts conflicting while the panel is open drops the full payload and pushes a delta like any other status change — the panel re-reads and banners it without a manual refresh. A structural loop-breaker guards the two comment-mirrored projections: `_refresh_check_status` records each URL's *changed* transition, and once the identical `(previous → new)` transition repeats past `_CHECK_FLAP_DAMP_THRESHOLD` consecutive refreshes — the signature of a chip↔full vocabulary divergence — it stops invalidating the full payload and emitting deltas for that URL and logs loudly once, degrading a would-be unbounded provider-polling loop to a stale glyph; a genuinely changing PR produces distinct transitions, which resets the counter and clears the damp. **Turn-boundary refresh**: when a chat slot goes idle, `DashboardState.refresh_slot_source_status` re-reads that slot's serialized chip URLs through `request_check_refresh_now`, which bypasses the chip TTL — an agent turn that opened a PR, pushed a revision, or drove a review round is the moment the remote state most likely moved, and TTL rotation alone can lag it by minutes once there are more PR-linked slots than `CHECK_STATUS_PENDING_MAX`. It is gated on at least one owner websocket (status is credential-backed and nobody else can render it, so a headless gateway spawns no provider work), scoped to the one slot that finished, floored to one forced read per URL per `_CHECK_FORCE_MIN_INTERVAL_SECS` (URLs inside the floor fall back to plain TTL pacing), and best-effort — a failure is logged and can never break turn completion. **Status deltas push instead of polling**: whenever a URL's cached status — `{ci, state}` or the merge pair — changes, the owner-only `source_status` WS event carries `{url, origin, ci?, state?, mergeable?, mergeStateStatus?}` to owner sockets (`DashboardState.push_source_status`, registered once as a delta sink at app wiring and unregistered on `on_cleanup`). `origin` (`"chip"` = the lightweight path learned it, `"detail"` = a full fetch's write-through produced it) is **diagnostic only** — the client patches its cached status batch AND invalidates `['pull-request-source', url]`/`['pull-request-checks', url]` for *every* changed delta regardless of origin. It must invalidate on `"detail"` too, because that delta is emitted by the single window whose full fetch ran; only that window received the fresh HTTP payload, so the other owner windows (whose detail query is `staleTime: Infinity`) would otherwise keep rendering the pre-change lifecycle. The initiating window's resulting refetch is harmless and cannot loop: `record_full_payload_status` runs only in the *uncached* fetch path, so the refetch hits the warm 30s cache and emits no further delta. The `origin` field is retained on the wire for diagnostics and possible future requester-aware routing; no consumer branches on it today. The client additionally invalidates the mounted pull-request queries on `chat_done` for the active slot, because lifecycle/CI deltas do not cover review comments or mergeability and the detail query is otherwise `staleTime: Infinity` (it would never refetch after mount). Polling remains the safety net for a missed event. POST `/api/source/pull-request/resolve` with `{url, threadId}`, POST `/api/source/pull-request/auto-merge` with `{url, confirmImmediateMerge?}`, and POST `/api/source/pull-request/ready` with `{url}` are the three mutations. All three go through one auth/audit/error wrapper (`_owner_mutation_response`), so a client disconnect, a rejected request, and a provider failure are recorded identically across them. Every credential-backed endpoint requires the explicit dashboard-user claim `request["app"] == ""`. When `state.owner_id` is configured, all six endpoints require an exact `request["user"] == owner_id` match. When no owner is configured, only the three read endpoints accept the signed machine-local bootstrap subjects `local-app` and `local-startup`; every mutation still returns 403. App tokens, non-owners, unrelated or missing subjects, and every unconfigured-owner mutation fail closed. Every direct source request makes a best-effort SEL audit attempt with only the caller, operation, coarse outcome/reason, never URL, thread id, provider output, or credentials. SEL write failure cannot weaken a denial or replace the request's response or exception. Provider CLI calls separately emit coarse `invoked`, `completed`, `denied`, or `failed` tool-invocation events with no argv or provider-controlled text. Cancellation while reading a request or awaiting a provider is recorded as `failed/request_cancelled` when SEL is available, then the original cancellation is re-raised; because a remote mutation may already have landed, mutation cancellation is intentionally an uncertain failure outcome. Cache removal, generation advancement, and stale in-flight detachment complete before provider mutation dispatch, so a cancellation cannot preserve or repopulate pre-mutation data. A mutation invalidates **both** caches: the full-source payload and the separate short-TTL chip-status cache the sidebar and `/api/source/pull-request/status` read, which would otherwise keep serving pre-mutation `draft`/CI state for up to one TTL. The chip cache carries its own per-URL generation counter for the same reason resolve advances the full-fetch generation: a status refresh that started before the mutation captures the generation at entry and discards its result if it changed, so an in-flight fetch cannot restore superseded state. The resolve endpoint validates thread ownership and resolves a supported review thread, returning `{resolved: true}`. The auto-merge endpoint arms provider auto-merge and returns `{autoMerge: true, mergeMethod}`: on GitHub it reads the pull request node plus the repository's allowed merge methods, refuses a draft or an already-armed pull request without dispatching, picks the first repository-allowed method in squash/merge/rebase order, and calls `enablePullRequestAutoMerge`; GitLab has no separate switch, so it reads the merge request first, refuses a draft or an already-armed merge request, and otherwise issues the merge call flagged `merge_when_pipeline_succeeds`; because that call merges immediately when no pipeline is pending, the GitLab path is a merge authorization, so with no pending head pipeline the request is refused unless the body carries `confirmImmediateMerge: true`. The field must be a real JSON boolean -- coercing it would let any truthy value, notably the string `"false"`, read as consent -- and any other value is a 400. That refusal is raised as `ConfirmationRequired` and answers with `{error, confirmationRequired: true}`, which is what makes the guard live rather than a constant the client asserts: the dashboard's confirming click sends `false`, and only the server's own refusal escalates the panel to a third, explicitly-worded `Merge now` step that sends `true` and quotes the server's reason. Confirm and Cancel are separate buttons, with Cancel standing where the arming button was, so the second half of an accidental double-click backs out instead of authorizing a merge. The ready endpoint clears draft state and returns `{ready: true}`: `markPullRequestReadyForReview` on GitHub, and on GitLab the GraphQL `mergeRequestSetDraft(draft: false)` mutation, refusing when the merge request is not a draft (`draft`, or legacy `work_in_progress`). Neither ready path rewrites the title: the draft-prefix grammar stays the provider's concern, a concurrent retitle cannot be lost, and a title that merely begins with a draft-like word (`Drafting widgets`) is never mangled. GraphQL reports refusals in the body with HTTP 200, so every mutation response is inspected for transport-level `errors` and the per-mutation `errors` field and raises rather than reading as success. Both read the current draft/auto-merge state before mutating so an inapplicable action is a 400 instead of a provider error, and both invalidate the cache before dispatch on the same rule as resolve. The full payload carries `autoMerge` (GitHub `autoMergeRequest`, GitLab `merge_when_pipeline_succeeds`) so the panel renders armed auto-merge as state rather than an available action. Invalid URLs/thread IDs return 400; provider CLI, authentication, secure-spawn, audit, or direct-fetch-capacity failures return 503. Sidebar source-link extraction ignores every non-durable message role (`chunk`, `done`, `streaming`, `queued`, and `permission`) and indexes only durable message content, preventing partial output, queue placeholders, and permission prompts from scheduling credential-backed provider work. Sidebar provider refresh and cached `state`/`ci` serialization use the same read-only boundary: an exact configured-owner request, or a signed `local-app`/`local-startup` dashboard request when no owner is configured. Non-owner and app-token slot responses retain the source URL/provider/number but cannot trigger or observe credential-backed status. **Chat Folders**: GET `/api/chat/folders` (list project folders, each enriched with a computed non-persisted `history_count` — the authoritative on-disk archived-session count per folder from `ConversationLog.list_sessions()`), POST `/api/chat/folders` (create; body `{name, parent_id?, project_dir?}`, background LLM emoji-icon generation), PATCH `/api/chat/folders/{id}` (update — accepts `hidden` (bool) alongside `name`/`collapsed`/`order`/`default_agent`/`project_dir`/`icon`; moving or reviving a session into a folder auto-unhides it via `_unhide_folder`), DELETE `/api/chat/folders/{id}` (delete + ungroup its slots) Folders may be linked to a project directory (`project_dir`, validated server-side: absolute, existing, non-sensitive path): the dashboard resolves the effective directory by walking up the folder tree to the nearest ancestor with `project_dir` set (cycle-guarded), and "new chat in folder" carries the resolved directory in the slot-create flow so the session starts on the linked project. **Agents**: GET `/api/agents` (KiroCrew agent roster ordered **most-used-first** — reorders config agents + discovered project agents by `ConversationLog.agent_usage()` (turn count, then recency), falling back gracefully to config-insertion order on any failure so the dropdown never breaks or drops agents), GET `/api/agents/installed` (list all kiro-cli agents from `~/.kiro/agents/`, with `package` field extracted from filename), GET/DELETE `/api/agents/detail/{name}` (full agent config JSON; DELETE removes the config file, protected for kirocrew/kirocrew-lite) **Capability Integration** (edition-supplied operations-based `CapabilityManager` seam — the edition owns its CLI grammar, output parsing, and error translation; on a vanilla OSS install `CapabilityManager.available()` is `False` so every endpoint returns HTTP 503 `"capability manager not available"`): GET `/api/capability/mcp` (list installed MCP servers), POST `/api/capability/mcp/install` (install an MCP server, pushes `refresh("agents")`), POST `/api/capability/mcp/uninstall` (pushes `refresh("agents")`), GET `/api/capability/mcp/registry` (browse available MCP servers — the manager returns already-parsed entries, which the core passes through as `{"servers": [...]}`), GET `/api/capability/skills` (list installed skill packages), POST `/api/capability/skills/install` (install by `package` only — **no `version_set`**; the manager owns version/source resolution; regenerates agent config, pushes `refresh("agents")`; the manager returns human-friendly errors), POST `/api/capability/skills/uninstall` (pushes `refresh("agents")`), GET `/api/capability/agents` (list installed agent packages). The routes were renamed from the former `/api/aim/*` to neutral `/api/capability/*` vocab so no Amazon-internal name fossilizes in the fork's public API; the agent install/uninstall/update routes (`/api/aim/agents/install`, `/api/aim/agents/uninstall`, `/api/aim/update`) were **removed** with the pure-AIM code. @@ -581,7 +581,7 @@ React 18 + TypeScript + Vite 5 + Redux Toolkit + React Router v7 + Tailwind CSS ### UI Pages -- **Chat** (`/chat`, default) — multi-session parallel chat, Slack-style grouped messages with timestamps (MMM DD, YYYY, HH:MM), KiroCrew logo as assistant avatar, full Markdown rendering via `react-markdown` + `remark-gfm` + `rehype-raw` with Mermaid diagram support, syntax-highlighted code blocks (highlight.js), and clickable file paths (inline `` containing paths → reveal in Finder via `/api/reveal`), session sidebar with titles and scroll-shadow panels (notifications moved to dedicated page), collapsible history section (default collapsed) with source tags (🖥 dashboard / 💬 slack) and creation dates, session delete from history, `EmptyState` component when no session is active. The tabbed SidePanel includes a singleton Changes view when a conversation contains supported PR/MR URLs; it renders provider metadata, lazily expanded diffs, review discussions, and CI checks, while sidebar source chips show provider/number plus cached state and CI. For open non-draft sources the panel header surfaces a merge-blocker banner derived from the normalized merge-state fields (allow-list gated on the raw provider state being `open` (GitHub) or `opened` (GitLab), so merged/closed/locked/other states never banner): `mergeable === 'conflicting'` renders a danger "Merge conflicts" banner whose "Add to chat" handoff asks the agent to resolve conflicts — preferring a merge of base into head on shared branches and reserving rebase + `--force-with-lease` for unshared ones; `mergeStateStatus === 'need_rebase'` (GitLab fast-forward-only projects) renders a warning "Rebase required" banner whose handoff explains that a merge commit cannot unblock the MR and asks the agent to coordinate before rewriting a shared branch, rebasing with `--force-with-lease` otherwise; `mergeStateStatus === 'behind'` renders a warning "Branch is behind" banner whose handoff prescribes a no-history-rewrite update (merge base into head or the provider's update-branch affordance, never an unprompted force-push); and `mergeStateStatus === 'blocked'` renders a warning "Merge blocked" banner for branch-protection gates (human-actionable, no handoff). Frontend discovery ignores every `chunk`/`streaming` message regardless of its array position and publishes a URL only after that message becomes durable, so appended thinking/tool/stop events cannot settle a numeric URL that an earlier stream is still extending. Extraction, incremental index state, and `PullRequestPanel` rendering preserve first-seen order while enforcing the same 64-source cap. **First-mention attribution** decides Changes vs Resources: a PR/MR becomes a Changes *source* only when its FIRST durable mention was agent-authored (assistant / tool / thinking output — e.g. a `gh pr create` result). A PR whose first mention was a **user** message is treated as a referenced link and surfaces in the Files-tab **Resources** list instead — it never auto-opens the Changes tab or fires a new-source notification. Because the dedup map records the first mention, a later agent echo of a user-referenced PR cannot reclassify it as a Change (and a later user reference cannot demote an agent-surfaced one). The 64-source cap is applied **per role**, so a flood of user-referenced links cannot starve agent Changes sources; user-first links are retained (bounded) solely for echo suppression and are excluded from the emitted Changes sources. The emitted `PullRequestLink` shape is unchanged, so backend source APIs and the Changes panel are unaffected. **Files-tab inline file preview**: opening a file from the SidePanel **Files** tab shows it **inline** (the file list is replaced by the file's content plus a "Back to files" bar) via the shared `MarkdownPanel` editor — the same view / Edit-Preview toggle / dirty tracking / Save / discard-guard used by document tabs — instead of spawning a separate document tab. The inline working copy lives in a module-level draft store (`usePanelTabs`, keyed by slot + path — the same file edited in two chat slots keeps independent drafts; `usePanelTabs` owns the key format) that sits ABOVE the SidePanel subtree, so an in-progress edit survives everything that unmounts the panel (close control, activity-tab switch, chat-slot switch, and the automatic force-collapse on window resize) and is restored on reopen; it is in-memory (parity with document-tab content, which is likewise stripped on persist and re-read on reload) and is cleared on save and on explicit discard. Exactly **one editor per path** is enforced at open time (there is no runtime "yield" of an already-open editor): opening a path from the Files list that is already open as a `file:${path}` document tab focuses that tab instead of opening a second inline editor, and a chat-link open of a path already open inline routes back to the inline editor (`ChatPage.handleFileOpen`) rather than spawning a document tab. A failed read (HTTP error or network-level rejection) renders a retryable error rather than an editable placeholder (so a save can't overwrite the file with empty/placeholder content), and a successful save refreshes the shared `['file-read', path]` cache so a reopen never seeds stale pre-save content. Per-slot seen-source state is also mirrored to a quota-safe, globally bounded localStorage record, so route remounts and page reloads do not reinterpret historical links as new sources or override a persisted panel dismissal. During uncached slot switches or reloads, source reconciliation preserves the persisted Changes tab while `slotLoading` is true and only closes it after hydrated history confirms that the slot has no supported source URLs. Chat uses WS for streaming (`?ws=1` mode): POST returns immediately, chunks arrive via WebSocket. Auto-approved tool calls broadcast via WS as ephemeral cards (not persisted to messages), inserted before streaming message in Virtuoso list. **Agent selection**: WelcomeView (pre-first-message) has agent picker that sets `pendingAgent` state — on first send, slot is created with that agent via `POST /api/chat/slots {agent}`. Agent selector dropdown also in top bar next to session title for mid-session switching. Agent badge (aim-colored pill) in sidebar slot list. **MCP info button** shows per-agent MCP servers: non-kirocrew agents show only their own MCPs from agent config; kirocrew shows all global MCPs. **Tool/approval payload viewer** (`ToolDetails`, website SPA): tool-call and pending-approval cards render the payload with a **Raw / Formatted** toggle (beside the Input/Output control, shown only for JSON-ish payloads where the two modes differ). Formatted renders the parsed JSON object as a key→value table — multi-line command values show real line breaks and quotes (JSON-decoded), with bash syntax highlighting on command-bearing keys (`command`/`cmd`/`script`/`shell`/`bash`) via the shared worker highlighter; Raw shows the exact verbatim payload with escaping intact (and is the fallback for truncated/streaming or non-object payloads). +- **Chat** (`/chat`, default) — multi-session parallel chat, Slack-style grouped messages with timestamps (MMM DD, YYYY, HH:MM), KiroCrew logo as assistant avatar, full Markdown rendering via `react-markdown` + `remark-gfm` + `rehype-raw` with Mermaid diagram support, syntax-highlighted code blocks (highlight.js), and clickable file paths (inline `` containing paths → reveal in Finder via `/api/reveal`), session sidebar with titles and scroll-shadow panels (notifications moved to dedicated page), collapsible history section (default collapsed) with source tags (🖥 dashboard / 💬 slack) and creation dates, session delete from history, `EmptyState` component when no session is active. The tabbed SidePanel includes a singleton Changes view when a conversation contains supported PR/MR URLs; it renders provider metadata, lazily expanded diffs, review discussions, and CI checks, while sidebar source chips show provider/number plus cached state and CI. For open non-draft sources the panel header surfaces a merge-blocker banner derived from the normalized merge-state fields (allow-list gated on the raw provider state being `open` (GitHub) or `opened` (GitLab), so merged/closed/locked/other states never banner): `mergeable === 'conflicting'` renders a danger "Merge conflicts" banner whose "Add to chat" handoff asks the agent to resolve conflicts — preferring a merge of base into head on shared branches and reserving rebase + `--force-with-lease` for unshared ones; `mergeStateStatus === 'need_rebase'` (GitLab fast-forward-only projects) renders a warning "Rebase required" banner whose handoff explains that a merge commit cannot unblock the MR and asks the agent to coordinate before rewriting a shared branch, rebasing with `--force-with-lease` otherwise; `mergeStateStatus === 'behind'` renders a warning "Branch is behind" banner whose handoff prescribes a no-history-rewrite update (merge base into head or the provider's update-branch affordance, never an unprompted force-push); and `mergeStateStatus === 'blocked'` renders a warning "Merge blocked" banner for branch-protection gates (human-actionable, no handoff). The panel's own payload is pinned (`staleTime: Infinity`) and never refetches on its own, so a merge state that settles *after* the payload loaded is carried by the server-driven chip↔payload invalidation protocol rather than by any client-side comparison: the merge pair is part of both the chip-status projection and the `source_status` delta, so a chip refresh that observes a changed merge pair drops the full payload server-side and notifies owner dashboards, and every window refetches. Each merge field is recorded into the chip entry only once it is *real* — an unanswered field is omitted rather than written as `unknown` — and the two fields are recorded independently, because GitLab settles `need_rebase` and its branch-protection gates in the detail field while `mergeable` stays `unknown`; dropping the detail because its sibling is unknown would leave exactly those banners invisible. Omission alone is not enough for "still computing" to be harmless, though: every writer replaces the chip entry WHOLESALE, so an omitted field would ERASE a settled one rather than read as "no news" — and because both providers evaluate lazily, an unsettled read of an already-known conflict is the common case, not a rare one. Both writers (chip refresh and full-payload write-through) therefore carry a settled merge field forward when the fresh read has no answer, the same keep-known rule already applied to the `ci` glyph. A real answer always wins, including one that CHANGES the value, so the carry only fills a gap and cannot pin a stale verdict; it stops once the source leaves an open state, where the pair is meaningless and permanently unanswered. The full-payload projection (`status_from_full_payload`) must therefore project the merge pair too: a field the chip path records but the full-payload path omits would re-appear as a changed transition on every chip refresh and spin the invalidation loop, which the flap damper (`_CHECK_FLAP_DAMP_THRESHOLD`) bounds structurally. Without this the conflict banner appeared only once the user hit refresh. Frontend discovery ignores every `chunk`/`streaming` message regardless of its array position and publishes a URL only after that message becomes durable, so appended thinking/tool/stop events cannot settle a numeric URL that an earlier stream is still extending. Extraction, incremental index state, and `PullRequestPanel` rendering preserve first-seen order while enforcing the same 64-source cap. **First-mention attribution** decides Changes vs Resources: a PR/MR becomes a Changes *source* only when its FIRST durable mention was agent-authored (assistant / tool / thinking output — e.g. a `gh pr create` result). A PR whose first mention was a **user** message is treated as a referenced link and surfaces in the Files-tab **Resources** list instead — it never auto-opens the Changes tab or fires a new-source notification. Because the dedup map records the first mention, a later agent echo of a user-referenced PR cannot reclassify it as a Change (and a later user reference cannot demote an agent-surfaced one). The 64-source cap is applied **per role**, so a flood of user-referenced links cannot starve agent Changes sources; user-first links are retained (bounded) solely for echo suppression and are excluded from the emitted Changes sources. The emitted `PullRequestLink` shape is unchanged, so backend source APIs and the Changes panel are unaffected. **Files-tab inline file preview**: opening a file from the SidePanel **Files** tab shows it **inline** (the file list is replaced by the file's content plus a "Back to files" bar) via the shared `MarkdownPanel` editor — the same view / Edit-Preview toggle / dirty tracking / Save / discard-guard used by document tabs — instead of spawning a separate document tab. The inline working copy lives in a module-level draft store (`usePanelTabs`, keyed by slot + path — the same file edited in two chat slots keeps independent drafts; `usePanelTabs` owns the key format) that sits ABOVE the SidePanel subtree, so an in-progress edit survives everything that unmounts the panel (close control, activity-tab switch, chat-slot switch, and the automatic force-collapse on window resize) and is restored on reopen; it is in-memory (parity with document-tab content, which is likewise stripped on persist and re-read on reload) and is cleared on save and on explicit discard. Exactly **one editor per path** is enforced at open time (there is no runtime "yield" of an already-open editor): opening a path from the Files list that is already open as a `file:${path}` document tab focuses that tab instead of opening a second inline editor, and a chat-link open of a path already open inline routes back to the inline editor (`ChatPage.handleFileOpen`) rather than spawning a document tab. A failed read (HTTP error or network-level rejection) renders a retryable error rather than an editable placeholder (so a save can't overwrite the file with empty/placeholder content), and a successful save refreshes the shared `['file-read', path]` cache so a reopen never seeds stale pre-save content. Per-slot seen-source state is also mirrored to a quota-safe, globally bounded localStorage record, so route remounts and page reloads do not reinterpret historical links as new sources or override a persisted panel dismissal. During uncached slot switches or reloads, source reconciliation preserves the persisted Changes tab while `slotLoading` is true and only closes it after hydrated history confirms that the slot has no supported source URLs. Chat uses WS for streaming (`?ws=1` mode): POST returns immediately, chunks arrive via WebSocket. Auto-approved tool calls broadcast via WS as ephemeral cards (not persisted to messages), inserted before streaming message in Virtuoso list. **Agent selection**: WelcomeView (pre-first-message) has agent picker that sets `pendingAgent` state — on first send, slot is created with that agent via `POST /api/chat/slots {agent}`. Agent selector dropdown also in top bar next to session title for mid-session switching. Agent badge (aim-colored pill) in sidebar slot list. **MCP info button** shows per-agent MCP servers: non-kirocrew agents show only their own MCPs from agent config; kirocrew shows all global MCPs. **Tool/approval payload viewer** (`ToolDetails`, website SPA): tool-call and pending-approval cards render the payload with a **Raw / Formatted** toggle (beside the Input/Output control, shown only for JSON-ish payloads where the two modes differ). Formatted renders the parsed JSON object as a key→value table — multi-line command values show real line breaks and quotes (JSON-decoded), with bash syntax highlighting on command-bearing keys (`command`/`cmd`/`script`/`shell`/`bash`) via the shared worker highlighter; Raw shows the exact verbatim payload with escaping intact (and is the fallback for truncated/streaming or non-object payloads). - **Notifications** (`/notifications`) — dedicated page with left/right split layout. Left: category tabs (All/Cron/Hooks/Heartbeat/Agent/Approval/Subagent/Tasks), search filter, date-grouped list (Today/Yesterday/This Week/Older). Right: detail panel with source label, full timestamp, Read/Unread badge, markdown-rendered body, and jump-to-source buttons. Jump logic: `slot` meta → "💬 Go to Chat" (active tab) or "💬 Resume Chat" (from history); `slack_link` meta → "💬 Open in Slack" (deep link); `task_id` → "💬 Continue in Chat"; `job_id` → "⏰ View Cron Jobs". Cron notifications have `CronAckBar` for acknowledge/delete. Notification meta includes `slot` (subagent/heartbeat from dashboard), `slack_link` (subagent from Slack), `session_key` (webhook), `job_id` (cron), `task_id` (task runner). `_notif_meta()` helper on `GatewayOrchestrator` builds meta from `parent_key`. StatCard row: Total/Unread/Cron/Hooks/Heartbeat. Nav badge shows unread count. - **Overview** (`/overview`) — `StatCard` components (with skeleton loading) + tabbed management console: - **Memory tab**: editable preferences.md / projects.md with Save buttons, read-only daily history. **Memory Graph Explorer**: sigma.js (WebGL) visualization of memory relationships (nodes = memory entries color-coded by group; edges link entries to the projects they mention). Node positions come from a one-shot client-side d3-force layout (time-bounded, then stopped — no live physics solver); the server sends only nodes/edges. diff --git a/src/kiro_crew/dashboard/handlers/source_providers.py b/src/kiro_crew/dashboard/handlers/source_providers.py index e55a263575e..929ee964cf2 100644 --- a/src/kiro_crew/dashboard/handlers/source_providers.py +++ b/src/kiro_crew/dashboard/handlers/source_providers.py @@ -772,6 +772,89 @@ def _github_thread_map(payload: Any) -> dict[str, dict[str, Any]]: return result +# Both providers compute mergeability lazily: reading a pull request that has +# not been evaluated recently returns "not known yet" (GitHub ``UNKNOWN``, +# GitLab ``checking``/``unchecked``) *and* kicks off the computation, so the +# real answer is only available on a later read. A single read therefore reports +# a conflicting pull request as having no merge blocker at all — which is why +# the panel's conflict banner used to appear only once the user hit refresh. +# These bound a short re-read of the merge fields alone (not the whole fanout), +# issued concurrently with the secondary provider calls so most of the wait is +# absorbed by work the request was already doing. +_MERGE_STATE_REREADS = 2 +_MERGE_STATE_REREAD_DELAY_SECS = 0.8 +# The one normalized value that means "the provider has not answered yet". It is +# shared by both fields of the merge pair and by both providers. +_UNSETTLED_MERGE_STATE = "unknown" + + +def _merge_state_real(value: str) -> bool: + """Whether one normalized merge field carries a real answer.""" + return bool(value) and value != _UNSETTLED_MERGE_STATE + + +def _merge_state_settled(mergeable: str, merge_state: str) -> bool: + """Whether a normalized merge pair is a real answer, so no re-read is due. + + A pair is settled once **either** field is real. GitLab reports `need_rebase` + and its branch-protection gates with ``mergeable == 'unknown'`` — the detail + IS the answer there, so keying only on ``mergeable`` would re-read a state + the provider had already settled and then discard it. A pair that is empty + rather than unknown means the provider did not report the fields at all, so + re-reading cannot settle it either. + """ + if mergeable == _UNSETTLED_MERGE_STATE or merge_state == _UNSETTLED_MERGE_STATE: + return _merge_state_real(mergeable) or _merge_state_real(merge_state) + return True + + +_MERGE_STATE_FIELDS = ("mergeable", "mergeStateStatus") +# Lifecycle states for which a merge answer is still meaningful. Once a source is +# merged or closed the providers stop answering the merge pair at all, so a +# carried-forward value could never be cleared again. +_MERGE_STATE_LIVE_STATES = frozenset({"open", "draft"}) + + +def _keep_known_merge_state( + status: dict[str, str], previous: dict[str, str] | None +) -> dict[str, str]: + """Carry a settled merge field forward when a fresh read has no answer yet. + + ``_record_merge_state`` omits a field the provider has not settled, on the + principle that "still computing" must never be published as a real answer. + That is necessary but not sufficient: every writer replaces the chip entry + WHOLESALE, so an omitted field does not read as "no news" downstream — it + erases whatever the previous entry had settled. + + That matters because an unsettled read is the COMMON case, not a rare one: + both providers compute mergeability lazily, so a poll that arrives after the + provider's evaluation lapsed returns ``unknown`` for a source whose conflict + is already known. Without this carry-forward, such a poll drops the merge + pair, which (a) removes it from the owner-gated sidebar payload that spreads + the entry whole, and (b) reads as a CHANGED chip status, dropping the full + payload and emitting a delta — whose refetch re-projects the real answer + straight back into the cache. That is the repeating chip<->full transition + ``_CHECK_FLAP_DAMP_THRESHOLD`` exists to contain, so the banner would survive + only until the damper tripped and then go stale. + + Mirrors the same keep-known rule already applied to the ``ci`` glyph. A real + answer always wins, including one that CHANGES the value, so this only ever + fills a gap and cannot pin a stale verdict. Carry-forward stops once the + source leaves an open state, where the pair is both meaningless and + permanently unanswered. + """ + if not previous: + return status + if status.get("state", "open") not in _MERGE_STATE_LIVE_STATES: + return status + carried = { + field: previous[field] + for field in _MERGE_STATE_FIELDS + if field not in status and field in previous + } + return {**status, **carried} if carried else status + + def _github_merge_state(details: dict[str, Any]) -> tuple[str, str]: """Normalize GitHub merge fields to (mergeable, mergeStateStatus). @@ -839,6 +922,59 @@ def _gitlab_merge_state(details: dict[str, Any]) -> tuple[str, str]: return mergeable, "" +async def _github_settled_merge_state(ref: SourceRef, details: dict[str, Any]) -> tuple[str, str]: + """Merge state for a GitHub PR, re-reading while it is still being computed. + + Re-reads only ``mergeable``/``mergeStateStatus``, at most + ``_MERGE_STATE_REREADS`` times. A failed or still-unsettled re-read keeps the + original value rather than raising: an unknown merge state degrades one + banner, and must never fail the whole panel. + """ + mergeable, merge_state = _github_merge_state(details) + if _merge_state_settled(mergeable, merge_state): + return mergeable, merge_state + for _ in range(_MERGE_STATE_REREADS): + await asyncio.sleep(_MERGE_STATE_REREAD_DELAY_SECS) + try: + data = await _run_json( + "gh", "pr", "view", ref.url, "--json", "mergeable,mergeStateStatus" + ) + except SourceProviderError: + break + if not isinstance(data, dict): + break + reread, reread_state = _github_merge_state(data) + if _merge_state_settled(reread, reread_state): + return reread, reread_state + return mergeable, merge_state + + +async def _gitlab_settled_merge_state( + ref: SourceRef, mr_api: str, details: dict[str, Any] +) -> tuple[str, str]: + """Merge state for a GitLab MR, re-reading while it is still being computed. + + GitLab exposes ``detailed_merge_status`` only on the merge-request endpoint, + so the re-read repeats that request and takes the merge fields from it. Same + failure posture as the GitHub path: degrade to the original value. + """ + mergeable, merge_state = _gitlab_merge_state(details) + if _merge_state_settled(mergeable, merge_state): + return mergeable, merge_state + for _ in range(_MERGE_STATE_REREADS): + await asyncio.sleep(_MERGE_STATE_REREAD_DELAY_SECS) + try: + data = await _run_json("glab", "api", mr_api) + except SourceProviderError: + break + if not isinstance(data, dict): + break + reread, reread_state = _gitlab_merge_state(data) + if _merge_state_settled(reread, reread_state): + return reread, reread_state + return mergeable, merge_state + + async def _fetch_github(ref: SourceRef) -> dict[str, Any]: fields = ",".join( [ @@ -876,7 +1012,8 @@ async def _fetch_github(ref: SourceRef) -> dict[str, Any]: files_raw: Any review_comments_raw: Any review_threads_raw: Any - files_raw, review_comments_raw, review_threads_raw = await asyncio.gather( + merge_state_raw: Any + files_raw, review_comments_raw, review_threads_raw, merge_state_raw = await asyncio.gather( _run_json( "gh", "api", @@ -903,6 +1040,9 @@ async def _fetch_github(ref: SourceRef) -> dict[str, Any]: f"number={ref.number}", max_output_bytes=_DISCUSSION_OUTPUT_BYTES, ), + # Runs alongside the secondary calls so its re-read wait overlaps with + # fetches this request was making anyway. + _github_settled_merge_state(ref, details), return_exceptions=True, ) partial_sections: list[str] = [] @@ -970,7 +1110,11 @@ async def _fetch_github(ref: SourceRef) -> dict[str, Any]: } ) - github_mergeable, github_merge_state = _github_merge_state(details) + github_mergeable, github_merge_state = ( + merge_state_raw + if isinstance(merge_state_raw, tuple) + else _github_merge_state(details) + ) return { "provider": "github", "url": details.get("url") or ref.url, @@ -1027,17 +1171,23 @@ async def _fetch_gitlab(ref: SourceRef) -> dict[str, Any]: discussions_raw: Any changes_raw: Any pipelines_raw: Any - commits_raw, discussions_raw, changes_raw, pipelines_raw = await asyncio.gather( - _run_json("glab", "api", f"{mr_api}/commits?per_page={_SECONDARY_PAGE_SIZE}"), - _run_json( - "glab", - "api", - f"{mr_api}/discussions?per_page={_SECONDARY_PAGE_SIZE}", - max_output_bytes=_DISCUSSION_OUTPUT_BYTES, - ), - _run_json("glab", "api", f"{mr_api}/changes", max_output_bytes=_DIFF_OUTPUT_BYTES), - _run_json("glab", "api", f"{mr_api}/pipelines?per_page=20"), - return_exceptions=True, + merge_state_raw: Any + commits_raw, discussions_raw, changes_raw, pipelines_raw, merge_state_raw = ( + await asyncio.gather( + _run_json("glab", "api", f"{mr_api}/commits?per_page={_SECONDARY_PAGE_SIZE}"), + _run_json( + "glab", + "api", + f"{mr_api}/discussions?per_page={_SECONDARY_PAGE_SIZE}", + max_output_bytes=_DISCUSSION_OUTPUT_BYTES, + ), + _run_json("glab", "api", f"{mr_api}/changes", max_output_bytes=_DIFF_OUTPUT_BYTES), + _run_json("glab", "api", f"{mr_api}/pipelines?per_page=20"), + # Runs alongside the secondary calls so its re-read wait overlaps + # with fetches this request was making anyway. + _gitlab_settled_merge_state(ref, mr_api, details), + return_exceptions=True, + ) ) partial_sections: list[str] = [] for raw_value, section in ( @@ -1139,7 +1289,11 @@ async def _fetch_gitlab(ref: SourceRef) -> dict[str, Any]: } ) - gitlab_mergeable, gitlab_merge_state = _gitlab_merge_state(details) + gitlab_mergeable, gitlab_merge_state = ( + merge_state_raw + if isinstance(merge_state_raw, tuple) + else _gitlab_merge_state(details) + ) gitlab_checks = [_gitlab_check(item) for item in _as_list(jobs)] # The single CI glyph is projected from the pipeline AGGREGATE (authoritative # and lossless — GitLab folds allow_failure into it and marks a blocking @@ -2171,10 +2325,12 @@ def _clear_check_flap(url: str) -> None: def get_cached_check_status(url: str) -> dict[str, str] | None: - """Cached status for a PR url: {"ci": running|passed|failed, "state": ...}. + """Cached status for a PR url: {"ci": ..., "state": ..., "mergeable": ...}. - ``ci`` and ``state`` are each present only when known. Returns None until - the first background refresh completes. + Every key is present only when known. ``mergeable``/``mergeStateStatus`` are + omitted while the provider is still computing mergeability, so a client must + treat their absence as "no news" rather than "nothing blocks the merge". + Returns None until the first background refresh completes. """ entry = _check_cache.get(url) return entry[1] if entry else None @@ -2254,6 +2410,24 @@ def _emit_status_delta(url: str, status: dict[str, str], origin: str) -> None: sink(delta) +def _record_merge_state(result: dict[str, str], mergeable: str, merge_state: str) -> None: + """Add the merge fields to a chip-status entry, each only once it is real. + + An unanswered field is left out entirely rather than written as ``unknown``: + the chip cache is a short-TTL hint the client compares against its loaded + pull-request payload, and "still computing" must not read as a disagreement + with a real answer the payload already has. The two fields are recorded + independently because GitLab settles `need_rebase` and its branch-protection + gates in the detail field while ``mergeable`` stays ``unknown`` — dropping the + detail because its sibling is unknown would leave exactly those banners + invisible to the poll. + """ + if _merge_state_real(mergeable): + result["mergeable"] = mergeable + if _merge_state_real(merge_state): + result["mergeStateStatus"] = merge_state + + def status_from_full_payload(payload: dict[str, Any]) -> dict[str, str] | None: """Derive the lightweight chip status from a FULL pull-request payload. @@ -2291,6 +2465,17 @@ def status_from_full_payload(payload: dict[str, Any]) -> dict[str, str] | None: state = _project_state(str(payload.get("state") or ""), draft=bool(payload.get("draft"))) if state is not None: result["state"] = state + # The merge pair must be projected here too, not just by the chip read. If the + # write-through omitted it, every full fetch would rewrite the chip entry + # WITHOUT the fields the chip read had recorded, so the next chip refresh + # would see a "change" and drop the full payload, which would write-through + # and strip them again — the exact repeating chip↔full transition the flap + # damper below exists to contain, spun by nothing but a projection gap. + _record_merge_state( + result, + str(payload.get("mergeable") or ""), + str(payload.get("mergeStateStatus") or ""), + ) return result or None @@ -2321,6 +2506,11 @@ def record_full_payload_status(url: str, payload: dict[str, Any]) -> None: and "checks" in (payload.get("partialSections") or []) ): status = {**status, "ci": previous[1]["ci"]} + # Never let a lazily-unsettled merge read erase a settled one. A first full + # fetch commonly returns `unknown` (that is the bug this module's re-reads + # address), so without this the write-through would strip a conflict the chip + # cache already knew. + status = _keep_known_merge_state(status, previous[1] if previous else None) _check_cache[url] = (time.monotonic(), status) _trim_check_cache() if previous is None or previous[1] != status: @@ -2490,6 +2680,12 @@ async def _refresh_check_status(url: str, on_update: _CheckUpdateCallback | None latest = _check_cache.get(url) if latest is not previous: return + if status is not None: + # Same keep-known rule as the full-payload writer: an unsettled merge read + # must not erase a settled one, or this refresh would strip the pair, + # judge itself "changed", and drive the invalidation loop the flap damper + # below contains. + status = _keep_known_merge_state(status, previous[1] if previous else None) _check_cache[url] = (time.monotonic(), status) _trim_check_cache() changed = status is not None and (previous is None or previous[1] != status) @@ -2527,7 +2723,12 @@ async def _fetch_check_status(url: str) -> dict[str, str] | None: result: dict[str, str] = {} if ref.provider == "github": data = await _run_json( - "gh", "pr", "view", ref.url, "--json", "statusCheckRollup,state,isDraft" + "gh", + "pr", + "view", + ref.url, + "--json", + "statusCheckRollup,state,isDraft,mergeable,mergeStateStatus", ) if not isinstance(data, dict): return None @@ -2541,6 +2742,7 @@ async def _fetch_check_status(url: str) -> dict[str, str] | None: state = _project_state(raw_state, draft=bool(data.get("isDraft"))) if state is not None: result["state"] = state + _record_merge_state(result, *_github_merge_state(data)) return result or None project = quote(ref.project, safe="") details = await _run_json("glab", "api", f"projects/{project}/merge_requests/{ref.number}") @@ -2556,6 +2758,7 @@ async def _fetch_check_status(url: str) -> dict[str, str] | None: ) if state is not None: result["state"] = state + _record_merge_state(result, *_gitlab_merge_state(details)) pipelines = await _run_json( "glab", "api", f"projects/{project}/merge_requests/{ref.number}/pipelines?per_page=1" ) diff --git a/test/test_source_providers.py b/test/test_source_providers.py index 6f584ec7045..963045bdf96 100644 --- a/test/test_source_providers.py +++ b/test/test_source_providers.py @@ -930,6 +930,340 @@ async def fake_run(*argv: str, **_kwargs: int): assert data["partialSections"] == [expected_section] +MERGE_STATE_REREAD_FIELDS = "mergeable,mergeStateStatus" + + +@pytest.mark.asyncio +async def test_fetch_github_rereads_merge_state_until_the_provider_settles_it( + monkeypatch, +) -> None: + """GitHub computes mergeability lazily: the first read says UNKNOWN. + + Without the re-read the panel reports no merge blocker at all on first open, + and the conflict only surfaces once the user hits refresh. + """ + monkeypatch.setattr(source, "_MERGE_STATE_REREAD_DELAY_SECS", 0) + rereads: list[str] = [] + + async def fake_run(*argv: str, **_kwargs: int): + command = " ".join(argv) + if MERGE_STATE_REREAD_FIELDS in command: + rereads.append(command) + if len(rereads) == 1: + return {"mergeable": "UNKNOWN", "mergeStateStatus": "UNKNOWN"} + return {"mergeable": "CONFLICTING", "mergeStateStatus": "DIRTY"} + if "pr view" in command: + return {"number": 12, "mergeable": "UNKNOWN", "mergeStateStatus": "UNKNOWN"} + return {} if "graphql" in command else [] + + monkeypatch.setattr(source, "_run_json", fake_run) + + data = await source._fetch_github( + source.parse_source_url("https://github.com/acme/repo/pull/12") + ) + + assert (data["mergeable"], data["mergeStateStatus"]) == ("conflicting", "dirty") + # The re-read asks for the merge fields alone, not another full fanout. + assert len(rereads) == 2 + assert all("statusCheckRollup" not in command for command in rereads) + + +@pytest.mark.asyncio +async def test_fetch_github_does_not_reread_settled_merge_state(monkeypatch) -> None: + monkeypatch.setattr(source, "_MERGE_STATE_REREAD_DELAY_SECS", 0) + rereads: list[str] = [] + + async def fake_run(*argv: str, **_kwargs: int): + command = " ".join(argv) + if MERGE_STATE_REREAD_FIELDS in command: + rereads.append(command) + return {"mergeable": "CONFLICTING", "mergeStateStatus": "DIRTY"} + if "pr view" in command: + return {"number": 12, "mergeable": "MERGEABLE", "mergeStateStatus": "CLEAN"} + return {} if "graphql" in command else [] + + monkeypatch.setattr(source, "_run_json", fake_run) + + data = await source._fetch_github( + source.parse_source_url("https://github.com/acme/repo/pull/12") + ) + + assert (data["mergeable"], data["mergeStateStatus"]) == ("mergeable", "clean") + assert rereads == [] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("outcome", ["unsettled", "provider_error", "invalid_payload"]) +async def test_fetch_github_degrades_to_unknown_when_reread_cannot_settle( + monkeypatch, outcome: str +) -> None: + """A merge state that stays unknown degrades one banner, never the panel.""" + monkeypatch.setattr(source, "_MERGE_STATE_REREAD_DELAY_SECS", 0) + rereads: list[str] = [] + + async def fake_run(*argv: str, **_kwargs: int): + command = " ".join(argv) + if MERGE_STATE_REREAD_FIELDS in command: + rereads.append(command) + if outcome == "provider_error": + raise source.SourceProviderError("merge state read failed") + if outcome == "invalid_payload": + return [] + return {"mergeable": "UNKNOWN", "mergeStateStatus": "UNKNOWN"} + if "pr view" in command: + return {"number": 12, "title": "Still checking", "mergeable": "UNKNOWN"} + return {} if "graphql" in command else [] + + monkeypatch.setattr(source, "_run_json", fake_run) + + data = await source._fetch_github( + source.parse_source_url("https://github.com/acme/repo/pull/12") + ) + + assert data["mergeable"] == "unknown" + assert data["title"] == "Still checking" + # A failed or invalid re-read stops immediately; an unsettled one uses the + # whole bounded budget and no more. + assert len(rereads) == (source._MERGE_STATE_REREADS if outcome == "unsettled" else 1) + + +@pytest.mark.asyncio +async def test_fetch_github_skips_reread_when_provider_omits_merge_fields(monkeypatch) -> None: + """An absent field is not "still computing" — re-reading it would never settle.""" + monkeypatch.setattr(source, "_MERGE_STATE_REREAD_DELAY_SECS", 0) + rereads: list[str] = [] + + async def fake_run(*argv: str, **_kwargs: int): + command = " ".join(argv) + if MERGE_STATE_REREAD_FIELDS in command: + rereads.append(command) + return {} + if "pr view" in command: + return {"number": 12} + return {} if "graphql" in command else [] + + monkeypatch.setattr(source, "_run_json", fake_run) + + data = await source._fetch_github( + source.parse_source_url("https://github.com/acme/repo/pull/12") + ) + + assert data["mergeable"] == "" + assert rereads == [] + + +@pytest.mark.asyncio +async def test_fetch_gitlab_rereads_merge_state_until_the_provider_settles_it( + monkeypatch, +) -> None: + """GitLab reports ``checking``/``unchecked`` while it evaluates the MR.""" + monkeypatch.setattr(source, "_MERGE_STATE_REREAD_DELAY_SECS", 0) + detail_reads: list[str] = [] + + async def fake_run(*argv: str, **_kwargs: int): + command = " ".join(argv) + if command.endswith("merge_requests/42"): + detail_reads.append(command) + if len(detail_reads) == 1: + return {"iid": 42, "detailed_merge_status": "checking"} + return {"iid": 42, "detailed_merge_status": "conflict"} + return [] + + monkeypatch.setattr(source, "_run_json", fake_run) + + data = await source._fetch_gitlab( + source.parse_source_url("https://gitlab.com/acme/repo/-/merge_requests/42") + ) + + assert (data["mergeable"], data["mergeStateStatus"]) == ("conflicting", "dirty") + assert len(detail_reads) == 2 + + +@pytest.mark.asyncio +async def test_fetch_gitlab_degrades_to_unknown_when_reread_cannot_settle(monkeypatch) -> None: + monkeypatch.setattr(source, "_MERGE_STATE_REREAD_DELAY_SECS", 0) + detail_reads: list[str] = [] + + async def fake_run(*argv: str, **_kwargs: int): + command = " ".join(argv) + if command.endswith("merge_requests/42"): + detail_reads.append(command) + return {"iid": 42, "title": "Still checking", "detailed_merge_status": "unchecked"} + return [] + + monkeypatch.setattr(source, "_run_json", fake_run) + + data = await source._fetch_gitlab( + source.parse_source_url("https://gitlab.com/acme/repo/-/merge_requests/42") + ) + + assert data["mergeable"] == "unknown" + assert data["title"] == "Still checking" + assert len(detail_reads) == 1 + source._MERGE_STATE_REREADS + + +@pytest.mark.asyncio +async def test_github_check_status_carries_settled_merge_state(monkeypatch) -> None: + """The chip cache carries merge state so a conflict that appears while the + panel is open lands on a poll instead of waiting for a manual refresh.""" + + async def fake_run(*argv: str, **_kwargs: int): + assert "mergeable,mergeStateStatus" in " ".join(argv) + return {"state": "OPEN", "mergeable": "CONFLICTING", "mergeStateStatus": "DIRTY"} + + monkeypatch.setattr(source, "_run_json", fake_run) + + status = await source._fetch_check_status("https://github.com/acme/repo/pull/12") + + assert status == {"state": "open", "mergeable": "conflicting", "mergeStateStatus": "dirty"} + + +@pytest.mark.asyncio +@pytest.mark.parametrize("raw_mergeable", ["UNKNOWN", None]) +async def test_github_check_status_omits_unsettled_merge_state( + monkeypatch, raw_mergeable: str | None +) -> None: + """"Still computing" must not overwrite the answer the full payload has.""" + + async def fake_run(*_argv: str, **_kwargs: int): + return {"state": "OPEN", "mergeable": raw_mergeable, "mergeStateStatus": "UNKNOWN"} + + monkeypatch.setattr(source, "_run_json", fake_run) + + status = await source._fetch_check_status("https://github.com/acme/repo/pull/12") + + assert status == {"state": "open"} + + +@pytest.mark.asyncio +async def test_gitlab_check_status_carries_settled_merge_state(monkeypatch) -> None: + async def fake_run(*argv: str, **_kwargs: int): + command = " ".join(argv) + if command.endswith("merge_requests/42"): + return {"state": "opened", "detailed_merge_status": "conflict"} + return [] + + monkeypatch.setattr(source, "_run_json", fake_run) + + status = await source._fetch_check_status( + "https://gitlab.com/acme/repo/-/merge_requests/42" + ) + + assert status == {"state": "open", "mergeable": "conflicting", "mergeStateStatus": "dirty"} + + +@pytest.mark.asyncio +async def test_fetch_gitlab_treats_a_detail_only_answer_as_settled(monkeypatch) -> None: + """GitLab settles need_rebase with ``mergeable`` still ``unknown``. + + Keying settledness on ``mergeable`` alone re-read a state the provider had + already answered, then threw the answer away. + """ + monkeypatch.setattr(source, "_MERGE_STATE_REREAD_DELAY_SECS", 0) + detail_reads: list[str] = [] + + async def fake_run(*argv: str, **_kwargs: int): + command = " ".join(argv) + if command.endswith("merge_requests/42"): + detail_reads.append(command) + return {"iid": 42, "detailed_merge_status": "need_rebase"} + return [] + + monkeypatch.setattr(source, "_run_json", fake_run) + + data = await source._fetch_gitlab( + source.parse_source_url("https://gitlab.com/acme/repo/-/merge_requests/42") + ) + + assert (data["mergeable"], data["mergeStateStatus"]) == ("unknown", "need_rebase") + assert len(detail_reads) == 1 + + +@pytest.mark.parametrize( + ("pair", "settled"), + [ + (("conflicting", "dirty"), True), + (("mergeable", "clean"), True), + # GitLab: the detail is the answer, mergeable never settles. + (("unknown", "need_rebase"), True), + (("unknown", "blocked"), True), + # Nothing answered yet -> a re-read may settle it. + (("unknown", "unknown"), False), + (("unknown", ""), False), + # Provider omitted the fields -> re-reading cannot settle them. + (("", ""), True), + ], +) +def test_merge_state_settled_considers_both_fields( + pair: tuple[str, str], settled: bool +) -> None: + assert source._merge_state_settled(*pair) is settled + + +@pytest.mark.asyncio +async def test_gitlab_check_status_carries_a_detail_only_answer(monkeypatch) -> None: + """The rebase/blocked banners are driven by the detail field alone. + + Dropping it because ``mergeable`` is unknown left exactly those banners + invisible to the status poll. + """ + + async def fake_run(*argv: str, **_kwargs: int): + command = " ".join(argv) + if command.endswith("merge_requests/42"): + return {"state": "opened", "detailed_merge_status": "need_rebase"} + return [] + + monkeypatch.setattr(source, "_run_json", fake_run) + + status = await source._fetch_check_status( + "https://gitlab.com/acme/repo/-/merge_requests/42" + ) + + assert status == {"state": "open", "mergeStateStatus": "need_rebase"} + + +def test_status_from_full_payload_projects_the_merge_pair() -> None: + """The write-through must carry the merge pair the chip read records. + + If it dropped the pair, every full fetch would rewrite the chip entry without + it, the next chip refresh would judge that a change and drop the full payload, + and the write-through would strip it again — the repeating chip↔full + transition PR #443's flap damper exists to contain, spun by a projection gap. + """ + projected = source.status_from_full_payload( + { + "state": "OPEN", + "draft": False, + "checks": [{"bucket": "passed"}], + "mergeable": "conflicting", + "mergeStateStatus": "dirty", + } + ) + + assert projected == { + "ci": "passed", + "state": "open", + "mergeable": "conflicting", + "mergeStateStatus": "dirty", + } + + +def test_full_payload_and_chip_projections_agree_on_the_merge_pair() -> None: + """Both surfaces must derive the identical pair, or they flap against each other.""" + chip: dict[str, str] = {} + source._record_merge_state(chip, "unknown", "need_rebase") + projected = source.status_from_full_payload( + {"state": "opened", "mergeable": "unknown", "mergeStateStatus": "need_rebase"} + ) + + assert chip == {"mergeStateStatus": "need_rebase"} + assert projected is not None + assert { + key: value for key, value in projected.items() if key.startswith("merge") + } == chip + + @pytest.mark.asyncio async def test_refresh_check_status_queues_broadcast_only_when_status_changes(monkeypatch) -> None: url = "https://github.com/acme/repo/pull/12" @@ -3641,6 +3975,127 @@ def test_record_full_payload_preserves_ci_when_checks_partial() -> None: source._check_cache.clear() +def test_record_full_payload_keeps_settled_merge_state_when_the_read_is_unsettled() -> None: + """An unsettled merge read must not erase a settled one. + + Both providers compute mergeability lazily, so a full fetch whose evaluation + lapsed returns ``unknown`` for a source whose conflict is already known. + Because every writer replaces the chip entry WHOLESALE, an omitted field is + destructive rather than neutral: without the keep-known guard this write + strips the pair, which reads as a changed status and drives the + chip<->full invalidation loop. + """ + url = "https://github.com/acme/repo/pull/36" + source._check_cache.clear() + source._status_delta_sinks.clear() + sink = MagicMock() + source.register_status_delta_sink(sink) + source._check_cache[url] = ( + source.time.monotonic(), + {"state": "open", "mergeable": "conflicting", "mergeStateStatus": "dirty"}, + ) + try: + source.record_full_payload_status( + url, + {"state": "OPEN", "checks": [], "mergeable": "unknown", "mergeStateStatus": "unknown"}, + ) + + assert source.get_cached_check_status(url) == { + "state": "open", + "mergeable": "conflicting", + "mergeStateStatus": "dirty", + } + # Nothing changed once the known pair is carried over, so the loop that + # would otherwise refetch the payload never starts. + sink.assert_not_called() + finally: + source.unregister_status_delta_sink(sink) + source._check_cache.clear() + + +def test_record_full_payload_lets_a_real_merge_answer_replace_a_settled_one() -> None: + """Carry-forward fills a gap only — it must never pin a stale verdict.""" + url = "https://github.com/acme/repo/pull/37" + source._check_cache.clear() + source._check_cache[url] = ( + source.time.monotonic(), + {"state": "open", "mergeable": "conflicting", "mergeStateStatus": "dirty"}, + ) + try: + source.record_full_payload_status( + url, + {"state": "OPEN", "checks": [], "mergeable": "mergeable", "mergeStateStatus": "clean"}, + ) + + assert source.get_cached_check_status(url) == { + "state": "open", + "mergeable": "mergeable", + "mergeStateStatus": "clean", + } + finally: + source._check_cache.clear() + + +def test_record_full_payload_stops_carrying_merge_state_once_the_source_closes() -> None: + """A merged/closed source stops being asked about mergeability at all. + + Carrying the pair forward there would pin it permanently, because no later + read can ever supply a real answer to replace it. + """ + url = "https://github.com/acme/repo/pull/38" + source._check_cache.clear() + source._check_cache[url] = ( + source.time.monotonic(), + {"state": "open", "mergeable": "conflicting", "mergeStateStatus": "dirty"}, + ) + try: + source.record_full_payload_status(url, {"state": "MERGED", "checks": []}) + + assert source.get_cached_check_status(url) == {"state": "merged"} + finally: + source._check_cache.clear() + + +@pytest.mark.asyncio +async def test_chip_refresh_keeps_settled_merge_state_and_starts_no_invalidation_loop( + monkeypatch, +) -> None: + """The chip refresh path needs the same keep-known rule as the full writer. + + A settled conflict followed by an unsettled poll is the exact sequence that + made the pair vanish from the owner-gated sidebar payload (which spreads the + entry whole) and judged itself "changed", spinning the invalidation loop. + """ + url = "https://github.com/acme/repo/pull/39" + source._check_cache.clear() + source._check_inflight.clear() + source._status_delta_sinks.clear() + sink = MagicMock() + source.register_status_delta_sink(sink) + source._check_cache[url] = ( + source.time.monotonic(), + {"state": "open", "mergeable": "conflicting", "mergeStateStatus": "dirty"}, + ) + monkeypatch.setattr( + source, + "_fetch_check_status", + AsyncMock(return_value={"state": "open"}), + ) + try: + await source._refresh_check_status(url) + + assert source.get_cached_check_status(url) == { + "state": "open", + "mergeable": "conflicting", + "mergeStateStatus": "dirty", + } + sink.assert_not_called() + finally: + source.unregister_status_delta_sink(sink) + source._check_cache.clear() + source._check_inflight.clear() + + def test_record_full_payload_clears_ci_when_checks_genuinely_empty() -> None: """The guard must be scoped to PARTIAL checks, not merely-empty ones. diff --git a/website/src/pages/ChatSidebar.tsx b/website/src/pages/ChatSidebar.tsx index 6d09dfad811..b72e07eb098 100644 --- a/website/src/pages/ChatSidebar.tsx +++ b/website/src/pages/ChatSidebar.tsx @@ -240,6 +240,10 @@ interface Slot { url: string ci?: 'running' | 'passed' | 'failed' | null state?: 'open' | 'draft' | 'merged' | 'closed' + // Owner-gated chips spread the whole cached chip-status entry, which also + // carries the settled merge pair. Present only once the provider settled it. + mergeable?: string + mergeStateStatus?: string }> source_links_total?: number } diff --git a/website/src/test/pullRequestStatusDelta.test.ts b/website/src/test/pullRequestStatusDelta.test.ts index 762fcf31618..1bae684d5b0 100644 --- a/website/src/test/pullRequestStatusDelta.test.ts +++ b/website/src/test/pullRequestStatusDelta.test.ts @@ -74,4 +74,43 @@ describe('applyStatusDelta', () => { expect(stripped).toEqual({ url: URL_A }) expect(applyStatusDelta(batch, stripped!)).toBe(batch) }) + + // The merge pair rides the same event. A branch that starts conflicting while + // the panel is open changes only these fields, so a delta carrying nothing else + // still has to land -- otherwise the merge-blocker banner waits for a refresh. + it('carries a merge-only change', () => { + const delta = parseStatusDelta({ url: URL_A, mergeable: 'conflicting', mergeStateStatus: 'dirty' }) + + expect(delta).toEqual({ url: URL_A, mergeable: 'conflicting', mergeStateStatus: 'dirty' }) + expect(applyStatusDelta(batch, delta!)?.statuses[URL_A]) + .toEqual({ mergeable: 'conflicting', mergeStateStatus: 'dirty' }) + }) + + it('carries a GitLab detail-only answer, where mergeable never settles', () => { + const delta = parseStatusDelta({ url: URL_A, state: 'open', mergeStateStatus: 'need_rebase' }) + + expect(delta).toEqual({ url: URL_A, state: 'open', mergeStateStatus: 'need_rebase' }) + }) + + it('rejects merge values that are not the shape of the normalized vocabulary', () => { + const delta = parseStatusDelta({ + url: URL_A, + state: 'open', + mergeable: '', + mergeStateStatus: 'x'.repeat(33), + }) + + expect(delta).toEqual({ url: URL_A, state: 'open' }) + }) + + it('re-renders when only the merge pair moved', () => { + const conflicting = applyStatusDelta(batch, { + url: URL_A, state: 'open', ci: 'running', mergeable: 'conflicting', + }) + expect(conflicting).not.toBe(batch) + // ...and still skips the re-render when the whole entry is unchanged. + expect(applyStatusDelta(conflicting, { + url: URL_A, state: 'open', ci: 'running', mergeable: 'conflicting', + })).toBe(conflicting) + }) }) diff --git a/website/src/types/index.ts b/website/src/types/index.ts index e9f239793a5..2ceb307d29c 100644 --- a/website/src/types/index.ts +++ b/website/src/types/index.ts @@ -261,7 +261,7 @@ export interface TodoList { } export interface ChatSlot { - key: string; title?: string; messages: number; running: boolean; stopping?: boolean; pending_approval?: boolean; created?: string; last_ts?: string; last_message?: string; agent?: string; model?: string; reasoning_effort?: string; mode?: string; surface?: string; workspace?: string; trust?: boolean; trust_reads?: boolean; folder_id?: string; pinned?: boolean; tags?: string[]; slack_linked?: boolean; slack_channel?: string; slack_thread_ts?: string; color_index?: number | null; memory_mode?: 'persistent' | 'incognito' | 'temporary'; clean_mode?: boolean; project?: string; forked_from?: string | null; source_links?: { provider: 'github' | 'gitlab'; number: number; url: string; ci?: 'running' | 'passed' | 'failed' | null; state?: 'open' | 'draft' | 'merged' | 'closed' }[]; source_links_total?: number + key: string; title?: string; messages: number; running: boolean; stopping?: boolean; pending_approval?: boolean; created?: string; last_ts?: string; last_message?: string; agent?: string; model?: string; reasoning_effort?: string; mode?: string; surface?: string; workspace?: string; trust?: boolean; trust_reads?: boolean; folder_id?: string; pinned?: boolean; tags?: string[]; slack_linked?: boolean; slack_channel?: string; slack_thread_ts?: string; color_index?: number | null; memory_mode?: 'persistent' | 'incognito' | 'temporary'; clean_mode?: boolean; project?: string; forked_from?: string | null; source_links?: { provider: 'github' | 'gitlab'; number: number; url: string; ci?: 'running' | 'passed' | 'failed' | null; state?: 'open' | 'draft' | 'merged' | 'closed'; mergeable?: string; mergeStateStatus?: string }[]; source_links_total?: number /** Metadata for kind="webapp" artifacts (deploy state, architecture, costs). */ webapp_metadata?: WebAppMetadata // Board fields @@ -283,12 +283,18 @@ export interface PullRequestCheck { } /** Lightweight per-URL status used by wayfinding chips (sidebar + Changes tab - * strip). Both fields are present only when known: the backend serves them + * strip). Every field is present only when known: the backend serves them * from a short-TTL cache and refreshes in the background, so a freshly seen - * pull request has no status until a later poll. */ + * pull request has no status until a later poll. The merge fields are omitted + * while the provider is still computing mergeability, so their absence means + * "no news" — never "nothing blocks the merge". */ export interface PullRequestStatus { state?: 'open' | 'draft' | 'merged' | 'closed' ci?: 'running' | 'passed' | 'failed' + /** Normalized merge ability, same vocabulary as `PullRequestSource.mergeable`. */ + mergeable?: string + /** Normalized merge-state detail, same vocabulary as `PullRequestSource.mergeStateStatus`. */ + mergeStateStatus?: string } /** Response of the batched status endpoint. `refreshing` names the URLs whose diff --git a/website/src/utils/pullRequestStatusDelta.ts b/website/src/utils/pullRequestStatusDelta.ts index 05b7f52bf95..9d6b1794408 100644 --- a/website/src/utils/pullRequestStatusDelta.ts +++ b/website/src/utils/pullRequestStatusDelta.ts @@ -8,12 +8,27 @@ import type { PullRequestStatus, PullRequestStatusBatch } from '../types' * or turn-boundary refresh) or `'detail'` (a full fetch). The client refetches * the detail payload for both origins so every owner window converges; the tag * is retained for diagnostics and potential requester-aware routing later. + * + * The merge pair rides the same event, so a branch that starts conflicting while + * the panel is open invalidates the pinned detail payload like any other status + * change and the merge-blocker banner follows without a manual refresh. */ export interface SourceStatusDelta extends PullRequestStatus { url: string origin?: 'chip' | 'detail' } +/** Normalized merge-state values are compared for equality and switched on by + * the panel (an unrecognized one simply renders no banner), so they are shape- + * checked rather than enumerated — a value this client does not know yet stays + * harmless, and enumerating would silently drop it on version skew. */ +function mergeStateValue(value: unknown): string | undefined { + return typeof value === 'string' && value.length > 0 && value.length <= 32 + && /^[a-z_]+$/.test(value) + ? value + : undefined +} + /** Narrow an untrusted websocket payload to a usable delta. */ export function parseStatusDelta(data: unknown): SourceStatusDelta | null { if (!data || typeof data !== 'object') return null @@ -27,6 +42,10 @@ export function parseStatusDelta(data: unknown): SourceStatusDelta | null { } const ci = record.ci if (ci === 'running' || ci === 'passed' || ci === 'failed') delta.ci = ci + const mergeable = mergeStateValue(record.mergeable) + if (mergeable) delta.mergeable = mergeable + const mergeStateStatus = mergeStateValue(record.mergeStateStatus) + if (mergeStateStatus) delta.mergeStateStatus = mergeStateStatus const origin = record.origin if (origin === 'chip' || origin === 'detail') delta.origin = origin return delta @@ -49,12 +68,20 @@ export function applyStatusDelta( // no usable information. Treating it as authoritative would blank a populated // {state, ci} entry — worse than the stale glyph it would replace — so ignore // it and let the retained poll reconcile once vocabularies realign. - if (!delta.state && !delta.ci) return batch + if (!delta.state && !delta.ci && !delta.mergeable && !delta.mergeStateStatus) return batch const next: PullRequestStatus = {} if (delta.state) next.state = delta.state if (delta.ci) next.ci = delta.ci + if (delta.mergeable) next.mergeable = delta.mergeable + if (delta.mergeStateStatus) next.mergeStateStatus = delta.mergeStateStatus const current = batch.statuses?.[delta.url] - if (current && current.state === next.state && current.ci === next.ci) return batch + if ( + current + && current.state === next.state + && current.ci === next.ci + && current.mergeable === next.mergeable + && current.mergeStateStatus === next.mergeStateStatus + ) return batch return { ...batch, statuses: { ...batch.statuses, [delta.url]: next },