Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions context/db-migrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ schema migrations.
reader reloads the stored statistics (`internal/db/db.go::Optimize`).
- Migrations must never drop or rebuild `sqlite_stat1`, and a query-plan assertion
needs seeded rows plus `DB.Optimize` first (`internal/db/db.go::Optimize`).
- Lookups keyed by an unbounded ID list, such as pull-list enrichment, must not
expand the list into `IN (?, ?, ...)` placeholders: SQLite caps a statement at
32,766 bound variables. Bind the list once as a JSON array through
`json_each(?)`, or batch it
(`internal/db/queries_stacks.go::ListStackPlacementsForMRs`,
`internal/db/queries.go::GetWorktreeLinksForMRs`).

## Federation Spoke Preparation

Expand Down
7 changes: 5 additions & 2 deletions context/platform-sync-invariants.md
Original file line number Diff line number Diff line change
Expand Up @@ -500,10 +500,13 @@ Repository import requests and route/query shapes should carry
pushed-head refresh, manual workspace refresh, and on-demand worktree sync
must check item visibility before item-specific provider access. Workspace and
Fleet projections retain local records but omit removed parent metadata;
visible stack members are renumbered contiguously after filtering.
visible stack members are renumbered contiguously after filtering, and the
pull list's per-row stack placement must report the same position and size
as the detail stack context.
`inaccessible` items remain visible.
(`internal/server/pullapi/helpers.go::visibleMergeRequest`,
`internal/server/issueapi/mutation_handlers.go::requireVisibleIssue`)
`internal/server/issueapi/mutation_handlers.go::requireVisibleIssue`,
`internal/db/queries_stacks.go::ListStackPlacementsForMRs`)
- Embedded navigation events for repo-bound routes must publish identity from
parsed route state, not from global embed config. When a route carries repo
identity, event payloads should include `provider`, `platform_host`, and
Expand Down
8 changes: 8 additions & 0 deletions context/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,14 @@ the diff, so scope diff locators to `.diff-area`.
Pane tab headers are `role="tab"` with an `aria-label`, so use
`getByRole("tab", { name })` — `getByRole(..., { hasText })` is not a valid
option and silently matches every tab.
The mock Playwright config's 30 s timeout covers the whole test, and every
`page.goto` is a full Vite dev-server load that slows several-fold under CI's
14 workers; keep a test to two navigations or split it, or it fails on the
first attempt and only passes on retry. (`frontend/playwright.config.ts`)
Sidebar item rows are `<button>`s whose accessible name concatenates every
descendant label, including indicator `aria-label`s such as "Stacked: 2/7", so
a page-wide `getByRole("button", { name })` for a detail chip also matches the
row; scope chip assertions to the chip's test id.

