diff --git a/README.md b/README.md index 9641725..c67a1c3 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,24 @@ the listen port is `$PORT` (default 7930); the server always binds `127.0.0.1` - the board lists open, non-draft MRs authored by any configured member in one of `projects`. a left sidebar switches between **All** (the whole team) and a single member; the **All** view (and each member view) can be grouped by age / author / status / pipeline and sorted by oldest / pipeline / review progress. the current member, grouping, and sort live in the URL (shareable) and are remembered across visits. +## tabs + +each tab shows a filtered view of MRs sourced from one of two kinds. tabs have a unique id, a sidebar label, and optional overrides for slack and review routing. define tabs in your team settings (`rt settings set board.tabs --scope team`) as an array of tab objects. + +**no tabs config**: the board creates a single implicit "Team" tab sourcing MRs from team members (the `authors` kind). + +**two source kinds**: +- `"authors"`: MRs authored by configured team members (the default) +- `"codeowners"`: MRs blocked on approval from a specific codeowners section. section name comes from your repo's `.gitlab/codeowners` (e.g. `Acme`, `Billing`), and excludeMembers (when true) hides MRs authored by team members so the queue shows work assigned to the team, not self-reviews. no excludeMembers = show all MRs (the section's full queue including team-authored ones) + +a codeowners tab needs `@mattstack/rt-client` >= 0.5.0 in the board and an rt daemon running the sections-aware `project-mrs:read` handler; an older daemon reports no codeowner sections at all, so the tab just renders empty with no badge explaining why. + +**per-tab overrides**: +- `slackChannel`: posts/reactions for this tab go to a different channel (instead of config.slack.channel) +- `reviewSkill`: skill binding for review launches from this tab (instead of the manifest binding or empty fallback) + +use `rt settings set` to edit tabs on the team scope -- `config.json` carries them until then, and a settings-store edit needs a board restart (settings are boot-read, not watched). + ## tokens `bun run setup` handles both. `.env` becomes optional with the daemon fallback below, not retired -- an env var still wins first when it's set, `bun run setup` and `/peer/join` both still write to it (`SWITCHBOARD_TOKEN` in particular), and it stays the simplest path for a solo/local install with no rt daemon at all. under the hood: diff --git a/bun.lock b/bun.lock index 3bfe4ad..626d63b 100644 --- a/bun.lock +++ b/bun.lock @@ -6,7 +6,7 @@ "name": "mr-board", "dependencies": { "@mattstack/glance": "^0.19.0", - "@mattstack/rt-client": "^0.4.1", + "@mattstack/rt-client": "^0.5.0", "@mattstack/settings-kit": "^0.1.2", "@mattstack/tui-kit": "file:../tui-kit", "invadrs": "^0.2.0", @@ -42,7 +42,7 @@ "@mattstack/glance": ["@mattstack/glance@0.19.0", "", { "dependencies": { "@gitbeaker/rest": "^43.8.0", "@octokit/core": "^7.0.7", "@octokit/graphql": "^9.0.4", "@octokit/plugin-paginate-rest": "^15.0.0", "@octokit/plugin-retry": "^8.1.1", "@octokit/plugin-throttling": "^11.0.5", "@octokit/request-error": "^7.1.1" } }, "sha512-H+QMuyC3IZ3SXl8TpdjIKVhZvx+vP2ltVdt88CDz8e6YFponB2fplyOrHXLvGFITN1EnnEM6DQyYTcQ06F+7fg=="], - "@mattstack/rt-client": ["@mattstack/rt-client@0.4.1", "", { "dependencies": { "jsonc-parser": "^3.3.1" }, "peerDependencies": { "@mattstack/glance": ">=0.13.0" } }, "sha512-FFXGJWQeqlIb6t0gA9XkMN7Y0V86rIN8BOzTmU67rRKgjT/zrUl3nVFX5IjrTJh1uQgORog62+k4hvQNZ/RK4A=="], + "@mattstack/rt-client": ["@mattstack/rt-client@0.5.0", "", { "dependencies": { "jsonc-parser": "^3.3.1" }, "peerDependencies": { "@mattstack/glance": ">=0.13.0" } }, "sha512-wDwoq1okNAJA+JBlZhIxYSxcl1+O6xC4FQg98/RsXNoU4fHtPQzoLnWf+S7kbKNMnm6YpEn9JKK8+VYeIXtgPg=="], "@mattstack/settings-kit": ["@mattstack/settings-kit@0.1.2", "", { "peerDependencies": { "@mattstack/rt-client": "^0.4.1", "react": ">=18" }, "optionalPeers": ["react"] }, "sha512-OjeE8GwP2OSjt82kM+zfFDFfzyujWPD0Epg/wxq8+p7aIkMRMudLEMMWTe6VmEx4QPPT73kVxxHB1sBt0E6zbA=="], diff --git a/config.example.json b/config.example.json index c5ff6ca..3992a06 100644 --- a/config.example.json +++ b/config.example.json @@ -54,5 +54,8 @@ }, "switchboard": { "url": "" - } + }, + "tabs": [ + { "id": "team", "label": "Team", "source": { "kind": "authors" } } + ] } diff --git a/config.team.example.json b/config.team.example.json index 826fb53..c7066a7 100644 --- a/config.team.example.json +++ b/config.team.example.json @@ -35,5 +35,11 @@ "approved": "white_check_mark" } }, - "switchboard": { "url": "" } + "switchboard": { "url": "" }, + "tabs": [ + { "id": "team", "label": "Team", "source": { "kind": "authors" } }, + { "id": "codeowner-queue", "label": "Codeowner Queue", + "source": { "kind": "codeowners", "section": "Acme", "excludeMembers": true }, + "slackChannel": "team-codeowners" } + ] } diff --git a/package.json b/package.json index 48c84eb..ce6e3bd 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ }, "dependencies": { "@mattstack/glance": "^0.19.0", - "@mattstack/rt-client": "^0.4.1", + "@mattstack/rt-client": "^0.5.0", "@mattstack/settings-kit": "^0.1.2", "@mattstack/tui-kit": "file:../tui-kit", "invadrs": "^0.2.0", diff --git a/src/__tests__/board.test.ts b/src/__tests__/board.test.ts index 401a5f1..0540344 100644 --- a/src/__tests__/board.test.ts +++ b/src/__tests__/board.test.ts @@ -1,8 +1,20 @@ import { describe, expect, test } from "bun:test"; import type { PullRequest } from "@mattstack/glance"; -import { aggregateSyncScope, boardDemand, buildBoard, buildRoster, projectPathFromWebUrl, stripDraftPrefix, type BoardMR } from "../data.ts"; +import { + aggregateSyncScope, + boardDemand, + buildBoard, + buildRoster, + channelForMR, + configuredSlackChannels, + projectPathFromWebUrl, + reviewSkillForTab, + stripDraftPrefix, + visibleMrsFor, + type BoardMR, +} from "../data.ts"; import { SnapshotCache, type FetchResult } from "../cache.ts"; -import { DEFAULT_SLACK_EMOJI, type BoardConfig } from "../config.ts"; +import { DEFAULT_SLACK_EMOJI, IMPLICIT_TABS, type BoardConfig, type TabConfig } from "../config.ts"; import { extractTicketId } from "../ticket.ts"; const config: BoardConfig = { @@ -32,8 +44,16 @@ const config: BoardConfig = { emoji: DEFAULT_SLACK_EMOJI, }, switchboard: { url: "" }, + tabs: IMPLICIT_TABS, }; +/** A team tab plus one codeowners tab watching "Acme" -- used by the + boardDemand and tagged-row buildBoard tests below. */ +const tabsWithCodeowners: TabConfig[] = [ + { id: "t", label: "T", source: { kind: "authors" } }, + { id: "q", label: "Q", source: { kind: "codeowners", section: "Acme" } }, +]; + function pr(overrides: Partial): PullRequest { return { id: "gitlab:1", @@ -258,6 +278,88 @@ describe("buildBoard", () => { }); }); +describe("buildBoard tagged rows (codeowner tabs)", () => { + const withTabs: BoardConfig = { ...config, tabs: tabsWithCodeowners }; + const now = Date.parse("2026-07-11T00:00:00Z"); + + test("keeps a tagged stranger and stamps codeownerSections", () => { + const stranger = pr({ id: "gitlab:900", iid: 9, author: { id: "gitlab:99", username: "outsider", name: "Outsider", avatarUrl: null } }); + const tags = new Map([[stranger.id, ["Acme"]]]); + const out = buildBoard([stranger], withTabs, now, tags); + expect(out).toHaveLength(1); + expect(out[0]!.codeownerSections).toEqual(["Acme"]); + }); + + test("stamps codeownerSections on a roster member's own tagged MR too", () => { + // Mirrors fetchMemberMRs' scoped refresh: the row is kept on isMember + // alone, but a tags map must still be passed for codeownerSections to + // land -- an omitted map (server.ts's prior bug) silently zeroes it. + const memberMr = pr({ id: "gitlab:904", iid: 13 }); // default author: alice, a roster member + const tags = new Map([[memberMr.id, ["Acme"]]]); + const out = buildBoard([memberMr], withTabs, now, tags); + expect(out).toHaveLength(1); + expect(out[0]!.codeownerSections).toEqual(["Acme"]); + }); + + test("still drops an untagged stranger, and tag-kept rows skip the prefix filter", () => { + const withPrefixes: BoardConfig = { ...withTabs, ticketPrefixes: ["CV"] }; + // untagged stranger -> dropped + const untaggedStranger = pr({ id: "gitlab:901", iid: 10, author: { id: "gitlab:100", username: "ghost", name: "Ghost", avatarUrl: null } }); + // tagged stranger with no ticket prefix while ticketPrefixes=["CV"] -> kept + const taggedNoPrefix = pr({ + id: "gitlab:902", + iid: 11, + author: { id: "gitlab:101", username: "outsider", name: "Outsider", avatarUrl: null }, + sourceBranch: "no-ticket", + title: "no ticket here", + }); + // tagged MR from a section no tab declares -> dropped + const taggedWrongSection = pr({ + id: "gitlab:903", + iid: 12, + author: { id: "gitlab:102", username: "outsider2", name: "Outsider2", avatarUrl: null }, + sourceBranch: "no-ticket", + title: "no ticket either", + }); + const tags = new Map([ + [taggedNoPrefix.id, ["Acme"]], + [taggedWrongSection.id, ["OtherSection"]], + ]); + const out = buildBoard([untaggedStranger, taggedNoPrefix, taggedWrongSection], withPrefixes, now, tags); + expect(out.map((m) => m.iid)).toEqual([11]); + }); +}); + +describe("visibleMrsFor (the /data.json payload gate)", () => { + test("keeps a tagged stranger's MR, drops a hidden member's own MR", () => { + const withHiddenAndTabs: BoardConfig = { + ...config, + members: [{ username: "alice" }, { username: "carol", hidden: true }], + tabs: tabsWithCodeowners, + }; + const now = Date.parse("2026-07-11T00:00:00Z"); + const hiddenMemberMr = pr({ + id: "gitlab:910", + iid: 20, + author: { id: "gitlab:110", username: "carol", name: "Carol", avatarUrl: null }, + }); + const taggedStranger = pr({ + id: "gitlab:911", + iid: 21, + author: { id: "gitlab:111", username: "outsider", name: "Outsider", avatarUrl: null }, + }); + const tags = new Map([[taggedStranger.id, ["Acme"]]]); + const snapshotMrs = buildBoard([hiddenMemberMr, taggedStranger], withHiddenAndTabs, now, tags); + // Both reach the snapshot -- buildBoard doesn't know about "hidden"; only + // visibleMrsFor (the /data.json gate) does. + expect(snapshotMrs.map((m) => m.iid)).toEqual([20, 21]); + + const visibleMembers = withHiddenAndTabs.members.filter((m) => !m.hidden); + const served = visibleMrsFor(snapshotMrs, visibleMembers); + expect(served.map((m) => m.iid)).toEqual([21]); + }); +}); + describe("buildRoster", () => { const members = [{ username: "alice" }, { username: "bob", name: "Bobby" }, { username: "carol" }]; @@ -293,6 +395,81 @@ describe("boardDemand", () => { expect(d.authors).toEqual(["a", "b"]); // hidden is a display state, not a demand state expect(d.declaredAt).toBeGreaterThan(0); }); + + test("declares the union of tab sections, and omits the field when no tab is codeowners", () => { + const withTabs: BoardConfig = { ...config, tabs: tabsWithCodeowners }; + expect(boardDemand(withTabs, 1).codeownerSections).toEqual(["Acme"]); + expect(boardDemand(config, 1).codeownerSections).toBeUndefined(); + }); +}); + +describe("channelForMR", () => { + const tabsWithSlackChannel: TabConfig[] = [ + { id: "t", label: "T", source: { kind: "authors" } }, + { id: "q", label: "Q", source: { kind: "codeowners", section: "Acme", excludeMembers: true }, slackChannel: "team-codeowners" }, + ]; + const withSlackTab: BoardConfig = { ...config, tabs: tabsWithSlackChannel }; + + test("routes a roster MR to the default channel", () => { + expect( + channelForMR(withSlackTab, { author: { id: "gitlab:1", username: "alice", name: "Alice", avatarUrl: null }, codeownerSections: [] }), + ).toBe("code-review"); + }); + + test("routes a tagged stranger to their codeowners tab's channel", () => { + expect( + channelForMR(withSlackTab, { + author: { id: "gitlab:2", username: "outsider", name: "Outsider", avatarUrl: null }, + codeownerSections: ["Acme"], + }), + ).toBe("team-codeowners"); + }); + + test("falls back to the default channel when a stranger's tags match no tab's slackChannel", () => { + expect( + channelForMR(withSlackTab, { + author: { id: "gitlab:3", username: "outsider", name: "Outsider", avatarUrl: null }, + codeownerSections: ["SomeOtherSection"], + }), + ).toBe("code-review"); + }); + + test("a stranger with no tags falls back to the default channel", () => { + expect( + channelForMR(withSlackTab, { author: { id: "gitlab:4", username: "outsider", name: "Outsider", avatarUrl: null }, codeownerSections: [] }), + ).toBe("code-review"); + }); +}); + +describe("configuredSlackChannels", () => { + test("is the default channel plus every distinct tab slackChannel", () => { + const withTabs: BoardConfig = { + ...config, + tabs: [ + { id: "t", label: "T", source: { kind: "authors" } }, + { id: "q", label: "Q", source: { kind: "codeowners", section: "Acme" }, slackChannel: "team-codeowners" }, + { id: "r", label: "R", source: { kind: "codeowners", section: "Billing" }, slackChannel: "team-codeowners" }, + ], + }; + expect(configuredSlackChannels(withTabs).sort()).toEqual(["code-review", "team-codeowners"]); + }); + + test("is just the default channel when no tab overrides it", () => { + expect(configuredSlackChannels(config)).toEqual(["code-review"]); + }); +}); + +describe("reviewSkillForTab", () => { + test("prefers the tab's reviewSkill and falls back to normal resolution", () => { + const cfg = { ...config, tabs: [ + { id: "t", label: "T", source: { kind: "authors" as const } }, + { id: "q", label: "Q", source: { kind: "codeowners" as const, section: "Acme" }, reviewSkill: "external:review" }, + ] }; + const fallback = () => "acme:review"; + expect(reviewSkillForTab(cfg, "q", "u", fallback)).toBe("external:review"); + expect(reviewSkillForTab(cfg, "t", "u", fallback)).toBe("acme:review"); + expect(reviewSkillForTab(cfg, undefined, "u", fallback)).toBe("acme:review"); + }); }); describe("aggregateSyncScope", () => { @@ -302,7 +479,12 @@ describe("aggregateSyncScope", () => { }); test("no reads yields null syncedAt/windowDays and an empty uncovered list", () => { - expect(aggregateSyncScope([])).toEqual({ dataSyncedAt: null, scopeUncovered: [], scopeWindowDays: null }); + expect(aggregateSyncScope([])).toEqual({ + dataSyncedAt: null, + scopeUncovered: [], + scopeWindowDays: null, + scopeUncoveredSections: [], + }); }); test("unions scope.uncovered across reads and takes the min windowDays", () => { @@ -319,12 +501,20 @@ describe("aggregateSyncScope", () => { expect(agg.scopeWindowDays).toBeNull(); expect(agg.scopeUncovered).toEqual([]); }); + + test("unions uncoveredSections", () => { + const agg = aggregateSyncScope([ + { syncedAt: 1, scope: { authors: [], windowDays: 30, uncovered: [], sections: [], uncoveredSections: ["Acme"] } }, + { syncedAt: 2 }, + ]); + expect(agg.scopeUncoveredSections).toEqual(["Acme"]); + }); }); /** Wrap a bare mrs array as the FetchResult shape SnapshotCache now expects, for tests that only care about the mrs field. */ function fetchResult(mrs: unknown[]): FetchResult { - return { mrs: mrs as BoardMR[], dataSyncedAt: null, scopeUncovered: [], scopeWindowDays: null }; + return { mrs: mrs as BoardMR[], dataSyncedAt: null, scopeUncovered: [], scopeWindowDays: null, scopeUncoveredSections: [] }; } describe("SnapshotCache", () => { diff --git a/src/__tests__/cache.test.ts b/src/__tests__/cache.test.ts index 9b42831..fb579a9 100644 --- a/src/__tests__/cache.test.ts +++ b/src/__tests__/cache.test.ts @@ -3,7 +3,7 @@ import { SnapshotCache, type FetchResult } from "../cache.ts"; /** Wrap a bare mrs array as the FetchResult shape SnapshotCache expects. */ function fetchResult(mrs: unknown[]): FetchResult { - return { mrs: mrs as FetchResult["mrs"], dataSyncedAt: null, scopeUncovered: [], scopeWindowDays: null }; + return { mrs: mrs as FetchResult["mrs"], dataSyncedAt: null, scopeUncovered: [], scopeWindowDays: null, scopeUncoveredSections: [] }; } describe("SnapshotCache forced-refresh failure", () => { diff --git a/src/__tests__/config-store-latch.test.ts b/src/__tests__/config-store-latch.test.ts index 7bfdbd0..e9aa013 100644 --- a/src/__tests__/config-store-latch.test.ts +++ b/src/__tests__/config-store-latch.test.ts @@ -199,6 +199,17 @@ describe("loadConfigFrom: config.json-optional boot once the team store owns the }); }); +describe("loadConfigFrom: board.tabs overlay", () => { + test("board.tabs store value overlays config.json", () => { + const p = tmpConfig(); + const resolve = fakeResolve({ + "board.tabs": [{ id: "q", label: "Q", source: { kind: "codeowners", section: "Acme", excludeMembers: true }, slackChannel: "team-codeowners" }], + }); + const cfg = loadConfigFrom(p, resolve); + expect(cfg.tabs[0]!.id).toBe("q"); + }); +}); + describe("saveMemberHidden: latch-gated writer", () => { test("unowned: writes config.json's inline hidden flag, store untouched", () => { const p = tmpConfig({ ...base, members: [{ username: "alice" }, { username: "bob" }] }); diff --git a/src/__tests__/config.test.ts b/src/__tests__/config.test.ts index 16f5b47..7777e08 100644 --- a/src/__tests__/config.test.ts +++ b/src/__tests__/config.test.ts @@ -107,6 +107,25 @@ describe("parseConfig", () => { expect(parseConfig(JSON.stringify(base)).rtRepos).toEqual({}); }); + test("config without tabs gets the implicit authors tab", () => { + const cfg = parseConfig(JSON.stringify(base)); + expect(cfg.tabs).toEqual([{ id: "team", label: "Team", source: { kind: "authors" } }]); + }); + + test("throws on an empty tabs array instead of silently producing a zero-tab board", () => { + expect(() => parseConfig(JSON.stringify({ ...base, tabs: [] }))).toThrow(/tabs.*must not be empty/); + }); + + test("tabs validate: unique ids, codeowners needs a section", () => { + expect(() => parseConfig(JSON.stringify({ ...base, tabs: [ + { id: "a", label: "A", source: { kind: "codeowners" } }, + ] }))).toThrow(/section/); + expect(() => parseConfig(JSON.stringify({ ...base, tabs: [ + { id: "a", label: "A", source: { kind: "authors" } }, + { id: "a", label: "B", source: { kind: "authors" } }, + ] }))).toThrow(/duplicate tab id/); + }); + test("port, host, reviewSkill, respondSkill, teamClone are gone from the parsed shape", () => { const cfg = parseConfig(JSON.stringify({ ...base, port: 9999, host: "0.0.0.0", reviewSkill: "x:review", respondSkill: "x:respond", teamClone: "~/team", diff --git a/src/__tests__/selection.test.ts b/src/__tests__/selection.test.ts index aef4c21..1c008c5 100644 --- a/src/__tests__/selection.test.ts +++ b/src/__tests__/selection.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect } from "bun:test"; -import { selectionOf, postableOf } from "../selection.ts"; +import { selectionOf, postableOf, tabChangeClearsSelection } from "../selection.ts"; const a = { webUrl: "https://gl/a", iid: 1 }; const b = { webUrl: "https://gl/b", iid: 2 }; @@ -48,3 +48,19 @@ describe("postableOf", () => { expect(postableOf([orphan, a])).toEqual([a]); }); }); + +describe("tabChangeClearsSelection", () => { + test("an actual tab-id change clears the selection", () => { + expect(tabChangeClearsSelection({ tab: "q" }, "team")).toBe(true); + }); + + test("re-sending the already-active tab id is not a change", () => { + expect(tabChangeClearsSelection({ tab: "team" }, "team")).toBe(false); + }); + + test("a member/group/sort-only patch (no tab key) never clears it", () => { + expect(tabChangeClearsSelection({ member: "bob" }, "team")).toBe(false); + expect(tabChangeClearsSelection({ group: "status" }, "team")).toBe(false); + expect(tabChangeClearsSelection({ sort: "progress" }, "team")).toBe(false); + }); +}); diff --git a/src/__tests__/server-slack-channel.test.ts b/src/__tests__/server-slack-channel.test.ts new file mode 100644 index 0000000..ede3828 --- /dev/null +++ b/src/__tests__/server-slack-channel.test.ts @@ -0,0 +1,94 @@ +import { afterAll, expect, test } from "bun:test"; +import { join } from "path"; +import { mkdirSync, mkdtempSync, writeFileSync } from "fs"; +import { tmpdir } from "os"; + +// Proves /slack/resolve and /slack/post reject a `channel` body value outside +// the configured set (config.slack.channel + every tab's slackChannel, see +// configuredSlackChannels in data.ts) with a 400 before ever touching Slack +// or GitLab. Real (non-fixture) boot -- fixture mode always reports slack as +// unconfigured (server.ts's getSlackToken short-circuits to null under +// BOARD_FIXTURE), which would hit the earlier "slack not configured" 400 +// before ever reaching the channel check this test targets. A fake $HOME +// with a team settings store (mirrors server-healthz-fast.test.ts) plus a +// non-empty SLACK_TOKEN gets past that gate without a real Slack workspace; +// the channel-validation 400 fires before either handler calls cache.get() +// or makes any Slack API call, so no live GitLab/Slack connectivity is +// needed for this assertion to hold. +const fakeHome = mkdtempSync(join(tmpdir(), "board-slack-channel-")); + +const teamDir = join(fakeHome, ".mattstack", "teams", "testteam", "mattstack"); +mkdirSync(teamDir, { recursive: true }); +writeFileSync( + join(teamDir, "settings.team.jsonc"), + JSON.stringify({ + "board.gitlabHost": "https://gitlab.example.com", + "board.projects": ["g/p"], + "board.members": [{ username: "alice" }], + }), +); + +const PORT = 47945; +const proc = Bun.spawn(["bun", "run", join(import.meta.dir, "..", "server.ts")], { + env: { + ...process.env, + HOME: fakeHome, + // Keeps the booted server from writing state/board-port into the repo. + BOARD_APP_ROOT: fakeHome, + PORT: String(PORT), + GITLAB_TOKEN: "", + // Truthy but fake: only needs to clear the `!slackToken` gate. Both + // endpoints reject an unconfigured `channel` before making any real + // Slack API call, so this token is never actually used over the wire. + SLACK_TOKEN: "fake-slack-token", + SWITCHBOARD_TOKEN: "", + SWITCHBOARD_ADMIN_TOKEN: "", + }, + stdout: "pipe", + stderr: "pipe", +}); + +afterAll(() => proc.kill()); + +async function ready(): Promise { + for (let i = 0; i < 100; i++) { + try { + if ((await fetch(`http://127.0.0.1:${PORT}/healthz`)).ok) return; + } catch { + // server not listening yet + } + await new Promise((r) => setTimeout(r, 100)); + } + throw new Error("server never came up"); +} + +test("/slack/resolve rejects a channel outside the configured set", async () => { + await ready(); + const res = await fetch(`http://127.0.0.1:${PORT}/slack/resolve`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + mrUrl: "https://gitlab.example.com/g/p/-/merge_requests/1", + iid: 1, + channel: "not-a-real-channel", + }), + }); + expect(res.status).toBe(400); + // "code-review" is the only configured channel (DEFAULT_SLACK.channel; no + // board.tabs override in this fake store) -- named in the error body. + expect(await res.text()).toContain("code-review"); +}, 15_000); + +test("/slack/post rejects a channel outside the configured set", async () => { + await ready(); + const res = await fetch(`http://127.0.0.1:${PORT}/slack/post`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + mrUrls: ["https://gitlab.example.com/g/p/-/merge_requests/1"], + channel: "not-a-real-channel", + }), + }); + expect(res.status).toBe(400); + expect(await res.text()).toContain("code-review"); +}, 15_000); diff --git a/src/__tests__/server-slack-post-channel.test.ts b/src/__tests__/server-slack-post-channel.test.ts new file mode 100644 index 0000000..be42150 --- /dev/null +++ b/src/__tests__/server-slack-post-channel.test.ts @@ -0,0 +1,229 @@ +import { afterAll, expect, test } from "bun:test"; +import { join } from "path"; +import { mkdirSync, mkdtempSync, writeFileSync } from "fs"; +import { tmpdir } from "os"; + +// Proves /slack/post's channel derivation (server.ts's targetChannel logic, +// right after `picked` resolves): with no explicit body channel, a single +// tagged-stranger MR posts into its codeowners tab's slackChannel rather than +// the board's default channel, and a multi-MR post whose picked MRs resolve to +// different channels 400s instead of guessing one. Real (non-fixture) boot -- +// fixture mode answers /data.json from a canned file and refuses every POST +// outright, so it can't exercise this path at all. A fake rt daemon (unix +// socket, mirrors server-healthz-fast.test.ts's pattern) serves the picked +// MRs so cache.get() resolves without any real GitLab connectivity, and +// slack-api-mock-preload.ts (loaded via `bun --preload`) answers slack.com's +// API locally so resolveSlackRef's channel-name lookup never leaves the box. +// The mock gives each channel name its own channel id (see CHANNEL_IDS +// there), so the returned permalink's channel id is a fingerprint for which +// channel name the derivation actually picked. +const fakeHome = mkdtempSync(join(tmpdir(), "board-slack-post-channel-")); + +const teamDir = join(fakeHome, ".mattstack", "teams", "testteam", "mattstack"); +mkdirSync(teamDir, { recursive: true }); +writeFileSync( + join(teamDir, "settings.team.jsonc"), + JSON.stringify({ + "board.gitlabHost": "https://gitlab.example.com", + "board.projects": ["g/p"], + "board.members": [{ username: "alice" }], + "board.tabs": [ + { id: "team", label: "Team", source: { kind: "authors" } }, + { + id: "acme", + label: "Acme", + source: { kind: "codeowners", section: "Acme" }, + slackChannel: "acme-channel", + }, + { + id: "other", + label: "Other", + source: { kind: "codeowners", section: "OtherSection" }, + slackChannel: "other-channel", + }, + ], + }), +); + +// board.rtRepos is machine-scoped (registry-defs.ts), not team-scoped -- a +// team-store value for it is silently ignored, which would leave +// fetchTeamMRs with no daemon mapping for "g/p" and every MR dropped before +// buildBoard ever saw them. +writeFileSync(join(fakeHome, ".mattstack", "machine-key"), "testmachine"); +const machineDir = join(fakeHome, ".mattstack", "user", "local", "testmachine"); +mkdirSync(machineDir, { recursive: true }); +writeFileSync( + join(machineDir, "settings.local.jsonc"), + JSON.stringify({ + "board.rtRepos": [{ project: "g/p", repo: "gitlab.example.com/g/p" }], + }), +); + +const GITLAB_HOST = "https://gitlab.example.com"; +const acmeMrUrl = `${GITLAB_HOST}/g/p/-/merge_requests/501`; +const otherSectionMrUrl = `${GITLAB_HOST}/g/p/-/merge_requests/502`; + +function fakePr(overrides: Record): Record { + return { + id: overrides.id, + iid: overrides.iid, + repositoryId: "gitlab:42", + title: "An MR", + description: null, + state: "opened", + draft: false, + conflicts: false, + webUrl: overrides.webUrl, + sourceBranch: "feat/x", + targetBranch: "main", + createdAt: "2026-08-01T00:00:00Z", + updatedAt: new Date().toISOString(), + sha: null, + author: overrides.author, + assignees: [], + reviewers: [], + roles: [], + pipeline: null, + unresolvedThreadCount: 0, + approvalsLeft: 1, + approved: false, + approvedBy: [], + diffStats: null, + detailedMergeStatus: null, + autoMergeEnabled: false, + autoMergeStrategy: null, + mergeUser: null, + mergeAfter: null, + divergedCommitsCount: null, + rebaseInProgress: false, + mergeOngoing: false, + inProgressMergeCommitSha: null, + mergeError: null, + shouldBeRebased: false, + mergeabilityChecks: [], + blockingMergeRequestsCount: 0, + approvalsRequired: 1, + squash: false, + squashOnMerge: false, + mergeTrainIndex: null, + }; +} + +const acmePr = fakePr({ + id: "gitlab:501", + iid: 501, + webUrl: acmeMrUrl, + author: { id: "gitlab:901", username: "outsider1", name: "Outsider One", avatarUrl: null }, +}); +const otherSectionPr = fakePr({ + id: "gitlab:502", + iid: 502, + webUrl: otherSectionMrUrl, + author: { id: "gitlab:902", username: "outsider2", name: "Outsider Two", avatarUrl: null }, +}); + +const rtDir = join(fakeHome, ".mattstack", "rt"); +mkdirSync(rtDir, { recursive: true }); + +// Every mapped project's opened MRs, tagged the way the daemon reports a +// codeowners match -- see fetchTeamMRs' `entry.codeownerSections`. Every +// other command answers `ok:false` immediately (never hangs), which is what +// lets the token round trips (board-secrets.ts) degrade to "not configured" +// without a wedged wait. +const rtDaemon = Bun.serve({ + unix: join(rtDir, "rt.sock"), + fetch(req) { + const { pathname } = new URL(req.url); + if (pathname === "/project-mrs:read") { + return new Response( + JSON.stringify({ + ok: true, + data: { + mrs: { + "gitlab:501": { pr: acmePr, fetchedAt: Date.now(), codeownerSections: ["Acme"] }, + "gitlab:502": { pr: otherSectionPr, fetchedAt: Date.now(), codeownerSections: ["OtherSection"] }, + }, + listSyncedAt: Date.now(), + source: "poll", + syncedAt: Date.now(), + }, + }), + { headers: { "content-type": "application/json" } }, + ); + } + return new Response(JSON.stringify({ ok: false, error: "not implemented" }), { + headers: { "content-type": "application/json" }, + }); + }, +}); + +const PORT = 47946; +const proc = Bun.spawn( + [ + "bun", + "run", + "--preload", + join(import.meta.dir, "slack-api-mock-preload.ts"), + join(import.meta.dir, "..", "server.ts"), + ], + { + env: { + ...process.env, + HOME: fakeHome, + BOARD_APP_ROOT: fakeHome, + PORT: String(PORT), + GITLAB_TOKEN: "", + SLACK_TOKEN: "fake-slack-token", + SWITCHBOARD_TOKEN: "", + SWITCHBOARD_ADMIN_TOKEN: "", + // Consumed by slack-api-mock-preload.ts's conversations.history stub. + SLACK_MOCK_MR_URL: acmeMrUrl, + }, + stdout: "pipe", + stderr: "pipe", + }, +); + +afterAll(() => { + proc.kill(); + rtDaemon.stop(true); +}); + +async function ready(): Promise { + for (let i = 0; i < 100; i++) { + try { + if ((await fetch(`http://127.0.0.1:${PORT}/healthz`)).ok) return; + } catch { + // server not listening yet + } + await new Promise((r) => setTimeout(r, 100)); + } + throw new Error("server never came up"); +} + +test("/slack/post with no explicit channel derives a tagged stranger's MR to its tab's slackChannel", async () => { + await ready(); + const res = await fetch(`http://127.0.0.1:${PORT}/slack/post`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ mrUrls: [acmeMrUrl] }), + }); + expect(res.status).toBe(200); + const body = (await res.json()) as { ok: boolean; linked?: boolean; permalink?: string }; + expect(body.ok).toBe(true); + // C_ACME is "acme-channel"'s id in the mock (see CHANNEL_IDS in + // slack-api-mock-preload.ts) -- proves the post targeted the tab's channel, + // not C_DEFAULT ("code-review", config.slack.channel). + expect(body.permalink).toContain("C_ACME"); +}, 15_000); + +test("/slack/post with no explicit channel 400s when picked MRs span different channels", async () => { + await ready(); + const res = await fetch(`http://127.0.0.1:${PORT}/slack/post`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ mrUrls: [acmeMrUrl, otherSectionMrUrl] }), + }); + expect(res.status).toBe(400); + expect(await res.text()).toBe("MRs span Slack channels; post them per tab"); +}, 15_000); diff --git a/src/__tests__/slack-api-mock-preload.ts b/src/__tests__/slack-api-mock-preload.ts new file mode 100644 index 0000000..e0ee30c --- /dev/null +++ b/src/__tests__/slack-api-mock-preload.ts @@ -0,0 +1,56 @@ +// Preloaded (via `bun --preload`) into the subprocess server.ts boots by +// server-slack-post-channel.test.ts, so slack.ts's real `https://slack.com/api/*` +// calls resolve locally instead of over the network. Each mocked channel name +// maps to its own distinguishable channel id, so a test can tell which channel +// name the caller resolved purely from the id embedded in the response. +const realFetch = globalThis.fetch; + +const CHANNEL_IDS: Record = { + "code-review": "C_DEFAULT", + "acme-channel": "C_ACME", + "other-channel": "C_OTHER", +}; + +function ok(data: Record): Response { + return new Response(JSON.stringify({ ok: true, ...data }), { + status: 200, + headers: { "content-type": "application/json" }, + }); +} + +function slackApi(method: string, params: Record): Response { + switch (method) { + case "auth.test": + return ok({ url: "https://mockteam.slack.com/" }); + case "conversations.list": + return ok({ + channels: Object.entries(CHANNEL_IDS).map(([name, id]) => ({ id, name })), + response_metadata: { next_cursor: "" }, + }); + case "conversations.history": { + // SLACK_MOCK_MR_URL: the one MR url this test's channel index should + // already show a review-request message for. + const mrUrl = process.env.SLACK_MOCK_MR_URL; + return ok({ + messages: mrUrl ? [{ ts: "100.000001", user: "U1", text: `please review ${mrUrl}` }] : [], + has_more: false, + }); + } + case "reactions.get": + return ok({ message: { reactions: [] } }); + case "chat.postMessage": + return ok({ ts: "200.000001" }); + default: + return ok({}); + } +} + +globalThis.fetch = (async (input: Parameters[0], init?: Parameters[1]) => { + const url = typeof input === "string" ? input : input instanceof URL ? input.href : (input as Request).url; + if (!url.startsWith("https://slack.com/api/")) return realFetch(input as never, init); + const method = url.slice("https://slack.com/api/".length).split("?")[0]!; + const params: Record = init?.body + ? (JSON.parse(init.body as string) as Record) + : Object.fromEntries(new URL(url).searchParams); + return slackApi(method, params); +}) as typeof fetch; diff --git a/src/__tests__/slack.test.ts b/src/__tests__/slack.test.ts index 640f454..742f96e 100644 --- a/src/__tests__/slack.test.ts +++ b/src/__tests__/slack.test.ts @@ -1,13 +1,22 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, existsSync, writeFileSync } from "fs"; +import { join } from "path"; +import { tmpdir } from "os"; import { buildPermalink, buildThreadPermalink, extractMrUrls, matchReviewMessage, slackRefPath, + slackIndexPath, + legacyIndexPath, + readIndex, + writeIndex, + adoptLegacyIndex, attachSlack, type SlackMessage, type SlackRef, + type SlackIndex, } from "../slack.ts"; const URL_A = "https://gitlab.com/acme/webapp/-/merge_requests/4821"; @@ -78,6 +87,89 @@ describe("slackRefPath", () => { }); }); +describe("slackIndexPath", () => { + test("is stable per channel and distinct across channels", () => { + expect(slackIndexPath("code-review")).not.toBe(slackIndexPath("team-codeowners")); + expect(slackIndexPath("code-review")).toBe(slackIndexPath("code-review")); + }); + + test("slugs unsafe characters and lives under the given dir", () => { + expect(slackIndexPath("pod/weird name!", "/s")).toBe("/s/slack-index-pod-weird-name-.json"); + }); +}); + +describe("readIndex / writeIndex per-channel", () => { + let dir: string; + beforeEach(() => { dir = mkdtempSync(join(tmpdir(), "slack-idx-")); }); + afterEach(() => { rmSync(dir, { recursive: true, force: true }); }); + + test("readIndex never touches the legacy file -- adoption is a separate, id-verified step", () => { + const legacy: SlackIndex = { channelId: "C1", teamDomain: "acme.slack.com", lastTs: "100.0", messages: [] }; + writeFileSync(legacyIndexPath(dir), JSON.stringify(legacy)); + expect(readIndex("code-review", dir)).toBeNull(); + expect(existsSync(legacyIndexPath(dir))).toBe(true); // untouched + }); + + test("with no legacy file, a fresh install just starts with null per channel", () => { + expect(readIndex("code-review", dir)).toBeNull(); + expect(readIndex("team-codeowners", dir)).toBeNull(); + }); + + test("writeIndex writes to the channel-specific path, independent of other channels", () => { + const idx: SlackIndex = { channelId: "C2", teamDomain: "acme.slack.com", lastTs: "200.0", messages: [] }; + writeIndex("team-codeowners", idx, dir); + expect(readIndex("team-codeowners", dir)).toEqual(idx); + expect(readIndex("code-review", dir)).toBeNull(); + }); +}); + +describe("adoptLegacyIndex", () => { + let dir: string; + beforeEach(() => { dir = mkdtempSync(join(tmpdir(), "slack-idx-")); }); + afterEach(() => { rmSync(dir, { recursive: true, force: true }); }); + + // The legacy index's channelId is the real Slack id of whichever channel a + // pre-tabs install was actually using -- here, "code-review"'s. + const legacy: SlackIndex = { channelId: "C1", teamDomain: "acme.slack.com", lastTs: "100.0", messages: [] }; + + test("adopts when the resolved channel id matches (the default channel winning the race)", () => { + writeFileSync(legacyIndexPath(dir), JSON.stringify(legacy)); + const adopted = adoptLegacyIndex("code-review", "C1", dir); + expect(adopted).toEqual(legacy); + expect(existsSync(legacyIndexPath(dir))).toBe(false); // consumed + expect(readIndex("code-review", dir)).toEqual(legacy); + }); + + test("refuses when the resolved channel id does not match (a codeowners tab winning the race), leaving the legacy file for its real owner", () => { + writeFileSync(legacyIndexPath(dir), JSON.stringify(legacy)); + // team-codeowners's own resolved id ("C2") is not code-review's ("C1"). + const adopted = adoptLegacyIndex("team-codeowners", "C2", dir); + expect(adopted).toBeNull(); + expect(readIndex("team-codeowners", dir)).toBeNull(); // no fresh file written by adoptLegacyIndex itself + expect(existsSync(legacyIndexPath(dir))).toBe(true); // legacy file untouched, still there for code-review + expect(readIndex("code-review", dir)).toBeNull(); // not adopted for code-review either -- still pending + + // The default channel can still adopt it afterward. + const laterAdopted = adoptLegacyIndex("code-review", "C1", dir); + expect(laterAdopted).toEqual(legacy); + expect(existsSync(legacyIndexPath(dir))).toBe(false); + expect(readIndex("code-review", dir)).toEqual(legacy); + }); + + test("returns null with no legacy file to adopt", () => { + expect(adoptLegacyIndex("code-review", "C1", dir)).toBeNull(); + }); + + test("returns null (does not overwrite) when the channel already has its own index", () => { + const own: SlackIndex = { channelId: "C1", teamDomain: "acme.slack.com", lastTs: "50.0", messages: [] }; + writeIndex("code-review", own, dir); + writeFileSync(legacyIndexPath(dir), JSON.stringify(legacy)); + expect(adoptLegacyIndex("code-review", "C1", dir)).toBeNull(); + expect(readIndex("code-review", dir)).toEqual(own); // untouched + expect(existsSync(legacyIndexPath(dir))).toBe(true); // legacy left alone too + }); +}); + describe("attachSlack", () => { test("attaches the client slice by webUrl, leaves others untouched", () => { const refs = new Map([ diff --git a/src/__tests__/view.test.ts b/src/__tests__/view.test.ts index 4350d4c..976a358 100644 --- a/src/__tests__/view.test.ts +++ b/src/__tests__/view.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import type { BoardMR } from "../data.ts"; -import { filterByMember, sortMRs, groupMRs, commentDot, dataAgeLabel, statusFlags, nestStacks, memberPeerState, joinRowState } from "../view.ts"; +import type { TabConfig } from "../config.ts"; +import { filterByMember, filterByTab, sortMRs, groupMRs, commentDot, dataAgeLabel, statusFlags, nestStacks, memberPeerState, joinRowState } from "../view.ts"; function mr(overrides: Partial): BoardMR { return { @@ -28,6 +29,35 @@ describe("filterByMember", () => { }); }); +describe("filterByTab", () => { + const members = new Set(["ada"]); + const rows = [ + mr({ iid: 1, author: { username: "ada" } as any, codeownerSections: ["Acme"] } as any), + mr({ iid: 2, author: { username: "outsider" } as any, codeownerSections: ["Acme"] } as any), + mr({ iid: 3, author: { username: "outsider" } as any, codeownerSections: [] } as any), + ]; + + test("authors tab excludes a tagged stranger, keeps roster rows", () => { + const team: TabConfig = { id: "t", label: "T", source: { kind: "authors" } }; + expect(filterByTab(rows, team, members).map((m) => m.iid)).toEqual([1]); + }); + + test("codeowners tab filters to the section and excludes roster authors", () => { + const q: TabConfig = { id: "q", label: "Q", source: { kind: "codeowners", section: "Acme", excludeMembers: true } }; + expect(filterByTab(rows, q, members).map((m) => m.iid)).toEqual([2]); + }); + + test("codeowners tab without excludeMembers keeps roster authors in the section", () => { + const q: TabConfig = { id: "q", label: "Q", source: { kind: "codeowners", section: "Acme" } }; + expect(filterByTab(rows, q, members).map((m) => m.iid)).toEqual([1, 2]); + }); + + test("codeowners tab drops rows outside the section regardless of authorship", () => { + const q: TabConfig = { id: "q", label: "Q", source: { kind: "codeowners", section: "Acme", excludeMembers: true } }; + expect(filterByTab(rows, q, members).some((m) => m.iid === 3)).toBe(false); + }); +}); + describe("commentDot", () => { test("no summary → no dot (fetch skipped/failed)", () => { expect(commentDot(undefined)).toBeNull(); @@ -400,6 +430,7 @@ describe("parseViewState", () => { member: "bob", group: "status", sort: "progress", + tab: "", }); }); test("ignores unknown member and invalid group/sort", () => { @@ -421,6 +452,22 @@ describe("parseViewState", () => { test("stored value still wins over defaultMember", () => { expect(parseViewState("", { member: "alice" }, members, "bob").member).toBe("alice"); }); + + test("no validTabs known yet resolves tab to empty, matching DEFAULT_VIEW", () => { + expect(parseViewState("?tab=q", null, members).tab).toBe(""); + }); + + test("a known tab id from the URL wins", () => { + expect(parseViewState("?tab=q", null, members, "all", ["t", "q"]).tab).toBe("q"); + }); + + test("an unknown tab id falls back to the first configured tab", () => { + expect(parseViewState("?tab=zzz", null, members, "all", ["t", "q"]).tab).toBe("t"); + }); + + test("no tab in the URL falls back to the first configured tab", () => { + expect(parseViewState("", null, members, "all", ["t", "q"]).tab).toBe("t"); + }); }); describe("serializeViewState", () => { @@ -428,7 +475,10 @@ describe("serializeViewState", () => { expect(serializeViewState(DEFAULT_VIEW)).toBe(""); }); test("includes non-defaults", () => { - expect(serializeViewState({ member: "bob", group: "status", sort: "oldest" })).toBe("?member=bob&group=status"); + expect(serializeViewState({ member: "bob", group: "status", sort: "oldest", tab: "" })).toBe("?member=bob&group=status"); + }); + test("includes a set tab, even a first-tab id", () => { + expect(serializeViewState({ member: "all", group: "age", sort: "oldest", tab: "team" })).toBe("?tab=team"); }); }); diff --git a/src/cache.ts b/src/cache.ts index 7765314..f7ea49a 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -112,7 +112,15 @@ export class SnapshotCache { return settle( this.snapshot ? { ...this.snapshot, fetchError: message } - : { mrs: [], fetchedAt: this.now(), fetchError: message, dataSyncedAt: null, scopeUncovered: [], scopeWindowDays: null }, + : { + mrs: [], + fetchedAt: this.now(), + fetchError: message, + dataSyncedAt: null, + scopeUncovered: [], + scopeWindowDays: null, + scopeUncoveredSections: [], + }, ); }) .finally(() => { diff --git a/src/client/__tests__/config-shapes.test.ts b/src/client/__tests__/config-shapes.test.ts index bb9298f..119d74f 100644 --- a/src/client/__tests__/config-shapes.test.ts +++ b/src/client/__tests__/config-shapes.test.ts @@ -33,15 +33,27 @@ function def(over: Partial & { key: string }): ConfigDef { }; } +/** Composite board.* registry keys with no edit UI yet -- rowKind's + "readonly" fallback (no COMPOSITE_SHAPES entry) is the intended + rendering for these, not a coverage gap. */ +const DELIBERATELY_READONLY_COMPOSITES = ["board.tabs"]; + describe("COMPOSITE_SHAPES", () => { - test("covers every composite board.* key in the registry", () => { + test("covers every composite board.* key in the registry except the deliberately-readonly ones", () => { const composites = allDefs() .filter((d) => d.key.startsWith("board.") && (d.type === "object" || d.type === "array")) .map((d) => d.key) + .filter((k) => !DELIBERATELY_READONLY_COMPOSITES.includes(k)) .sort(); expect(Object.keys(COMPOSITE_SHAPES).sort()).toEqual(composites); }); + test("board.tabs is a composite key deliberately left readonly", () => { + expect(allDefs().find((d) => d.key === "board.tabs")?.type).toBe("array"); + expect(COMPOSITE_SHAPES["board.tabs"]).toBeUndefined(); + expect(rowKind(def({ key: "board.tabs", type: "array" }))).toBe("readonly"); + }); + test("names no key the registry lacks", () => { const known = new Set(allDefs().map((d) => d.key)); for (const key of Object.keys(COMPOSITE_SHAPES)) expect(known.has(key)).toBe(true); diff --git a/src/client/board/Board.tsx b/src/client/board/Board.tsx index c29cb47..5b4858c 100644 --- a/src/client/board/Board.tsx +++ b/src/client/board/Board.tsx @@ -1,8 +1,8 @@ import { useCallback, useEffect, useRef, useState } from "react"; import type { BoardMR } from "../../data.ts"; -import { filterByMember, sortMRs, groupMRs, parseViewState, serializeViewState, dataAgeLabel } from "../../view.ts"; +import { filterByMember, filterByTab, sortMRs, groupMRs, parseViewState, serializeViewState, dataAgeLabel } from "../../view.ts"; import type { ViewState } from "../../view.ts"; -import { selectionOf, postableOf } from "../../selection.ts"; +import { selectionOf, postableOf, tabChangeClearsSelection } from "../../selection.ts"; import type { DraftInfo, BoardMRWithReview, @@ -68,12 +68,12 @@ export function Board() { setTheme(m); }; const update = (patch: Partial) => { - setState((prev) => { - const next = { ...prev, ...patch }; - localStorage.setItem(STATE_KEY, JSON.stringify(next)); - history.replaceState(null, "", serializeViewState(next) || location.pathname); - return next; - }); + const clearsSelection = tabChangeClearsSelection(patch, state.tab); + const next = { ...state, ...patch }; + localStorage.setItem(STATE_KEY, JSON.stringify(next)); + history.replaceState(null, "", serializeViewState(next) || location.pathname); + setState(next); + if (clearsSelection) setSelected(new Set()); }; // Re-resolve the view state's member against the roster the instant real @@ -93,7 +93,9 @@ export function Board() { } catch { stored = null; } - setState(parseViewState(location.search, stored, usernames, d.defaultMember)); + // Tab validated once here, same as member -- never re-derived from the + // URL on later polls, so a live tab pick survives the 60s refresh cycle. + setState(parseViewState(location.search, stored, usernames, d.defaultMember, d.tabs.map((t) => t.id))); } else { setState((prev) => (prev.member === "all" || usernames.includes(prev.member) ? prev : { ...prev, member: "all" })); } @@ -204,15 +206,15 @@ export function Board() { axis: "review", path: "/review", verbing: "launching review", noun: "review", optimistic: optimisticLifecycle, addToast, reload: load, }); - const handleLaunch = useCallback((mr: BoardMR, note?: string) => launchReview(mr, {}, note), [launchReview]); + const handleLaunch = useCallback((mr: BoardMR, note?: string) => launchReview(mr, { tabId: state.tab }, note), [launchReview, state.tab]); const reReviewAction = useLaunchAction({ axis: "review", path: "/review", verbing: "re-reviewing", noun: "review", optimistic: optimisticLifecycle, addToast, reload: load, }); const handleReReview = useCallback( - (mr: BoardMR, note?: string) => reReviewAction(mr, { reReview: true }, note), - [reReviewAction], + (mr: BoardMR, note?: string) => reReviewAction(mr, { reReview: true, tabId: state.tab }, note), + [reReviewAction, state.tab], ); const respondAction = useLaunchAction({ @@ -405,17 +407,32 @@ export function Board() { ? `board shows ${data.staleAfterDays} days but rt syncs ${data.scopeWindowDays} days... align configs` : null; + // data.tabs is always non-empty (server falls back to IMPLICIT_TABS); state.tab + // itself may briefly lag on the very first render before onData's validation + // pass lands, so fall back to the first tab rather than trust it blindly. + const activeTab = data.tabs.find((t) => t.id === state.tab) ?? data.tabs[0]!; + const isCodeownersTab = activeTab.source.kind === "codeowners"; + // A codeowners tab's "who counts as roster" set for excludeMembers. + const rosterUsernames = new Set(data.members.map((m) => m.username)); + const tabSyncing = + activeTab.source.kind === "codeowners" && data.scopeUncoveredSections.includes(activeTab.source.section); + // Server state wins; otherwise show an optimistic "queued" badge if pending. const mrs = overlay(data.mrs, optimisticLifecycle.state); - const filtered = filterByMember(mrs, state.member); + const tabFiltered = filterByTab(mrs, activeTab, rosterUsernames); + // Codeowners tabs bypass member filtering entirely (and the sidebar that + // drives it) -- the queue is scoped by section, not by roster author. + const filtered = isCodeownersTab ? tabFiltered : filterByMember(tabFiltered, state.member); const groups = groupMRs(filtered, state.group, data.members.map((m) => m.username), now).map((g) => ({ label: g.label, mrs: sortMRs(g.mrs, state.sort), })); - const activeMember = state.member === "all" ? null : data.members.find((m) => m.username === state.member) ?? null; + const activeMember = + !isCodeownersTab && state.member !== "all" ? data.members.find((m) => m.username === state.member) ?? null : null; // Show each row's author only when the view mixes authors: the All view - // grouped by anything but author (where the group header isn't the name). - const showAuthor = state.member === "all" && state.group !== "author"; + // grouped by anything but author (where the group header isn't the name), + // or a codeowners tab, which is never narrowed to one author. + const showAuthor = isCodeownersTab || (state.member === "all" && state.group !== "author"); // Under author grouping the header IS the name, so rows normally drop the // author tag -- but a stack pulled to its root's group can carry a // co-author's MR under someone else's header. Tag the rows whenever a group @@ -467,20 +484,28 @@ export function Board() { canPostSummary: data.slackEnabled && data.local && postableMrs.length > 0, postingSummary, onPostSummary: () => handlePostSummary(postableMrs), + tabs: data.tabs, + tabSyncing, }; return ( -
- {/* Desktop roster (hidden on mobile, where it moves into the drawer). */} - update({ member })} - onSettings={openSettings} - onConfig={openConfig} - scopeUncovered={data.scopeUncovered} - /> +
+ {/* Desktop roster (hidden on mobile, where it moves into the drawer). + Also hidden on a codeowners tab: it isn't filtered by member, so the + roster has nothing to drive. */} + {!isCodeownersTab && ( + update({ member })} + onSettings={openSettings} + onConfig={openConfig} + scopeUncovered={data.scopeUncovered} + /> + )}
@@ -570,18 +595,20 @@ export function Board() { {ICONS.close}
- { - update({ member }); - setMenuOpen(false); - }} - onSettings={openSettings} - onConfig={openConfig} - scopeUncovered={data.scopeUncovered} - /> + {!isCodeownersTab && ( + { + update({ member }); + setMenuOpen(false); + }} + onSettings={openSettings} + onConfig={openConfig} + scopeUncovered={data.scopeUncovered} + /> + )}
diff --git a/src/client/board/Controls.tsx b/src/client/board/Controls.tsx index 497580e..4b1a58b 100644 --- a/src/client/board/Controls.tsx +++ b/src/client/board/Controls.tsx @@ -1,7 +1,8 @@ import { GROUP_KEYS, SORT_KEYS } from "../../view.ts"; import type { ViewState } from "../../view.ts"; +import type { TabConfig } from "../../config.ts"; import type { ThemeMode, ViewMode } from "../types.ts"; -import { CopyButton, ICONS, LabeledSeg, Segmented } from "@mattstack/tui-kit"; +import { Chip, CopyButton, ICONS, LabeledSeg, Segmented } from "@mattstack/tui-kit"; import { GROUP_LABEL, SORT_LABEL } from "./format.ts"; import { SLACK_ICON } from "./chips.tsx"; @@ -21,6 +22,8 @@ function Controls({ onPostSummary, canPostSummary, postingSummary, + tabs, + tabSyncing, stacked = false, }: { state: ViewState; @@ -36,17 +39,32 @@ function Controls({ onPostSummary?: () => void; canPostSummary?: boolean; postingSummary?: boolean; + /** Board tabs, in display order; the bar itself only renders past one tab. */ + tabs: TabConfig[]; + /** Whether the active codeowners tab's section is still mid-backfill. */ + tabSyncing: boolean; stacked?: boolean; }) { const group = update({ group: g })} />; const sort = update({ sort: s })} />; const viewSeg = ; const themeSeg = ; + const tabIds = tabs.map((t) => t.id); + const tabLabels = Object.fromEntries(tabs.map((t) => [t.id, t.label])); + const tabBar = tabs.length > 1 && ( + update({ tab })} /> + ); + const syncingChip = tabSyncing && ( + + codeowner queue syncing + + ); // Drawer: labeled full-width rows, so a mobile user can tell what each does. if (stacked) { return ( <> + {tabBar &&
tab{tabBar}{syncingChip}
}
group{group}
sort{sort}
view{viewSeg}
@@ -83,6 +101,8 @@ function Controls({ board's own plain buttons (the refresh button above, the selection bar's post/clear), which are not CopyButton instances. */} {canCopy && } + {tabBar} + {syncingChip} {group} {sort} {viewSeg} diff --git a/src/client/types.ts b/src/client/types.ts index f49c08d..70ea0e6 100644 --- a/src/client/types.ts +++ b/src/client/types.ts @@ -2,6 +2,7 @@ import type { MouseEvent } from "react"; import type { BoardMR } from "../data.ts"; import type { SlackTemplates } from "../template.ts"; import type { RespondStatus } from "../respond-outcome.ts"; +import type { TabConfig } from "../config.ts"; export interface RosterMember { username: string; @@ -64,6 +65,9 @@ export interface BoardData { dataSyncedAt: number | null; /** Authors this board demanded but rt hasn't finished backfilling yet. */ scopeUncovered: string[]; + /** Codeowners sections this board demanded but rt hasn't finished backfilling + yet -- drives the "codeowner queue syncing" badge on the matching tab. */ + scopeUncoveredSections: string[]; /** Narrowest sync window (days) among the daemon reads; null when none carried one. */ scopeWindowDays: number | null; /** The board's own configured stale cutoff (days), for comparing against @@ -75,6 +79,9 @@ export interface BoardData { /** Peering health: "ok" when the switchboard accepts us, "unauthorized" when it rejects us, null when this board isn't peering at all. */ peering: "ok" | "unauthorized" | null; + /** Board tabs, in display order. Always non-empty (config.tabs falls back to + IMPLICIT_TABS server-side). */ + tabs: TabConfig[]; } export type ThemeMode = "light" | "dark" | "system"; diff --git a/src/config.ts b/src/config.ts index d75419c..a6a9063 100644 --- a/src/config.ts +++ b/src/config.ts @@ -33,6 +33,19 @@ export function daemonRepoField(config: Pick, projectPat return repoIdentityField(config.rtRepos[projectPath]); } +export interface TabConfig { + id: string; + label: string; + source: { kind: "authors" } | { kind: "codeowners"; section: string; excludeMembers?: boolean }; + /** Overrides slack.channel for this tab's index, reactions, and posts. */ + slackChannel?: string; + /** Overrides review-launch skill resolution for this tab. Empty/absent = normal resolution. */ + reviewSkill?: string; +} + +/** No config.json/store tabs = one classic authors-roster tab, never zero tabs. */ +export const IMPLICIT_TABS: TabConfig[] = [{ id: "team", label: "Team", source: { kind: "authors" } }]; + export interface Member { username: string; /** Optional display name; falls back to the GitLab profile lookup, then username. */ @@ -90,6 +103,8 @@ export interface BoardConfig { /** Peer-boards relay. Empty url disables every peer feature (publish, poll, nudge endpoint) cleanly. Token comes from SWITCHBOARD_TOKEN, not config. */ switchboard: SwitchboardBoardConfig; + /** Board tabs, in display order. Absent in config.json/store = IMPLICIT_TABS. */ + tabs: TabConfig[]; } export interface SwitchboardBoardConfig { @@ -202,6 +217,7 @@ export function parseConfig(raw: string, source = "config.json"): BoardConfig { } const slack = parseSlack(cfg.slack, source); const switchboard = parseSwitchboard(cfg.switchboard, source); + const tabs = parseTabs(cfg.tabs, source); const rtRepos = (cfg.rtRepos && typeof cfg.rtRepos === "object" && !Array.isArray(cfg.rtRepos)) ? Object.fromEntries(Object.entries(cfg.rtRepos).filter(([, v]) => typeof v === "string")) : {}; @@ -226,9 +242,67 @@ export function parseConfig(raw: string, source = "config.json"): BoardConfig { rtRepos, slack, switchboard, + tabs, }; } +/** Absent = IMPLICIT_TABS (the classic single authors-roster board), never zero tabs. */ +function parseTabs(raw: unknown, source: string): TabConfig[] { + if (raw === undefined) return IMPLICIT_TABS; + if (!Array.isArray(raw)) throw new Error(`${source} "tabs" must be an array`); + if (raw.length === 0) throw new Error(`${source} "tabs" must not be empty (omit "tabs" for the implicit default)`); + const seenIds = new Set(); + return raw.map((entry, i) => { + const label = `tabs[${i}]`; + if (!entry || typeof entry !== "object") { + throw new Error(`${source} "${label}" must be an object`); + } + const t = entry as Partial; + if (!t.id || typeof t.id !== "string") { + throw new Error(`${source} "${label}" is missing a non-empty "id"`); + } + if (seenIds.has(t.id)) { + throw new Error(`${source} has a duplicate tab id "${t.id}" in ${label}`); + } + seenIds.add(t.id); + if (!t.label || typeof t.label !== "string") { + throw new Error(`${source} "${label}" is missing a non-empty "label"`); + } + if (!t.source || typeof t.source !== "object") { + throw new Error(`${source} "${label}.source" must be an object`); + } + const src = t.source as { kind?: string; section?: string; excludeMembers?: unknown }; + if (src.kind !== "authors" && src.kind !== "codeowners") { + throw new Error(`${source} "${label}.source.kind" must be "authors" or "codeowners"`); + } + let source_: TabConfig["source"]; + if (src.kind === "authors") { + source_ = { kind: "authors" }; + } else { + if (!src.section || typeof src.section !== "string") { + throw new Error(`${source} "${label}.source.section" is required for a codeowners tab`); + } + if (src.excludeMembers !== undefined && typeof src.excludeMembers !== "boolean") { + throw new Error(`${source} "${label}.source.excludeMembers" must be a boolean`); + } + source_ = { kind: "codeowners", section: src.section, ...(src.excludeMembers !== undefined ? { excludeMembers: src.excludeMembers } : {}) }; + } + if (t.slackChannel !== undefined && typeof t.slackChannel !== "string") { + throw new Error(`${source} "${label}.slackChannel" must be a string`); + } + if (t.reviewSkill !== undefined && typeof t.reviewSkill !== "string") { + throw new Error(`${source} "${label}.reviewSkill" must be a string`); + } + return { + id: t.id, + label: t.label, + source: source_, + ...(t.slackChannel !== undefined ? { slackChannel: t.slackChannel } : {}), + ...(t.reviewSkill !== undefined ? { reviewSkill: t.reviewSkill } : {}), + }; + }); +} + /** Shared with saveSwitchboardUrl's owned-branch write, so a trailing slash never lands in the store either — peer/onboard.ts builds `${url}/invites` verbatim, and a stored slash would double up into `//invites`. */ @@ -391,6 +465,7 @@ function withBoardStoreFallback(fileConfig: BoardConfig, resolve: GetSettingFn): doctorCwd: cwds?.doctor ?? fileConfig.doctorCwd, rtRepos: rtReposStore ? Object.fromEntries(rtReposStore.map((r) => [r.project, r.repo])) : fileConfig.rtRepos, switchboard: { url: storeValue("board.switchboardUrl", resolve) ?? fileConfig.switchboard.url }, + tabs: storeValue("board.tabs", resolve) ?? fileConfig.tabs, }; return parseConfig(JSON.stringify(merged), "a board.* team settings-store value"); diff --git a/src/data.ts b/src/data.ts index 6133099..e7d7186 100644 --- a/src/data.ts +++ b/src/data.ts @@ -32,6 +32,11 @@ export type BoardMR = MRDashboardProps & { /** rt repo name for daemon reads (config.rtRepos[projectPath]); null when the project has no mapping, which surfaces as a fetch error server-side. */ rtRepo: string | null; + /** Codeowner sections this row was tagged into that a configured codeowners + tab also declares. Empty when the row wasn't tag-kept (a member's own + MR) or its tags matched no configured tab. Display filtering by tab is + Task 11's job -- buildBoard only stamps the intersection. */ + codeownerSections: string[]; }; export interface Snapshot { @@ -48,6 +53,9 @@ export interface Snapshot { /** Narrowest `scope.windowDays` among the daemon reads; null when no read carried a scope. */ scopeWindowDays: number | null; + /** Union of `scope.uncoveredSections` across the daemon reads: codeowner + sections some project's sync hasn't swept yet. */ + scopeUncoveredSections: string[]; } /** Parse "group/project" out of a GitLab MR web URL. */ @@ -84,20 +92,37 @@ function scrubAvatarUrls(value: unknown): void { } /** - * Shape raw MRs into a flat board list: authored by a configured member, open, - * not draft, in a configured project. Each MR is tagged with its author, created - * / updated timestamps, unresolved-thread count, and derived pipeline state. The - * client owns all grouping and sorting, so this list is unsorted. + * Shape raw MRs into a flat board list: authored by a configured member (or + * tagged into a configured codeowners tab's section), open, not draft, in a + * configured project. Each MR is tagged with its author, created / updated + * timestamps, unresolved-thread count, derived pipeline state, and its kept + * codeowner tags. The client owns all grouping and sorting, so this list is + * unsorted. `tags` (keyed by pr.id) is the per-MR codeowner sections a daemon + * read reported; omitted entirely for callers that never declare + * codeownerSections demand (e.g. the single-member fetch). */ -export function buildBoard(prs: PullRequest[], config: BoardConfig, now: number = Date.now()): BoardMR[] { +export function buildBoard( + prs: PullRequest[], + config: BoardConfig, + now: number = Date.now(), + tags?: Map, +): BoardMR[] { const members = new Set(config.members.map((m) => m.username)); const projects = new Set(config.projects); const staleCutoff = now - config.staleAfterDays * 86_400_000; const prefixes = new Set(config.ticketPrefixes); + const tabSections = new Set(config.tabs.flatMap((t) => (t.source.kind === "codeowners" ? [t.source.section] : []))); const out: BoardMR[] = []; for (const pr of prs) { if (pr.state !== "opened") continue; - if (!pr.author || !members.has(pr.author.username)) continue; + if (!pr.author) continue; + const isMember = members.has(pr.author.username); + // Only tags matching a currently-configured tab count -- a tab removed + // from config must not keep stale-tagged rows on the board. + const tagged = (tags?.get(pr.id) ?? []).filter((s) => tabSections.has(s)); + // A tagged row is on the board regardless of author; per-tab display + // filtering by section is Task 11's job, not buildBoard's. + if (!isMember && tagged.length === 0) continue; // Someone else's draft isn't yours to act on, so it stays off the board; // your own show up with a DRAFT chip and a "mark ready" action. With // defaultMember "all" there's no single "you", so no drafts are shown. @@ -106,7 +131,9 @@ export function buildBoard(prs: PullRequest[], config: BoardConfig, now: number if (pr.updatedAt && Date.parse(pr.updatedAt) < staleCutoff) continue; // Team filter: keep only MRs whose Linear ticket prefix is configured. // No prefixes configured → keep everything. Untagged MRs are dropped. - if (prefixes.size > 0) { + // Ticket-prefix filtering is a roster-board concept; a row kept by its + // codeowner tag rides the board regardless of whose ticket it carries. + if (prefixes.size > 0 && tagged.length === 0) { const ticket = extractTicketId(pr.sourceBranch, pr.title); const prefix = ticket ? ticket.slice(0, ticket.indexOf("-")) : null; if (!prefix || !prefixes.has(prefix)) continue; @@ -132,6 +159,7 @@ export function buildBoard(prs: PullRequest[], config: BoardConfig, now: number isDraft: pr.draft === true, repositoryId: pr.repositoryId, rtRepo: config.rtRepos[path] ?? null, + codeownerSections: tagged, }); } return out; @@ -161,17 +189,46 @@ export function stripDraftPrefix(title: string): string { // (the actual listen port is env/default-derived, not config), so callers // pass the resolved port explicitly. export function boardDemand(config: BoardConfig, port: number): DemandDecl { + const sections = [...new Set(config.tabs.flatMap((t) => (t.source.kind === "codeowners" ? [t.source.section] : [])))]; return { client: `mr-board:${port}`, authors: config.members.map((m) => m.username), + ...(sections.length > 0 ? { codeownerSections: sections } : {}), declaredAt: Date.now(), }; } +/** Which Slack channel an MR's review-request lives in: roster MRs use the + board channel; a tag-only row uses its codeowners tab's channel (first + match in tab order), falling back to the board channel when none of its + tags carry a slackChannel. */ +export function channelForMR(config: BoardConfig, mr: Pick): string { + const isMember = config.members.some((m) => m.username === mr.author.username); + if (!isMember) { + for (const tab of config.tabs) { + if (tab.source.kind === "codeowners" && tab.slackChannel && mr.codeownerSections.includes(tab.source.section)) { + return tab.slackChannel; + } + } + } + return config.slack.channel; +} + +/** Every Slack channel this board's config can route a review-request to: + the default channel plus every tab-level override, deduped. Used to + validate a client-supplied channel override. */ +export function configuredSlackChannels(config: Pick): string[] { + const channels = new Set([config.slack.channel]); + for (const tab of config.tabs) { + if (tab.slackChannel) channels.add(tab.slackChannel); + } + return [...channels]; +} + /** One project's sync facts, the shape aggregateSyncScope folds across projects. */ export interface SyncScopeRead { syncedAt: number; - scope?: { authors: string[]; windowDays: number; uncovered: string[] }; + scope?: { authors: string[]; windowDays: number; uncovered: string[]; sections?: string[]; uncoveredSections?: string[] }; } /** @@ -179,23 +236,26 @@ export interface SyncScopeRead { * board-wide picture. `dataSyncedAt` is the oldest syncedAt (the board is * only as fresh as its stalest project); `scopeUncovered` unions every * project's uncovered authors; `scopeWindowDays` is the narrowest window - * (the tightest constraint any project reported). A project that errored - * before yielding a read is simply absent from `reads`. + * (the tightest constraint any project reported); `scopeUncoveredSections` + * unions every project's uncovered codeowner sections. A project that + * errored before yielding a read is simply absent from `reads`. */ export function aggregateSyncScope( reads: SyncScopeRead[], -): { dataSyncedAt: number | null; scopeUncovered: string[]; scopeWindowDays: number | null } { +): { dataSyncedAt: number | null; scopeUncovered: string[]; scopeWindowDays: number | null; scopeUncoveredSections: string[] } { let dataSyncedAt: number | null = null; let scopeWindowDays: number | null = null; const uncovered = new Set(); + const uncoveredSections = new Set(); for (const read of reads) { dataSyncedAt = dataSyncedAt === null ? read.syncedAt : Math.min(dataSyncedAt, read.syncedAt); if (read.scope) { scopeWindowDays = scopeWindowDays === null ? read.scope.windowDays : Math.min(scopeWindowDays, read.scope.windowDays); for (const author of read.scope.uncovered) uncovered.add(author); + for (const section of read.scope.uncoveredSections ?? []) uncoveredSections.add(section); } } - return { dataSyncedAt, scopeUncovered: [...uncovered], scopeWindowDays }; + return { dataSyncedAt, scopeUncovered: [...uncovered], scopeWindowDays, scopeUncoveredSections: [...uncoveredSections] }; } /** @@ -207,6 +267,16 @@ export function hasChangesRequested(mr: BoardMR): boolean { return mr.reviews.reviewers?.some((r) => getReviewDisplayState(r.reviewState ?? null) === "changes_requested") ?? false; } +/** Which snapshot MRs the served board keeps: a visible (non-hidden) roster + member's MR, or any MR carrying at least one codeowner tag. The tag arm is + what lets a codeowner-tagged stranger -- never a roster member -- reach a + codeowners tab; without it every tagged row from outside the roster would + be dropped here before a tab ever saw it. */ +export function visibleMrsFor(mrs: BoardMR[], visibleMembers: Member[]): BoardMR[] { + const visibleNames = new Set(visibleMembers.map((m) => m.username)); + return mrs.filter((mr) => visibleNames.has(mr.author.username) || mr.codeownerSections.length > 0); +} + export interface RosterMember { username: string; name: string | null; @@ -221,3 +291,17 @@ export function buildRoster(members: Member[], mrs: BoardMR[], names: Map mr.author.username === member.username).length, })); } + +export function reviewSkillForTab( + config: BoardConfig, + tabId: string | undefined, + mrUrl: string, + fallback: (kind: "review", mrUrl: string) => string, +): string { + const tab = tabId ? config.tabs.find((t) => t.id === tabId) : undefined; + if (tab?.reviewSkill) { + console.log(`review skill: ${tab.reviewSkill} (tab ${tab.id})`); + return tab.reviewSkill; + } + return fallback("review", mrUrl); +} diff --git a/src/selection.ts b/src/selection.ts index ca53734..4876396 100644 --- a/src/selection.ts +++ b/src/selection.ts @@ -1,6 +1,7 @@ /** Board selection, kept out of client.tsx so it can be tested without a DOM. Selection is keyed by webUrl rather than by position, which is what lets it survive the refresh poll and any member/group/sort change. */ +import type { ViewState } from "./view.ts"; /** The selected MRs, in board order. A url that's no longer on the board just drops out -- MRs merge and leave while a selection is open, and that needs @@ -20,3 +21,12 @@ export function postableOf !!m.webUrl && !m.slack?.posted); } + +/** Whether a ViewState patch should drop the current selection. A codeowners + tab is a differently-scoped queue (its own Slack channel, in a later task), + so unlike a member/group/sort change -- which a selection deliberately + survives -- an actual tab-id change must not carry a selection across. + Re-sending the already-active tab id (a re-click) is not a change. */ +export function tabChangeClearsSelection(patch: Partial, currentTab: string): boolean { + return patch.tab !== undefined && patch.tab !== currentTab; +} diff --git a/src/server.ts b/src/server.ts index cc38adf..2b7468a 100644 --- a/src/server.ts +++ b/src/server.ts @@ -10,7 +10,7 @@ import { loadConfig, loadGitLabToken, loadSlackToken, loadSwitchboardToken, load import { memoizeAsync } from "./memoize-async.ts"; import { resolveBoardSkill, type BoardSkillKind } from "./manifest-bindings.ts"; import { upsertEnvKeys } from "./env-file.ts"; -import { aggregateSyncScope, boardDemand, buildBoard, buildRoster, projectPathFromWebUrl, type BoardMR, type SyncScopeRead } from "./data.ts"; +import { aggregateSyncScope, boardDemand, buildBoard, buildRoster, channelForMR, configuredSlackChannels, projectPathFromWebUrl, reviewSkillForTab, visibleMrsFor, type BoardMR, type SyncScopeRead } from "./data.ts"; import { GitLabProvider, ReadBackFailedError, NoteMutator, parseRepoId } from "@mattstack/glance"; import { summarizeDiscussions, threadStatusCounts, unresolvedReviewerCount } from "./discussions.ts"; import { readProjectMRs, readDiscussions, subscribe } from "@mattstack/rt-client"; @@ -168,12 +168,16 @@ function attachPeerState(mrs: T[], now: nu const FETCH_CONCURRENCY = 4; /** fetchTeamMRs' result: the opened MRs plus the aggregated sync facts from - every project read, for the caller to fold into the snapshot. */ + every project read, for the caller to fold into the snapshot. `tags` is + every tagged MR's codeowner sections, keyed by pr.id, for buildBoard to + intersect against the configured tabs. */ interface TeamMRsResult { prs: PullRequest[]; dataSyncedAt: number | null; scopeUncovered: string[]; scopeWindowDays: number | null; + scopeUncoveredSections: string[]; + tags: Map; } /** @@ -193,6 +197,7 @@ interface TeamMRsResult { */ async function fetchTeamMRs(force = false): Promise { const byId = new Map(); + const tags = new Map(); const errors: string[] = []; const reads: SyncScopeRead[] = []; const demand = boardDemand(config, port); @@ -209,11 +214,13 @@ async function fetchTeamMRs(force = false): Promise { } reads.push({ syncedAt: res.data.syncedAt, scope: res.data.scope }); for (const entry of Object.values(res.data.mrs)) { - if (entry.pr.state === "opened") byId.set(entry.pr.id, entry.pr); + if (entry.pr.state !== "opened") continue; + byId.set(entry.pr.id, entry.pr); + if (entry.codeownerSections?.length) tags.set(entry.pr.id, entry.codeownerSections); } } if (errors.length) throw new Error(errors.join(" · ")); - return { prs: [...byId.values()], ...aggregateSyncScope(reads) }; + return { prs: [...byId.values()], ...aggregateSyncScope(reads), tags }; } /** Author string for a herdr tab label: the display name, else the username. */ @@ -274,10 +281,10 @@ const cache = new SnapshotCache(async () => { // latched for the background refreshes that follow. const force = forceNextFetch; forceNextFetch = false; - const { prs, dataSyncedAt, scopeUncovered, scopeWindowDays } = await fetchTeamMRs(force); - const mrs = buildBoard(prs, config); + const { prs, dataSyncedAt, scopeUncovered, scopeWindowDays, scopeUncoveredSections, tags } = await fetchTeamMRs(force); + const mrs = buildBoard(prs, config, undefined, tags); await enrichReviewerComments(mrs); - return { mrs, dataSyncedAt, scopeUncovered, scopeWindowDays }; + return { mrs, dataSyncedAt, scopeUncovered, scopeWindowDays, scopeUncoveredSections }; }); /** @@ -287,6 +294,7 @@ const cache = new SnapshotCache(async () => { */ async function fetchMemberMRs(username: string): Promise { const out: PullRequest[] = []; + const tags = new Map(); const errors: string[] = []; for (const projectPath of config.projects) { const repoId = daemonRepoField(config, projectPath); @@ -294,11 +302,13 @@ async function fetchMemberMRs(username: string): Promise { const res = await readProjectMRs(repoId, 20_000); if (!res.ok || !res.data) { errors.push(`${projectPath}: ${res.error ?? "empty daemon response"}`); continue; } for (const entry of Object.values(res.data.mrs)) { - if (entry.pr.state === "opened" && entry.pr.author?.username === username) out.push(entry.pr); + if (entry.pr.state !== "opened" || entry.pr.author?.username !== username) continue; + out.push(entry.pr); + if (entry.codeownerSections?.length) tags.set(entry.pr.id, entry.codeownerSections); } } if (errors.length) throw new Error(errors.join(" · ")); - const mrs = buildBoard(out, config); + const mrs = buildBoard(out, config, undefined, tags); await enrichReviewerComments(mrs); return mrs; } @@ -500,8 +510,7 @@ const httpServer = Bun.serve({ // and its counts — but stay in `allMembers` so the settings modal can // check them back in. const visible = config.members.filter((m) => !m.hidden); - const visibleNames = new Set(visible.map((m) => m.username)); - const visibleMrs = snapshot.mrs.filter((mr) => visibleNames.has(mr.author.username)); + const visibleMrs = visibleMrsFor(snapshot.mrs, visible); // Retain review/respond/doctor state for exactly as long as its MR is on // the board; prune once it merges/closes/goes stale and drops off. Gated // on a healthy, non-empty snapshot so a failed fetch (stale/empty data) @@ -559,7 +568,9 @@ const httpServer = Bun.serve({ dataSyncedAt: snapshot.dataSyncedAt, scopeUncovered: snapshot.scopeUncovered, scopeWindowDays: snapshot.scopeWindowDays, + scopeUncoveredSections: snapshot.scopeUncoveredSections, staleAfterDays: config.staleAfterDays, + tabs: config.tabs, }), { headers: { "content-type": "application/json" } }, ); @@ -619,6 +630,7 @@ const httpServer = Bun.serve({ if (!parsed) return new Response("expected { mrUrl: string, iid: number }", { status: 400 }); const resume = (body as { resume?: unknown })?.resume === true; const reReview = (body as { reReview?: unknown })?.reReview === true; + const tabId = (body as { tabId?: unknown })?.tabId; const noteParse = parseLaunchNote(body); if (!noteParse.ok) return new Response(noteParse.error, { status: 400 }); const note = noteParse.note; @@ -646,7 +658,7 @@ const httpServer = Bun.serve({ void launchReReview(parsed.mrUrl, parsed.iid, { cwd: config.reviewCwd, workspaceLabel: config.reviewsWorkspace, - skill: resolveLaunchSkill("review", parsed.mrUrl), + skill: reviewSkillForTab(config, typeof tabId === "string" ? tabId : undefined, parsed.mrUrl, resolveLaunchSkill), author, claudeCommand: config.claudeCommand, note, @@ -682,7 +694,7 @@ const httpServer = Bun.serve({ cwd: config.reviewCwd, workspaceLabel: config.reviewsWorkspace, statePath, - skill: resolveLaunchSkill("review", parsed.mrUrl), + skill: reviewSkillForTab(config, typeof tabId === "string" ? tabId : undefined, parsed.mrUrl, resolveLaunchSkill), author, claudeCommand: config.claudeCommand, note, @@ -911,9 +923,12 @@ const httpServer = Bun.serve({ // The sweeper usually resolves the ref first, but a review launched and // finished inside one sweep interval can beat it here. try { + const signalSnapshot = await cache.get(); + const signalMr = signalSnapshot.mrs.find((m) => m.webUrl === signal.mrUrl); + const signalChannel = signalMr ? channelForMR(config, signalMr) : config.slack.channel; const existing = readSlackRefs().get(signal.mrUrl); if (existing?.status !== "found" || !existing.messageTs) { - await resolveSlackRef(slackToken, config.slack.channel, signal.mrUrl, signal.iid); + await resolveSlackRef(slackToken, signalChannel, signal.mrUrl, signal.iid); } const ref = await reactToMR(slackToken, signal.mrUrl, emoji); return new Response(JSON.stringify({ ok: true, reacted: true, reactions: ref.reactions ?? [] }), { @@ -1163,8 +1178,16 @@ const httpServer = Bun.serve({ } const parsed = parseReviewRequestBody(body); if (!parsed) return new Response("expected { mrUrl: string, iid: number }", { status: 400 }); + const { channel } = (body ?? {}) as { channel?: unknown }; + const allowedChannels = configuredSlackChannels(config); + if (channel !== undefined && (typeof channel !== "string" || !allowedChannels.includes(channel))) { + return new Response(`"channel" must be one of ${allowedChannels.join(", ")}`, { status: 400 }); + } try { - const ref = await resolveSlackRef(slackToken, config.slack.channel, parsed.mrUrl, parsed.iid); + const snapshot = await cache.get(); + const mr = snapshot.mrs.find((m) => m.webUrl === parsed.mrUrl); + const resolvedChannel = typeof channel === "string" ? channel : mr ? channelForMR(config, mr) : config.slack.channel; + const ref = await resolveSlackRef(slackToken, resolvedChannel, parsed.mrUrl, parsed.iid); return new Response( JSON.stringify({ ok: true, status: ref.status, permalink: ref.permalink, reactions: ref.reactions ?? [] }), { headers: { "content-type": "application/json" } }, @@ -1191,7 +1214,7 @@ const httpServer = Bun.serve({ } catch { return new Response("invalid json", { status: 400 }); } - const { mrUrls, header } = (body ?? {}) as { mrUrls?: unknown; header?: unknown }; + const { mrUrls, header, channel } = (body ?? {}) as { mrUrls?: unknown; header?: unknown; channel?: unknown }; if (!Array.isArray(mrUrls) || mrUrls.length === 0 || !mrUrls.every((u) => typeof u === "string")) { return new Response("expected { mrUrls: string[] }", { status: 400 }); } @@ -1199,12 +1222,32 @@ const httpServer = Bun.serve({ if (header !== undefined && headerOverride === null) { return new Response(`"header" must be a non-empty string of at most ${MAX_HEADER_LEN} characters`, { status: 400 }); } + const allowedChannels = configuredSlackChannels(config); + if (channel !== undefined && (typeof channel !== "string" || !allowedChannels.includes(channel))) { + return new Response(`"channel" must be one of ${allowedChannels.join(", ")}`, { status: 400 }); + } const snapshot = await cache.get(); const byUrl = new Map(snapshot.mrs.map((m) => [m.webUrl, m] as const)); const picked = (mrUrls as string[]).map((u) => byUrl.get(u)).filter((m): m is BoardMR => !!m); if (picked.length !== mrUrls.length) { return new Response("one or more mrUrls are not on the board", { status: 400 }); } + // An explicit body channel (already validated above) always wins. + // Otherwise derive per-MR: resolve/sweeper look in the tab's channel + // via channelForMR, so a post with no explicit channel must land + // there too, or the ref would pin the wrong channelId. A multi-MR + // post only has one channel to post to, so every picked MR must + // resolve to the same one. + let targetChannel: string; + if (typeof channel === "string") { + targetChannel = channel; + } else { + const resolved = new Set(picked.map((m) => channelForMR(config, m))); + if (resolved.size > 1) { + return new Response("MRs span Slack channels; post them per tab", { status: 400 }); + } + targetChannel = [...resolved][0]!; + } // Guard against duplicate posts: check for an existing ref file first, // and for MRs we've never resolved, sync the channel index and look for // the author's original review-request. If any MR already has a @@ -1214,7 +1257,7 @@ const httpServer = Bun.serve({ const freshlyFound: Array<{ iid: number; permalink?: string }> = []; try { for (const m of toResolve) { - const ref = await resolveSlackRef(slackToken, config.slack.channel, m.webUrl!, m.iid); + const ref = await resolveSlackRef(slackToken, targetChannel, m.webUrl!, m.iid); if (ref.status === "found") freshlyFound.push({ iid: m.iid, permalink: ref.permalink }); } } catch (err) { @@ -1259,7 +1302,7 @@ const httpServer = Bun.serve({ try { const refs = await postToSlack( slackToken, - config.slack.channel, + targetChannel, text, picked.map((m) => ({ webUrl: m.webUrl!, iid: m.iid })), ); @@ -1356,7 +1399,7 @@ async function autoResolveSlackRefs(): Promise { if (!targets.length) return; for (const mr of targets) { try { - await resolveSlackRef(slackToken, config.slack.channel, mr.webUrl!, mr.iid); + await resolveSlackRef(slackToken, channelForMR(config, mr), mr.webUrl!, mr.iid); } catch (err) { console.error(`auto-resolve !${mr.iid} failed: ${err instanceof Error ? err.message : err}`); } diff --git a/src/slack.ts b/src/slack.ts index 0af8470..85a5653 100644 --- a/src/slack.ts +++ b/src/slack.ts @@ -1,10 +1,11 @@ -import { readFileSync, writeFileSync, mkdirSync, readdirSync, existsSync } from "fs"; +import { readFileSync, writeFileSync, mkdirSync, readdirSync, existsSync, renameSync } from "fs"; import { join } from "path"; import { APP_ROOT } from "./app-root.ts"; const STATE_ROOT = join(APP_ROOT, "state"); export const SLACK_REF_DIR = join(STATE_ROOT, "slack"); -const INDEX_PATH = join(STATE_ROOT, "slack-index.json"); +/** Pre-tabs installs had exactly one channel and one index at this path. */ +const LEGACY_INDEX_NAME = "slack-index.json"; /** How far back the first index build reaches. Review requests older than the board's stale window are irrelevant, so we never page the whole channel. */ @@ -124,17 +125,63 @@ async function resolveChannelId(token: string, channelName: string): Promise { - const existing = readIndex(); + let existing = readIndex(channelName); const channelId = existing?.channelId ?? (await resolveChannelId(token, channelName)); + // No per-channel index yet: the resolved id above lets us verify (not just + // assume) whether the legacy single-channel index actually belongs here. + if (!existing) existing = adoptLegacyIndex(channelName, channelId); const domain = existing?.teamDomain ?? (await teamDomain(token)); const oldest = existing?.lastTs && existing.lastTs !== "0" ? existing.lastTs @@ -170,7 +220,7 @@ export async function syncIndex(token: string, channelName: string, now: number const merged = [...(existing?.messages ?? []), ...fresh]; const lastTs = merged.reduce((max, m) => (parseFloat(m.ts) > parseFloat(max) ? m.ts : max), existing?.lastTs ?? "0"); const index: SlackIndex = { channelId, teamDomain: domain, lastTs, messages: merged }; - writeIndex(index); + writeIndex(channelName, index); return index; } @@ -235,8 +285,9 @@ export async function postToSlack( now: number = Date.now(), ): Promise { if (!mrs.length) throw new Error("nothing to post"); - const existing = readIndex(); + let existing = readIndex(channelName); const channelId = existing?.channelId ?? (await resolveChannelId(token, channelName)); + if (!existing) existing = adoptLegacyIndex(channelName, channelId); const domain = existing?.teamDomain ?? (await teamDomain(token)); const ts = await postMessage(token, channelId, text); const multi = mrs.length > 1; diff --git a/src/style.css b/src/style.css index 2c032d6..0e3e612 100644 --- a/src/style.css +++ b/src/style.css @@ -332,6 +332,10 @@ /* team sidebar layout */ .tui-app { display: grid; grid-template-columns: 220px 1fr; gap: 20px; align-items: start; } .tui-app.tui-wide { max-width: 1400px; } +/* No rendered (a codeowners tab, scoped by section rather than + roster) -- drop the 220px track, or grid auto-placement strands the lone + .tui-main child in that empty first column instead of the 1fr one. */ +.tui-app.tui-no-sidebar { grid-template-columns: 1fr; } .tui-main { min-width: 0; } /* The roster gets the same framed-panel backdrop as the MR groups -- bare transparent buttons on the page grid disappear in dark mode. Item hover/ diff --git a/src/view.ts b/src/view.ts index 2bef110..bcd7022 100644 --- a/src/view.ts +++ b/src/view.ts @@ -1,6 +1,7 @@ import type { BoardMR } from "./data.ts"; import { hasChangesRequested } from "./data.ts"; import { projectKeyOf } from "./triage/stack.ts"; +import type { TabConfig } from "./config.ts"; export type GroupKey = "age" | "author" | "status" | "review"; export type SortKey = "oldest" | "progress"; @@ -153,6 +154,21 @@ export function filterByMember(mrs: BoardMR[], member: string): BoardMR[] { return member === "all" ? mrs : mrs.filter((m) => m.author.username === member); } +/** An authors tab narrows to roster members -- further per-member filtering + stays downstream in filterByMember. Without this narrowing, a + codeowner-tagged stranger (never a roster member, but let through the + server's visibility gate for the codeowners tab) would leak onto the + authors tab too. A codeowners tab narrows to rows tagged with its section; + excludeMembers additionally drops the roster's own authors, so the tab + reads as the outside-the-team queue for that section. */ +export function filterByTab(mrs: BoardMR[], tab: TabConfig, members: Set): BoardMR[] { + if (tab.source.kind === "authors") return mrs.filter((mr) => members.has(mr.author.username)); + const { section, excludeMembers } = tab.source; + return mrs.filter( + (mr) => mr.codeownerSections.includes(section) && (!excludeMembers || !members.has(mr.author.username)), + ); +} + /** Return a new array ordered by the chosen sort. Never mutates the input. */ export function sortMRs(mrs: BoardMR[], sort: SortKey): BoardMR[] { // Order by last activity (updatedAt) — the same axis the row's age token and @@ -317,16 +333,22 @@ export interface ViewState { member: string; group: GroupKey; sort: SortKey; + tab: string; } -export const DEFAULT_VIEW: ViewState = { member: "all", group: "age", sort: "oldest" }; +export const DEFAULT_VIEW: ViewState = { member: "all", group: "age", sort: "oldest", tab: "" }; -/** URL query params win, then stored localStorage values, then defaults. Invalid values are dropped. */ +/** URL query params win, then stored localStorage values, then defaults. Invalid + values are dropped. `validTabs` mirrors `validMembers`: an unknown or empty + tab (including "no tabs known yet", the state before /data.json's first + reply) resolves to the first configured tab, matching DEFAULT_VIEW.tab when + validTabs is empty. */ export function parseViewState( search: string, stored: Partial | null, validMembers: string[], defaultMember: string = "all", + validTabs: string[] = [], ): ViewState { const params = new URLSearchParams(search); const members = ["all", ...validMembers]; @@ -344,6 +366,7 @@ export function parseViewState( member: resolve("member", members, memberFallback), group: resolve("group", GROUP_KEYS, "age"), sort: resolve("sort", SORT_KEYS, "oldest"), + tab: resolve("tab", validTabs, validTabs[0] ?? ""), }; } @@ -382,6 +405,7 @@ export function serializeViewState(v: ViewState): string { if (v.member !== DEFAULT_VIEW.member) params.set("member", v.member); if (v.group !== DEFAULT_VIEW.group) params.set("group", v.group); if (v.sort !== DEFAULT_VIEW.sort) params.set("sort", v.sort); + if (v.tab !== DEFAULT_VIEW.tab) params.set("tab", v.tab); const s = params.toString(); return s ? `?${s}` : ""; }