[hugocasa]

Summary

PR chips in the dashboard were colored only by CI status, so there was no way to tell a draft PR from one that is ready for review. This threads GitHub's isDraft flag through the stack and surfaces it in the UI.

The chip's outer border/background stays CI-driven — that color is the fastest read on whether a PR is green — and the draft signal is layered into the label segment instead.

Changes

• PrEntry gains isDraft: boolean in the domain model and the api-contract zod schema
• fetchAllPrs requests isDraft from gh pr list --json; parsePrResponse maps it
• readWorktreePrs accepts PR files written before this change and defaults them to isDraft: false, so stored merged/closed PRs don't disappear from the UI on upgrade
• New isDraftPr() helper; prStateTextClass / prBadgeClass now take the PR rather than just its state
• PrStatusGroup renders a muted DRAFT tag next to the PR number, with an "Open draft PR" tooltip
• PrBadge (sidebar) colors draft PRs grey; ready-for-review stays green, merged purple, closed red
• Tests for the parse path, the legacy-storage default, and the styling helpers

No CLI change: webmux list doesn't render PR state, it only uses prStates for sort ordering.

Screenshots

Captured from this branch running against real data — windmill-labs/windmill#10705 is genuinely a draft, windmilldocs#1675 is ready for review.

PR chip — draft vs ready for review. The DRAFT tag sits in the label segment; the shell border stays CI-driven (green in both).

draft	ready for review

Sidebar badge — draft is muted grey, ready for review stays green (merged stays purple).

draft (grey) + merged (purple)	ready for review (green)

Test plan

• bun run test — full suite passes (537 backend / 205 bin / 145 frontend / 4 contract)
• bun run --cwd backend check and bun run --cwd frontend check are clean
• Open the dashboard on a project with a draft PR: the chip shows DRAFT in muted text, sidebar badge is grey
• Mark that PR ready for review: within a sync cycle the tag disappears and the sidebar badge turns green
• Merged/closed PRs keep their purple/red colors

---

Generated with Claude Code

[claude]

Claude finished @hugocasa's task in 3m 20s —— View job

---

Code review — feat: show draft vs ready-for-review state on PR badges

• Read the PR diff
• Review backend changes (model, pr-service, fs adapter, contract schema)
• Review frontend changes (utils, PrBadge, PrStatusGroup)
• Verify parity across surfaces (frontend / CLI) and storage back-compat
• Post review

Overall this is a clean, well-scoped change: the flag is threaded end-to-end (gh JSON → domain model → zod contract → UI), the styling logic stays in one shared helper instead of being duplicated per component, and the legacy-storage default is both handled and tested. The decision to gate isDraftPr() on state === "open" is the right call — GitHub keeps isDraft: true on a closed draft, so without that gate a closed draft would render muted instead of red.