Every `@lucide/svelte/icons/<name>` import added anywhere in `frontend/src`
must also be added to the `optimizeDeps.include` list in
Expand Down
17 changes: 17 additions & 0 deletions frontend/openapi/openapi.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions frontend/src/lib/api/generated/schema.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion frontend/src/lib/components/detail/StackStatus.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@
{expanded}
>
<Layers2Icon size={12} strokeWidth={2.3} aria-hidden="true" />
<span class="stack-chip-label">Stacked: {data.position}/{data.size}</span>
<span class="stack-chip-label">{data.position}/{data.size}</span>
{#if downstackBlockerCount > 0}
<span class="stack-chip-failure" aria-hidden="true">
<XIcon size={14} strokeWidth={2.8} /><span class="stack-chip-failure-count">{downstackBlockerCount}</span>
Expand Down
27 changes: 27 additions & 0 deletions frontend/src/lib/components/sidebar/PullItem.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import CITokenCluster, { composeAriaLabel } from "../shared/CITokenCluster.svelte";
import CircleAlertIcon from "@lucide/svelte/icons/circle-alert";
import CircleCheckBigIcon from "@lucide/svelte/icons/circle-check-big";
import Layers2Icon from "@lucide/svelte/icons/layers-2";
import OctagonXIcon from "@lucide/svelte/icons/octagon-x";
import LabelRow from "../shared/LabelRow.svelte";
import WorkspaceIndicator from "../shared/WorkspaceIndicator.svelte";
Expand Down Expand Up @@ -222,6 +223,16 @@
</svg>
</span>
{/if}
{#if pr.stack}
<span
class="stack-indicator"
aria-label={`Stacked: ${pr.stack.position}/${pr.stack.size}`}
title={`Stacked: ${pr.stack.position}/${pr.stack.size}`}
>
<Layers2Icon size={13} strokeWidth={2.2} aria-hidden="true" />
<span class="stack-indicator-count" aria-hidden="true">{pr.stack.position}/{pr.stack.size}</span>
</span>
{/if}
{#if parsed.error !== null}
<span
class="ci ci-unavailable"
Expand Down Expand Up @@ -440,6 +451,21 @@
color: var(--accent-red);
}

.stack-indicator {
display: inline-flex;
align-items: center;
gap: var(--space-1);
flex: 0 0 auto;
color: var(--text-muted);
}

.stack-indicator-count {
font-size: var(--font-size-2xs);
font-weight: 500;
font-variant-numeric: tabular-nums;
line-height: 1;
}

.ci-unavailable {
color: var(--state-warn, var(--accent-amber, #c08a2a));
opacity: 0.85;
Expand Down Expand Up @@ -553,6 +579,7 @@
:global(.mobile-main) .meta-text,
:global(.mobile-main) .time,
:global(.mobile-main) .worktree-name,
:global(.mobile-main) .stack-indicator-count,
:global(.mobile-main) .item-number,
:global(.mobile-main) .repo-name {
font-size: var(--font-size-sm);
Expand Down
14 changes: 14 additions & 0 deletions frontend/src/lib/components/sidebar/PullItem.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,20 @@ describe("PullItem kanban status", () => {
expect(screen.getByLabelText("Workspace attached (ready)")).toBeTruthy();
});

it("shows the stack position and size when the PR belongs to a stack", () => {
renderItem(mkPR({ stack: { position: 4, size: 7 } }));

const indicator = screen.getByLabelText("Stacked: 4/7");
expect(indicator.textContent?.trim()).toBe("4/7");
expect(indicator.querySelector("svg")).toBeTruthy();
});

it("hides the stack indicator when the PR is not stacked", () => {
renderItem(mkPR({}));

expect(screen.queryByLabelText(/^Stacked:/)).toBeNull();
});

it("shows an approved indicator when the PR is approved", () => {
renderItem(
mkPR({
Expand Down
41 changes: 38 additions & 3 deletions frontend/tests/e2e/stack-status.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,11 @@ async function mockStackedPR(
const method = route.request().method();

if (method === "GET" && pathname === "/api/v1/pulls") {
await fulfillJson(route, [pr]);
const currentStackMembers = options.stackMembers?.() ?? stackMembers;
const position = currentStackMembers.findIndex((member) => member.number === pr.Number) + 1;
const stack =
position > 0 && currentStackMembers.length > 1 ? { position, size: currentStackMembers.length } : undefined;
await fulfillJson(route, [{ ...pr, stack }]);
return;
}

Expand Down Expand Up @@ -447,6 +451,11 @@ test("stack status shares the PR detail expandable slot with CI", async ({ page

await page.goto("/pulls/github/acme/widgets/102");

const listStackIndicator = page.locator(".pull-item").getByLabel("Stacked: 2/7");
await expect(listStackIndicator).toHaveText("2/7");
await expect(page.getByTestId("stack-chip")).toContainText("2/7");
await expect(page.getByTestId("stack-chip")).not.toContainText("Stacked");

await page.getByTestId("ci-chip").click();
await expect(page.getByText("frontend / vp check")).toBeVisible();

Expand Down Expand Up @@ -537,15 +546,41 @@ test("stack status follows refreshed detail stack data", async ({ page }) => {
];
await emitPRDetailRefreshed(page, 102);

await expect(page.getByRole("button", { name: /Stacked: 2\/2/i })).toBeVisible();
await expect(page.getByRole("button", { name: /Stacked: 2\/7/i })).toHaveCount(0);
// Scope to the chip: the sidebar row button also exposes "Stacked: n/m"
// through its accessible name, so a page-wide role query would match it.
await expect(page.getByTestId("stack-chip")).toHaveAccessibleName(/Stacked: 2\/2/i);

currentStackMembers = [];
await emitPRDetailRefreshed(page, 102);

await expect(page.getByTestId("stack-chip")).toHaveCount(0);
});

test("phone pull rows fit the stack indicator beside the other indicators", async ({ page }) => {
await page.setViewportSize({ width: 390, height: 844 });
await mockStackedPR(page);

await page.goto("/m/pulls");
const row = page.locator(".mobile-shell .pull-item").first();
const indicator = row.getByLabel("Stacked: 2/7");
await expect(indicator).toHaveText("2/7");
// The fixture also renders the approved review, CI cluster, and time
// indicators, so the row exercises the full trailing cluster.
await expect(row.locator(".review-indicator--approved")).toBeVisible();
await expect(row.locator(".ci")).toBeVisible();

const rowBox = await row.boundingBox();
const indicatorBox = await indicator.boundingBox();
expect(rowBox).not.toBeNull();
expect(indicatorBox).not.toBeNull();
if (rowBox && indicatorBox) {
expect(rowBox.x + rowBox.width).toBeLessThanOrEqual(390);
expect(indicatorBox.x).toBeGreaterThanOrEqual(rowBox.x);
expect(indicatorBox.x + indicatorBox.width).toBeLessThanOrEqual(rowBox.x + rowBox.width);
}
expect(await page.evaluate(() => document.documentElement.scrollWidth)).toBeLessThanOrEqual(390);
});

test("stack status stays rendered while navigating to a stack member", async ({ page }) => {
let releaseStackResponse: () => void = () => {};
const delayedStackResponse = new Promise<void>((resolve) => {
Expand Down
12 changes: 11 additions & 1 deletion frontend/tests/e2e/workspace-sidebar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4689,7 +4689,10 @@ test.describe("workspace list bubble opens right sidebar", () => {
expect(maxRight - minRight).toBeLessThanOrEqual(1);
});

test("workspace recency uses item-list typography without phone overflow", async ({ page }) => {
// Desktop and phone parity are separate tests: each full navigation through
// the Vite dev server is slow under CI worker load, and four in one test
// exhausted the per-test budget on the first attempt.
test("workspace recency uses item-list typography on desktop", async ({ page }) => {
await setupTerminalMocks(page);
await page.addInitScript(() => {
localStorage.setItem("kenn-forge:workspaceListSort", "created");
Expand All @@ -4711,6 +4714,13 @@ test.describe("workspace list bubble opens right sidebar", () => {
return { fontFamily: style.fontFamily, fontSize: style.fontSize, lineHeight: style.lineHeight };
});
expect(desktopTypography).toEqual(pullTypography);
});

test("workspace recency uses item-list typography on phones without overflow", async ({ page }) => {
await setupTerminalMocks(page);
await page.addInitScript(() => {
localStorage.setItem("kenn-forge:workspaceListSort", "created");
});

await page.setViewportSize({ width: 390, height: 844 });
await page.goto("/m/pulls");
Expand Down
10 changes: 10 additions & 0 deletions internal/apiclient/generated/client.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

60 changes: 60 additions & 0 deletions internal/db/queries_stacks.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package db
import (
"context"
"database/sql"
"encoding/json"
"errors"
"fmt"
"strings"
Expand Down Expand Up @@ -360,6 +361,65 @@ func (d *DB) getStackForPRWhere(ctx context.Context, where string, args ...any)
return &stack, members, rows.Err()
}

// ListStackPlacementsForMRs returns the visible stack position and size for
// each merge request that belongs to a stack. Members hidden as removed
// upstream are excluded and the remaining members are renumbered contiguously,
// matching GetStackForPR. The IDs are bound once as a JSON array so the pull
// list size is not limited by SQLite's bind-parameter ceiling.
func (d *DB) ListStackPlacementsForMRs(ctx context.Context, mrIDs []int64) (map[int64]StackPlacement, error) {
placements := make(map[int64]StackPlacement)
if len(mrIDs) == 0 {
return placements, nil
}

payload, err := json.Marshal(mrIDs)
if err != nil {
return nil, fmt.Errorf("encode stack placement ids: %w", err)
}

rows, err := d.roQueryContext(ctx, `
WITH requested AS (
SELECT CAST(value AS INTEGER) AS merge_request_id FROM json_each(?)
),
visible AS (
SELECT sm.stack_id, sm.merge_request_id,
ROW_NUMBER() OVER (PARTITION BY sm.stack_id ORDER BY sm.position) AS position,
COUNT(*) OVER (PARTITION BY sm.stack_id) AS size
FROM forge_stack_members sm
JOIN forge_merge_requests p ON p.id = sm.merge_request_id
WHERE sm.stack_id IN (
SELECT stack_id FROM forge_stack_members
WHERE merge_request_id IN (SELECT merge_request_id FROM requested)
)
AND NOT EXISTS (
SELECT 1 FROM forge_archive_items ai
WHERE ai.repo_id = p.repo_id
AND ai.item_type = 'merge_request'
AND ai.item_number = p.number
AND ai.lifecycle_state = 'removed_upstream'
)
)
SELECT merge_request_id, position, size
FROM visible
WHERE merge_request_id IN (SELECT merge_request_id FROM requested)`,
string(payload),
)
if err != nil {
return nil, fmt.Errorf("list stack placements: %w", err)
}
defer rows.Close()

for rows.Next() {
var id int64
var placement StackPlacement
if err := rows.Scan(&id, &placement.Position, &placement.Size); err != nil {
return nil, fmt.Errorf("scan stack placement: %w", err)
}
placements[id] = placement
}
return placements, rows.Err()
}

// ListMRsBlockedByStackConflicts returns merge request IDs whose stack has an
// earlier non-merged dirty member. It is used by API response assembly to
// surface the stack-root conflict without mutating the provider-sourced PR row.
Expand Down
Loading
Loading