I could not run bun run test / check here (dependency install isn't permitted in this sandbox), so the findings below are from static reading only.

---

1. refreshStalePrData refreshes state but not isDraft — low

backend/src/services/pr-service.ts:412-424 handles worktrees whose PR wasn't returned by the open-PR list. It re-fetches state per entry and writes back { ...entry, state }, so isDraft keeps whatever was stored:

const state = await fetchPrState(entry.url);
return state ? { ...entry, state } : entry;

That's fine for the intended case (the PR left the list because it got merged/closed — isDraftPr then returns false regardless). It goes stale when fetchPrState still reports open, which happens whenever the list fetch missed the PR rather than the PR closing: a fetchAllPrs failure for that repo (the result.ok === false branch at line ~443 just logs and continues), or PR_FETCH_LIMIT truncation. In that window a draft that was marked ready-for-review keeps showing the DRAFT tag until the next successful sync.

Cheap fix — fetchPrState already spawns gh pr view --json state; ask for state,isDraft and merge both fields.

Fix this →

2. GhPrEntry.isDraft is typed as required but treated as optional — nit

backend/src/services/pr-service.ts:71 declares isDraft: boolean, while line 233 defends with entry.isDraft === true and backend/src/__tests__/pr.test.ts:131 explicitly asserts the missing-field case. Declaring it isDraft?: boolean makes the type match what the code and the test actually assume. (The existing updatedAt: string + entry.updatedAt ?? "" pair has the same shape, so this is consistency-neutral — take it or leave it.)

3. Draft rendering is only covered at the helper level — nit

frontend/src/lib/utils.test.ts covers prStateTextClass / prBadgeClass thoroughly, but nothing asserts that PrStatusGroup actually renders the DRAFT tag or that PrBadge gets the title. TopBar.test.ts shows component-level rendering tests are already set up, so a single render assertion would close the loop between the helper and the markup.

---

Things I checked and found correct

• CLI parity — confirmed: listWorktrees (bin/src/worktree-commands.ts:830-836) prints only name/status/profile-agent, and prStates feeds compareWorktreeOrder for sorting only. Nothing PR-state-related is rendered, so the "no CLI change" claim in the description holds (per AGENTS.md §3 this would otherwise be a blocker).
• Storage back-compat — isPrEntry accepting isDraft === undefined plus the ?? false map in readWorktreePrs means pre-upgrade prs.json files still validate rather than being dropped wholesale, which would have silently erased merged/closed history from the UI. Good catch, and worktree-storage.test.ts:487 pins it.
• Contract/model sync — PrEntrySchema and PrEntry both gained the field; PrEntrySchema isn't used for runtime response validation in packages/api-contract/src/client.ts, so making it required can't reject an older payload at the client.
• Chip layering — prStatusShellClass deliberately left untouched, so the CI-driven border/background is preserved and the draft signal only affects the label segment, matching the description.
• Merged/closed drafts — utils.test.ts:93-95 pins the state-wins ordering in both helpers.

Nothing here is blocking; item 1 is the only behavioral gap and it's a bounded, self-healing staleness window.
• branch feat/pr-draft-status

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	webmux	b92f92f	Commit Preview URL Branch Preview URL	Aug 10 2026, 05:20 PM

[hugocasa]

@claude please re-review — commit b92f92f addresses the three items from your first pass:

1. refreshStalePrData now refreshes isDraft alongside state (fetchPrStatus asks gh pr view --json state,isDraft, parsing split into an exported pure parsePrViewStatus).
2. GhPrEntry.isDraft is now optional, matching the === true defense.
3. Added frontend/src/lib/PrStatusGroup.test.ts covering the rendered DRAFT tag.

Focus on whether the stale-refresh merge is actually correct and whether the new tests pin the right behavior.

[claude]

Claude finished @hugocasa's task in 1m 32s —— View job

---

Re-review — commit b92f92f

• Read trigger comment and identify the three fixes to verify
• Review commit b92f92f diff
• Verify parsePrViewStatus / fetchPrStatus correctness
• Verify refreshStalePrData merge logic
• Verify GhPrEntry.isDraft optional change
• Verify new PrStatusGroup.test.ts coverage
• Post final review

All three items from the first pass are addressed correctly.

1. Stale-refresh merge — correct

backend/src/services/pr-service.ts:390-421 — fetchPrStatus now requests state,isDraft and delegates parsing to the exported pure parsePrViewStatus, which validates state is a string (rejecting malformed output) and normalizes isDraft via data.isDraft === true — the same defensive pattern already used in parsePrResponse (line 236), so a gh version that omits the field degrades to isDraft: false rather than throwing.

refreshStalePrData (line 427-438) merges with { ...entry, ...status }, so both state and isDraft get overwritten together from the same gh pr view call — no risk of the two fields getting out of sync with each other, which is the property that matters here (a single atomic read of both fields, rather than two separate fetches that could race). The entry.state !== "open" guard is unchanged, so already-terminal entries are still skipped, consistent with isDraftPr's gate on state === "open".

2. GhPrEntry.isDraft now optional — correct

Line 71: isDraft?: boolean, matching the entry.isDraft === true defense at line 236 (unchanged) and the new data.isDraft === true in parsePrViewStatus. Confirmed this only loosens the raw gh CLI response shape — the domain PrEntry.isDraft (backend/src/domain/model.ts:181) is still a required boolean, which is the right boundary: external/optional at the edge, guaranteed internally.

3. New test coverage — pins the right behavior

backend/src/__tests__/pr.test.ts:138-156 (parsePrViewStatus) covers: open+not-draft, open+draft, merged+not-draft, invalid JSON, and JSON missing state. Good coverage of the pure parser; the merge itself ({...entry, ...status}) is a one-liner so testing the parser is the right level — no need to also mock Bun.spawn/refreshStalePrData end-to-end for this.

frontend/src/lib/PrStatusGroup.test.ts closes the gap flagged in the first pass: it renders the component (not just the helper) and asserts the draft tag text and title attribute for draft/ready/merged-but-flagged-draft cases. The third case (state: "merged", isDraft: true) directly pins the same "closed/merged drafts don't render muted" behavior that isDraftPr's state gate protects — good, this was exactly the subtle behavior worth a regression test.

Nothing else outstanding. As before, I couldn't run bun run test/check in this sandbox (no node_modules, dependency install not permitted) — this review is from static reading of the diff only.
• branch feat/pr-draft-status

[claude]

Claude finished @hugocasa's task in 3m 15s —— View job

---

Code review — feat: show draft vs ready-for-review state on PR badges

• Read the full PR diff (git diff origin/main...HEAD)
• Review backend (model, pr-service, fs adapter, contract schema)
• Review frontend (utils, PrBadge, PrStatusGroup, tests)
• Verify parity, back-compat, and that isDraft survives every hop to the UI
• Post review

The feature is complete and correctly threaded end-to-end. I traced isDraft along the full path — gh pr list --json → parsePrResponse → writeWorktreePrs/readWorktreePrs → ProjectRuntime.setPrs → buildProjectSnapshot → buildAgentsUiWorktreeSummary → PrEntrySchema → components — and it survives every hop, because all three clonePrEntry-style mappers (snapshot-service.ts:24, project-runtime.ts:73, agents-ui-service.ts:26) spread rather than enumerate fields. Nothing blocking; three low-severity items below.

I could not run bun run test / check — this checkout has no node_modules and dependency install isn't permitted in the sandbox — so this is static reading only. The test-plan checkboxes in the description are still unticked; worth confirming the suite locally before merge.

---

1. The .every() guard doesn't actually narrow, so entry.isDraft is any — low

backend/src/adapters/fs.ts:354-355:

if (!Array.isArray(raw) || !raw.every((entry) => isPrEntry(entry))) return [];
return raw.map((entry) => ({ ...entry, isDraft: entry.isDraft ?? false }));

Array.isArray on an unknown narrows to any[], and the callback is an arrow returning boolean — not a type predicate — so no further narrowing happens. entry in the .map is any, which means the newly-widened isPrEntry return type (Omit<PrEntry, "isDraft"> & { isDraft?: boolean }) is decorative: the compiler never checks that the mapped object satisfies PrEntry, and entry.isDraft ?? false would typecheck even if the field were misspelled. backend/CLAUDE.md bans any, and this is the one spot in the diff where the guard is load-bearing for correctness.

Passing the guard by reference makes the declared type do real work:

const raw: unknown = await Bun.file(prsPath).json();
if (!Array.isArray(raw)) return [];
const entries: unknown[] = raw;
if (!entries.every(isPrEntry)) return [];
return entries.map((entry) => ({ ...entry, isDraft: entry.isDraft ?? false }));

The behavior is already correct and pinned by worktree-storage.test.ts:487; this only closes the typing hole. (The pre-existing return raw had the same weakness, so this isn't a regression — just the natural moment to fix it.)

Fix this →

2. parsePrViewStatus casts an unvalidated string into the state enum — low

backend/src/services/pr-service.ts:394-397 checks that state is a string, then casts data.state.toLowerCase() to PrEntry["state"] without checking it's one of open | closed | merged. The result is written straight to prs.json by refreshStalePrData.

The blast radius is larger than it looks: readWorktreePrs is all-or-nothing — one entry failing isPrEntry makes the whole file return [], wiping every PR for that worktree from the UI until the next successful sync. In practice gh only emits those three values, so this is theoretical, but you now have a tested pure function where the check is one line:

const state = data.state.toLowerCase();
if (state !== "open" && state !== "closed" && state !== "merged") return null;

That also removes the as cast, which backend/CLAUDE.md restricts to validated boundaries — and this is the validated boundary. Note parsePrResponse:235 has the identical cast, so fixing both keeps them consistent.

Fix this →

3. Sidebar draft signal is color-only — nit / product call

PrBadge.svelte distinguishes a draft purely by grey-vs-green plus a title tooltip, and grey is also prBadgeClass's fallback for anything non-open. The chip in PrStatusGroup gets a textual DRAFT tag; the sidebar gets nothing readable without hovering. If that asymmetry is deliberate (the sidebar badge is tiny and space-constrained), it's fine as-is — flagging it only because the screenshots show grey draft next to purple merged, which are hard to separate at a glance in a monochrome or low-vision context.

---

Verified correct

• Both first-pass items landed properly. fetchPrStatus reads state,isDraft in a single gh pr view, so refreshStalePrData's { ...entry, ...status } can't desync the two fields; GhPrEntry.isDraft is now optional at the raw-CLI boundary while the domain PrEntry.isDraft stays required — external-optional, internal-guaranteed is the right split.
• isDraftPr gating on state === "open" is the load-bearing detail: GitHub keeps isDraft: true on a closed draft, so without the gate a closed draft would render muted grey instead of red. utils.test.ts:93-95 and the third case in PrStatusGroup.test.ts both pin it.
• Legacy storage. isPrEntry accepting isDraft === undefined plus the ?? false default means pre-upgrade prs.json files still validate. Without this, every stored merged/closed PR would have silently vanished from the UI on upgrade — good catch, and worktree-storage.test.ts:487 pins it.
• CLI parity. Re-confirmed: bin/src/worktree-commands.ts only touches PrEntry["state"] (line 747, feeding sort order) and never reads or writes prs.json, so the "no CLI change" claim holds under AGENTS.md §3.
• No field-enumerating mappers. Checked all three PR clone sites and agents-ui-service; every one spreads, so no surface silently drops the flag. PrEntrySchema isn't used for runtime response validation on the client, so making the field required can't reject an older payload.
• Chip layering matches the description. prStatusShellClass is untouched, so the border/background stays CI-driven and draft only affects the label segment.
• New tests pin the right things. The PrStatusGroup render test closes the helper→markup gap from the first pass, and jest-dom matchers are already wired (used in TopBar.test.ts), so the new assertions will resolve.
  • branch feat/pr-draft-status