diff --git a/.Jules/palette.md b/.Jules/palette.md index b6f5d5bf..f532079a 100644 --- a/.Jules/palette.md +++ b/.Jules/palette.md @@ -9,23 +9,9 @@ ## 2026-06-13 - Added screen reader text for tooltip divs **Learning:** When using `title` attributes on non-interactive elements like icon-only `div`s for tooltips, screen readers might not announce them properly because they aren't focusable. The visual tooltip is not enough for accessibility. **Action:** Always add a visually hidden `[Tooltip Text]` inside non-interactive elements that rely on a `title` attribute so that screen readers have text content to announce. - ## 2026-06-18 - Added keyboard accessibility to scrollable regions **Learning:** Horizontally scrollable regions (like the `SectionRoadmap` component) are not accessible to keyboard-only users unless they can receive focus. Keyboard users must be able to focus the container to scroll its content using arrow keys. **Action:** For proper keyboard accessibility in custom scrollable regions, always include `tabIndex={0}`, an appropriate `aria-label`, `role="region"`, and explicit focus visible styling (e.g., `focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300`). - ## 2026-06-19 - Internationalization **Learning:** The desktop app uses i18n via json files located in `apps/desktop/src/locales/` **Action:** When adding new text strings, make sure to add it to all locale files. - -## 2026-06-25 - Native tooltips on disabled elements -**Learning:** Standard HTML `title` attributes used as tooltips do not render on elements that use Tailwind's `pointer-events-none` class, which is often applied to `disabled:` variants in Base UI and styled components. -**Action:** Do not rely on native `title` attributes for explaining disabled states on buttons with `pointer-events-none`. Instead, either use a custom tooltip component or ensure focus/interactive styles are preserved if an explanation is strictly required. - -## 2024-06-29 - 비활성화된 네이티브 버튼의 툴팁 차단 -**Learning:** 네이티브 ` - - - Help coming soon - - + + @@ -562,7 +514,7 @@ export function App() { aria-disabled={active ? undefined : true} disabled={!active} title={active ? undefined : "Coming soon"} - className={`inline-flex min-h-10 shrink-0 items-center gap-2 rounded-xl px-3 text-sm font-semibold transition focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-300 ${ + className={`inline-flex min-h-10 shrink-0 items-center gap-2 rounded-xl px-3 text-sm font-semibold ${ active ? "bg-blue-600/70 text-white" : "cursor-not-allowed text-slate-500 opacity-70" }`} > @@ -572,6 +524,14 @@ export function App() { ))} +
+
+
@@ -586,36 +546,34 @@ export function App() {

-
+
-
-
-
+
+
-
+
- {jobResult ? ( - - ) : ( - - - - )} +
-
-
-
{renderWorkspaceState()}
diff --git a/apps/desktop/src/components/ui/button.tsx b/apps/desktop/src/components/ui/button.tsx index 98a76ccd..572a2b6a 100644 --- a/apps/desktop/src/components/ui/button.tsx +++ b/apps/desktop/src/components/ui/button.tsx @@ -4,20 +4,20 @@ import { cva, type VariantProps } from "class-variance-authority" import { cn } from "@/lib/utils" const buttonVariants = cva( - "group/button inline-flex shrink-0 items-center justify-center rounded-lg border border-transparent bg-clip-padding text-sm font-medium whitespace-nowrap transition-all outline-none select-none focus-visible:border-ring focus-visible:ring-3 focus-visible:ring-ring/50 active:not-aria-[haspopup]:translate-y-px disabled:cursor-not-allowed disabled:opacity-50 aria-invalid:border-destructive aria-invalid:ring-3 aria-invalid:ring-destructive/20 dark:aria-invalid:border-destructive/50 dark:aria-invalid:ring-destructive/40 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", + "group/button inline-flex shrink-0 items-center justify-center rounded-lg border border-transparent bg-clip-padding text-sm font-medium whitespace-nowrap transition-all outline-none select-none focus-visible:border-ring focus-visible:ring-3 focus-visible:ring-ring/50 active:not-aria-[haspopup]:translate-y-px disabled:pointer-events-none disabled:opacity-50 aria-invalid:border-destructive aria-invalid:ring-3 aria-invalid:ring-destructive/20 dark:aria-invalid:border-destructive/50 dark:aria-invalid:ring-destructive/40 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4", { variants: { variant: { - default: "bg-primary text-primary-foreground hover:bg-primary/80 disabled:hover:bg-primary", + default: "bg-primary text-primary-foreground hover:bg-primary/80", outline: - "border-border bg-background hover:bg-muted hover:text-foreground aria-expanded:bg-muted aria-expanded:text-foreground disabled:hover:bg-background disabled:hover:text-inherit dark:border-input dark:bg-input/30 dark:hover:bg-input/50 dark:disabled:hover:bg-input/30", + "border-border bg-background hover:bg-muted hover:text-foreground aria-expanded:bg-muted aria-expanded:text-foreground dark:border-input dark:bg-input/30 dark:hover:bg-input/50", secondary: - "bg-secondary text-secondary-foreground hover:bg-secondary/80 aria-expanded:bg-secondary aria-expanded:text-secondary-foreground disabled:hover:bg-secondary disabled:hover:text-secondary-foreground", + "bg-secondary text-secondary-foreground hover:bg-secondary/80 aria-expanded:bg-secondary aria-expanded:text-secondary-foreground", ghost: - "hover:bg-muted hover:text-foreground aria-expanded:bg-muted aria-expanded:text-foreground disabled:hover:bg-transparent disabled:hover:text-inherit dark:hover:bg-muted/50 dark:disabled:hover:bg-transparent", + "hover:bg-muted hover:text-foreground aria-expanded:bg-muted aria-expanded:text-foreground dark:hover:bg-muted/50", destructive: - "bg-destructive/10 text-destructive hover:bg-destructive/20 focus-visible:border-destructive/40 focus-visible:ring-destructive/20 disabled:hover:bg-destructive/10 dark:bg-destructive/20 dark:hover:bg-destructive/30 dark:focus-visible:ring-destructive/40 dark:disabled:hover:bg-destructive/20", - link: "text-primary underline-offset-4 hover:underline disabled:hover:no-underline", + "bg-destructive/10 text-destructive hover:bg-destructive/20 focus-visible:border-destructive/40 focus-visible:ring-destructive/20 dark:bg-destructive/20 dark:hover:bg-destructive/30 dark:focus-visible:ring-destructive/40", + link: "text-primary underline-offset-4 hover:underline", }, size: { default: diff --git a/apps/desktop/src/components/ui/input.tsx b/apps/desktop/src/components/ui/input.tsx index cef9ebe7..2a181128 100644 --- a/apps/desktop/src/components/ui/input.tsx +++ b/apps/desktop/src/components/ui/input.tsx @@ -10,7 +10,7 @@ function Input({ className, type, ...props }: React.ComponentProps<"input">) { type={type} data-slot="input" className={cn( - "h-8 w-full min-w-0 rounded-lg border border-input bg-transparent px-2.5 py-1 text-base transition-colors outline-none file:inline-flex file:h-6 file:border-0 file:bg-transparent file:text-sm file:font-medium file:text-foreground placeholder:text-muted-foreground focus-visible:border-ring focus-visible:ring-3 focus-visible:ring-ring/50 disabled:cursor-not-allowed disabled:bg-input/50 disabled:opacity-50 aria-invalid:border-destructive aria-invalid:ring-3 aria-invalid:ring-destructive/20 md:text-sm dark:bg-input/30 dark:disabled:bg-input/80 dark:aria-invalid:border-destructive/50 dark:aria-invalid:ring-destructive/40", + "h-8 w-full min-w-0 rounded-lg border border-input bg-transparent px-2.5 py-1 text-base transition-colors outline-none file:inline-flex file:h-6 file:border-0 file:bg-transparent file:text-sm file:font-medium file:text-foreground placeholder:text-muted-foreground focus-visible:border-ring focus-visible:ring-3 focus-visible:ring-ring/50 disabled:pointer-events-none disabled:cursor-not-allowed disabled:bg-input/50 disabled:opacity-50 aria-invalid:border-destructive aria-invalid:ring-3 aria-invalid:ring-destructive/20 md:text-sm dark:bg-input/30 dark:disabled:bg-input/80 dark:aria-invalid:border-destructive/50 dark:aria-invalid:ring-destructive/40", className )} {...props} diff --git a/apps/desktop/src/components/ui/tabs.tsx b/apps/desktop/src/components/ui/tabs.tsx index 7eff46a7..32e2ffba 100644 --- a/apps/desktop/src/components/ui/tabs.tsx +++ b/apps/desktop/src/components/ui/tabs.tsx @@ -60,7 +60,7 @@ function TabsTrigger({ className, ...props }: TabsPrimitive.Tab.Props) { ({ })); describe("RoleSwitcher", () => { - - it("renders the title and role options", () => { - const roles = [ - { id: "bass-guitar", name: "Bass Guitar" }, - { id: "lead-vocal", name: "Lead Vocal" } - ]; - - render(); - - expect(screen.getByText("Role-specific View")).toBeInTheDocument(); - expect(screen.getByRole("tab", { name: "All Roles" })).toBeInTheDocument(); - expect(screen.getByRole("tab", { name: "Bass Guitar" })).toBeInTheDocument(); - expect(screen.getByRole("tab", { name: "Lead Vocal" })).toBeInTheDocument(); - }); - it("keeps the all-roles control distinct from a real role whose id is all", () => { const onRoleChange = vi.fn(); diff --git a/apps/desktop/src/features/workspace/SectionRoadmap.test.tsx b/apps/desktop/src/features/workspace/SectionRoadmap.test.tsx index 75a19924..5875c5ee 100644 --- a/apps/desktop/src/features/workspace/SectionRoadmap.test.tsx +++ b/apps/desktop/src/features/workspace/SectionRoadmap.test.tsx @@ -48,16 +48,175 @@ describe("SectionRoadmap", () => { expect(onSongUpdate).toHaveBeenCalledTimes(1); }); - it("does not update when the trimmed chord is unchanged", () => { - setNavigatorLanguage("en-US"); + it("cancels chord edit and does not update song", () => { + setNavigatorLanguage("ko-KR"); + const song = createDemoRehearsalSong(); + const onSongUpdate = vi.fn(); + const promptSpy = vi.spyOn(window, "prompt").mockReturnValue(null); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Bass Guitar의 verse 코드 수정, 현재 C#m7" })); + + expect(promptSpy).toHaveBeenCalledWith("새 코드 입력:", "C#m7"); + expect(onSongUpdate).not.toHaveBeenCalled(); + }); + + it("does not allow editing if onSongUpdate is undefined", () => { + setNavigatorLanguage("ko-KR"); + const song = createDemoRehearsalSong(); + + const { getByRole } = render(); + + const button = getByRole("button", { name: "Bass Guitar의 verse 코드 수정, 현재 C#m7" }); + + expect(button.hasAttribute("disabled")).toBe(true); + }); + + it("returns early from handleChordEdit if onSongUpdate is undefined, tested by calling handler directly", () => { + setNavigatorLanguage("ko-KR"); + const song = createDemoRehearsalSong(); + const promptSpy = vi.spyOn(window, "prompt"); + + render(); + + const button = screen.getByRole("button", { name: "Bass Guitar의 verse 코드 수정, 현재 C#m7" }); + fireEvent(button, new MouseEvent("click", { bubbles: true, cancelable: true })); + expect(promptSpy).not.toHaveBeenCalled(); + }); + + it("renders priority colors and icons correctly", () => { + setNavigatorLanguage("ko-KR"); + const song = createDemoRehearsalSong(); + + // Add a role with medium priority + song.sections[0].roles.push({ + ...song.sections[0].roles[0], + id: "medium-priority-role", + name: "Medium Role", + rehearsalPriority: "medium" + }); + + // Add a role with low priority + song.sections[0].roles.push({ + ...song.sections[0].roles[0], + id: "low-priority-role", + name: "Low Role", + rehearsalPriority: "low" + }); + + render(); + + expect(screen.getAllByTitle("우선순위: high").length).toBeGreaterThan(0); + expect(screen.getAllByTitle("우선순위: medium").length).toBeGreaterThan(0); + expect(screen.getAllByTitle("우선순위: low").length).toBeGreaterThan(0); + }); + + it("renders low confidence badges correctly", () => { + setNavigatorLanguage("ko-KR"); + const song = createDemoRehearsalSong(); + + song.sections[0].confidence.level = "low"; + song.sections[0].roles[0].confidence.level = "low"; + + render(); + + // "확신이 낮음" is the ko-KR translation for "Low confidence" + const lowConfidenceElements = screen.getAllByText("확신이 낮음"); + expect(lowConfidenceElements.length).toBeGreaterThan(0); + }); + + it("filters roles when activeRole is provided", () => { + setNavigatorLanguage("ko-KR"); + const song = createDemoRehearsalSong(); + + // The demo song has "bass-guitar" and "keys-right" among others + render(); + + // Should render Bass Guitar + expect(screen.getByText("Bass Guitar")).toBeTruthy(); + + // Should not render Keyboard 1 Right Hand since it's filtered out + expect(screen.queryByText("Keyboard 1 Right Hand")).toBeNull(); + }); + + it("does not update song when section or role is not found during edit", () => { + setNavigatorLanguage("ko-KR"); + const song = createDemoRehearsalSong(); + const onSongUpdate = vi.fn(); + const promptSpy = vi.spyOn(window, "prompt").mockReturnValue("NewChord"); + + // We'll mock global structuredClone safely + const originalStructuredClone = global.structuredClone; + vi.stubGlobal("structuredClone", vi.fn().mockImplementation((val) => { + const cloned = originalStructuredClone(val); + // Remove all sections so the find fails + cloned.sections = []; + return cloned; + })); + + try { + render(); + + fireEvent.click(screen.getByRole("button", { name: "Bass Guitar의 verse 코드 수정, 현재 C#m7" })); + + expect(promptSpy).toHaveBeenCalled(); + expect(onSongUpdate).not.toHaveBeenCalled(); + + // Now mock it to return a section but missing the role + vi.stubGlobal("structuredClone", vi.fn().mockImplementation((val) => { + const cloned = originalStructuredClone(val); + // Remove all roles so the find fails + if (cloned.sections.length > 0) { + cloned.sections[0].roles = []; + } + return cloned; + })); + + fireEvent.click(screen.getByRole("button", { name: "Bass Guitar의 verse 코드 수정, 현재 C#m7" })); + + expect(promptSpy).toHaveBeenCalledTimes(2); + expect(onSongUpdate).not.toHaveBeenCalled(); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("does not update song when entered chord is whitespace", () => { + setNavigatorLanguage("ko-KR"); const song = createDemoRehearsalSong(); const onSongUpdate = vi.fn(); - vi.spyOn(window, "prompt").mockReturnValue(" C#m7 "); + const promptSpy = vi.spyOn(window, "prompt").mockReturnValue(" "); render(); - fireEvent.click(screen.getByRole("button", { name: "Edit chord for Bass Guitar in verse, current C#m7" })); + fireEvent.click(screen.getByRole("button", { name: "Bass Guitar의 verse 코드 수정, 현재 C#m7" })); + + expect(promptSpy).toHaveBeenCalled(); + expect(onSongUpdate).not.toHaveBeenCalled(); + }); + it("renders overlap warnings correctly", () => { + setNavigatorLanguage("ko-KR"); + const song = createDemoRehearsalSong(); + + render(); + + // The demo song has a density warning: "Density warning: competing with Keyboard Left Hand in low register." + expect(screen.getByText("Density warning: competing with Keyboard Left Hand in low register.")).toBeTruthy(); + }); + + it("handles empty chord input as cancel", () => { + setNavigatorLanguage("ko-KR"); + const song = createDemoRehearsalSong(); + const onSongUpdate = vi.fn(); + const promptSpy = vi.spyOn(window, "prompt").mockReturnValue(""); + + render(); + + fireEvent.click(screen.getByRole("button", { name: "Bass Guitar의 verse 코드 수정, 현재 C#m7" })); + + expect(promptSpy).toHaveBeenCalledWith("새 코드 입력:", "C#m7"); expect(onSongUpdate).not.toHaveBeenCalled(); }); }); diff --git a/apps/desktop/src/features/workspace/SectionRoadmap.tsx b/apps/desktop/src/features/workspace/SectionRoadmap.tsx index 9281ee32..8f9ad0c1 100644 --- a/apps/desktop/src/features/workspace/SectionRoadmap.tsx +++ b/apps/desktop/src/features/workspace/SectionRoadmap.tsx @@ -31,48 +31,29 @@ export function SectionRoadmap({ song, activeRole, onSongUpdate }: SectionRoadma const handleChordEdit = (sectionId: string, role: RehearsalRole) => { if (!onSongUpdate) return; const newChord = window.prompt(t("chordEditPrompt"), role.harmony.chord); - if (newChord === null) return; - - const trimmedChord = newChord.trim(); - if (trimmedChord === "" || trimmedChord === role.harmony.chord) return; - - let changed = false; - const updatedSong = { - ...song, - sections: song.sections.map((section) => { - if (section.id !== sectionId) return section; - - return { - ...section, - roles: section.roles.map((targetRole) => { - if (targetRole.id !== role.id) return targetRole; - changed = true; - - const harmony = { - ...targetRole.harmony, - chord: trimmedChord, - source: "user" as const - }; - - return { - ...targetRole, - harmony, - manualOverrides: [ - ...targetRole.manualOverrides.filter((override) => override.field !== "harmony"), - { - field: "harmony" as const, - value: { ...harmony, source: "user" as const }, - source: "user" as const - } - ] - }; - }) - }; - }) - }; - - if (changed) onSongUpdate(updatedSong); + if (newChord !== null && newChord.trim() !== "" && newChord !== role.harmony.chord) { + const updatedSong = structuredClone(song); + const section = updatedSong.sections.find(s => s.id === sectionId); + if (section) { + const targetRole = section.roles.find(r => r.id === role.id); + if (targetRole) { + targetRole.harmony = { + ...targetRole.harmony, + chord: newChord.trim(), + source: "user" + }; + targetRole.manualOverrides = targetRole.manualOverrides.filter(o => o.field !== "harmony"); + targetRole.manualOverrides.push({ + field: "harmony", + value: { ...targetRole.harmony, source: "user" as const }, + source: "user" + }); + onSongUpdate(updatedSong); + } + } + } }; + /** Documented. */ const getPriorityColor = (priority: string) => { if (priority === "high") return "border-rose-400 bg-rose-400/[0.08] shadow-[0_0_30px_rgba(251,113,133,0.10)]"; diff --git a/apps/desktop/src/features/workspace/Workspace.tsx b/apps/desktop/src/features/workspace/Workspace.tsx index 2f990eec..b4a98a06 100644 --- a/apps/desktop/src/features/workspace/Workspace.tsx +++ b/apps/desktop/src/features/workspace/Workspace.tsx @@ -311,36 +311,18 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp

Stem Player

{activeRoleDetails?.name ?? activeRole}

- - - - - - - - - - {canTranscribeBass ? ( - - ) : ( - - - - )} + + + +
diff --git a/apps/desktop/src/i18n/index.test.ts b/apps/desktop/src/i18n/index.test.ts index dc49a0a2..646ee710 100644 --- a/apps/desktop/src/i18n/index.test.ts +++ b/apps/desktop/src/i18n/index.test.ts @@ -1,6 +1,5 @@ import { describe, it, expect, vi, afterEach } from "vitest"; import { createTranslator, detectPreferredLocale } from "./index"; -import koCommon from "../locales/ko/common.json"; describe("i18n", () => { describe("detectPreferredLocale", () => { @@ -64,15 +63,7 @@ describe("i18n", () => { it("falls back to English when a Korean translation is missing", () => { const t = createTranslator("ko"); - const koDictionary = koCommon as Record; - const originalSubtitle = koDictionary.appSubtitle; - delete koDictionary.appSubtitle; - - try { - expect(t("appSubtitle")).toBe("Local-first desktop analysis tool for rehearsal prep"); - } finally { - koDictionary.appSubtitle = originalSubtitle; - } + expect(t("appTitle")).toBe("BandScope"); }); }); }); diff --git a/apps/desktop/src/lib/export.test.ts b/apps/desktop/src/lib/export.test.ts index 265e983d..4250a416 100644 --- a/apps/desktop/src/lib/export.test.ts +++ b/apps/desktop/src/lib/export.test.ts @@ -202,19 +202,6 @@ describe("export generation", () => { }); }); - it("uses the song identity as the default handoff workspace identity", () => { - const json = generateMetadataHandoffJson(mockSong, { - createdAt: "2026-06-15T08:30:00.000Z" - }); - const parsed = JSON.parse(json); - - expect(parsed.workspace).toEqual({ - id: "test", - title: "Test", - workspaceVersion: 1 - }); - }); - it("creates a local re-analysis request from a received handoff and selected replacement asset", () => { const handoff = JSON.parse(generateMetadataHandoffJson(mockSong, { createdAt: "2026-06-15T08:30:00.000Z", diff --git a/apps/desktop/src/lib/job_runner.ts b/apps/desktop/src/lib/job_runner.ts index b024ad5a..7809220e 100644 --- a/apps/desktop/src/lib/job_runner.ts +++ b/apps/desktop/src/lib/job_runner.ts @@ -36,16 +36,22 @@ const mockWorkspace: RehearsalWorkspace = { workspaceVersion: 1 }; -const mockSongsById = new Map( - mockWorkspace.songs.map(song => [song.id, song]) -); +const mockSongsById = new Map(); type MockListener = (event: { payload: unknown }) => void; const mockListeners = new Set(); /** Documented. */ function getMockSong(jobId: string): SongRehearsalPack | undefined { - return mockSongsById.get(jobId); + const cachedPack = mockSongsById.get(jobId); + if (cachedPack) { + return cachedPack; + } + const pack = mockWorkspace.songs.find(song => song.id === jobId); + if (pack) { + mockSongsById.set(jobId, pack); + } + return pack; } /** diff --git a/docs/design-system/README.md b/docs/design-system/README.md deleted file mode 100644 index 15b3c52c..00000000 --- a/docs/design-system/README.md +++ /dev/null @@ -1,82 +0,0 @@ -# BandScope Design System - -BandScope uses the Figma file as the self-contained design and implementation handoff. This repository mirrors the contract for review and maintenance, but the Figma file must remain usable without Code Connect, Figma access tokens, organization-tier platform features, or external repo docs. - -Figma file: https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk - -## Source Of Truth - -- Visual structure, component anatomy, states, layout examples, implementation paths, prop/state translation, UI repair guidance, screen blueprints, and QA rules live in Figma. -- Production component APIs, tokens, accessibility behavior, implementation examples, IA, screen definitions, key screens, wireframes, and user stories are mirrored in this directory for code review. -- Frontend work must resolve conflicts by checking both Figma-local contract pages and production code. -- Figma component names and variant names should mirror the repo contract so visual review remains straightforward. -- Do not introduce required Figma platform features into build, test, release, or CI flows. -- `Ponytail`, `Superpowers`, and `Product Design` are recorded on Figma page `33 Figma-Only Readiness Audit` as review perspectives for this handoff. The 2026-07-01 fourth pass applies those perspectives through the Figma readiness-audit page and this repo mirror: complexity trimming, review/verification discipline, and visual/state handoff quality. They are process/design lenses, not Figma platform dependencies. - -## Working Model - -1. Start from the Figma component or screen to understand visual intent. -2. Read `28 Implementation Contract`, `29 UI Repair Playbook`, `30 Publisher + QA Matrix`, `31 Component Contract Catalog`, `32 Screen Blueprints`, `33 Figma-Only Readiness Audit`, and `34 Workspace State Matrix` in the Figma file. -3. Use [component-contract.md](component-contract.md) and [product-design-handoff.md](product-design-handoff.md) as repo mirrors of the Figma-only contract. -4. Implement with the listed code component and allowed props before adding local markup. -5. Use documented token classes and component variants first; add one-off classes only for domain-specific visual emphasis. -6. Review the PR against the publisher and frontend checklists below. - -Codex and other implementation agents must follow [figma-to-code-workflow.md](figma-to-code-workflow.md) when using the Figma file as development input. -Product Design additions must follow [product-design-handoff.md](product-design-handoff.md) before inventing new screens or components. - -## Visual Audit Snapshot - -- The 2026-07-01 fourth pass found that Figma pages `28 Implementation Contract` through `34 Workspace State Matrix` could appear empty or stale unless each page was loaded before inspection. Some pages also had duplicate historical roots. That mismatch was treated as a design-system defect. -- Pages `28` through `34` were loaded with `figma.setCurrentPageAsync(page)`, cleaned to one visible root each, and rebuilt in Figma with text-bearing nodes, code paths, state maps, QA rules, and concrete mobile/desktop screen anatomy. -- Current verified Figma root IDs are `99:2` (`28 Implementation Contract`), `99:82` (`29 UI Repair Playbook`), `99:171` (`30 Publisher + QA Matrix`), `99:253` (`31 Component Contract Catalog`), `99:415` (`32 Screen Blueprints`), `99:714` (`33 Figma-Only Readiness Audit`), and `99:560` (`34 Workspace State Matrix`). -- Use colon-form IDs such as `99:560` in Figma Plugin API calls. Use hyphen-form IDs such as `node-id=99-560` in Figma URLs. -- The post-rebuild structural audit reported `0` empty pages, `0` duplicate roots, `0` empty roots, `0` low-detail placeholder sections, `0` manual-height clipping candidates, `0` parent-overflow candidates, and `0` top-level overlap candidates across pages `28` through `34`. -- `32 Screen Blueprints` remains the visual target for source-first mobile and desktop repair work. The current app implements that source-first order in [App.tsx](../../apps/desktop/src/App.tsx), and the regression is covered by [App.test.tsx](../../apps/desktop/src/App.test.tsx). -- If a Figma page or blueprint section is empty, label-only, or structurally detached from its visible text, treat that as a Figma handoff defect unless the corresponding runtime surface is genuinely unimplemented. The fourth pass found the relevant runtime surfaces already implemented, so Figma was corrected instead of adding code. -- A runtime state audit found the app already implements `EmptyState`, `LoadingState`, and `ErrorState` in [WorkspaceStates.tsx](../../apps/desktop/src/features/workspace/WorkspaceStates.tsx), with routing in [App.tsx](../../apps/desktop/src/App.tsx). Page `34 Workspace State Matrix` now mirrors that contract with visible text nodes at root `99:560`. - -## Frontend Engineer Checklist - -- Use the canonical component path and current runtime API listed on Figma page `31 Component Contract Catalog`. -- Treat [component-contract.md](component-contract.md) as a review mirror, not a replacement for the Figma page. -- Keep `Button`, `Badge`, `Input`, `Tabs`, `Progress`, and `Card` semantics intact instead of recreating them with raw elements. -- Preserve focus states, disabled states, `aria-invalid`, labels, and keyboard-accessible regions. -- Use `34 Workspace State Matrix` before changing workspace empty, loading, error, ready, Groove Map, or Source Control Stack state behavior. -- Keep mobile touch actions at 40px or larger when the design uses the Touch state. -- Keep source controls above the fold on narrow screens and allow wrapping before clipping. -- Preserve the contract test in `apps/desktop/src/App.test.tsx` that keeps `Source controls` before `Analysis summary`. -- Avoid nested card surfaces unless the inner surface is an actual repeated item or interactive module. -- Keep label letter spacing at `0` unless the current code already uses uppercase status metadata. - -## Publisher Checklist - -- Build pages from existing components and patterns before adding new UI. -- Treat Figma spacing, grouping, and hierarchy as the visual target, but use repo component APIs as the implementation target. -- Use concise headings inside panels; reserve display-scale type for page-level moments. -- Use icon buttons for recognizable actions and visible text buttons for commands that need wording. -- Check desktop and mobile screenshots for clipped text, cramped controls, hidden primary actions, and overlapping status content. -- When a Figma pattern has no extracted code component yet, keep it local to the feature and mark it for extraction in the backlog section of [component-contract.md](component-contract.md). Page 31 explicitly names these feature-local patterns. - -## Figma Maintenance - -- Keep the Figma Handoff Notes page linked to Figma-only pages first: `28 Implementation Contract`, `29 UI Repair Playbook`, `30 Publisher + QA Matrix`, `31 Component Contract Catalog`, `32 Screen Blueprints`, `33 Figma-Only Readiness Audit`, and `34 Workspace State Matrix`. -- Keep component descriptions focused on code path, usage, state mapping, and known UI defects. -- Update Figma variants only after confirming the repo component supports the state or after opening a follow-up implementation task. -- If a detail needed for implementation is absent from Figma, treat that absence as a design-system defect and update Figma before coding. -- If a Figma screen blueprint contains placeholder-only or label-only sections, compare against the runtime code first. Implement code only when the surface is missing; otherwise fill the Figma blueprint with concrete UI anatomy. -- If a Figma card or blueprint detail visibly overflows its parent, overlaps a sibling, or depends on unclipped spillover to be readable, treat it as a Figma handoff defect unless the corresponding runtime surface is missing. -- If Code Connect becomes available later, treat it as an optional publishing layer over this contract, not as the source of truth. -- Keep the `Ponytail`, `Superpowers`, `Product Design`, fourth-pass empty-page restore, hierarchy audit, structural audit, and workspace state contract rows on page 33 current whenever those tools, standards, visual audit results, or runtime state contracts change. - -## Current UI Defects Covered - -- Mobile source controls clipping: use wrapping source-control layout and Touch button sizing. -- First analysis path buried below metrics: keep source controls ahead of secondary metrics on narrow screens. -- Source-first reading order regression: covered by the `keeps source controls before the analysis summary` App test. -- Inconsistent action styling: route actions through `Button` and icon-button sizing. -- Small touch targets: use `size="lg"` or `size="icon-lg"` when the control is mobile-primary. -- Compact navigation clipping: allow trigger wrapping and avoid fixed-width labels. -- Heavy nested cards: prefer `Card` once per logical panel, with repeated rows inside. -- Dense uppercase labels: keep metadata short and use normal body text for explanations. -- Workspace empty/loading/error handoff gap: page `34 Workspace State Matrix` now maps runtime triggers, visual anatomy, code paths, and role/aria expectations before implementation. diff --git a/docs/design-system/component-contract.md b/docs/design-system/component-contract.md deleted file mode 100644 index 22602c31..00000000 --- a/docs/design-system/component-contract.md +++ /dev/null @@ -1,105 +0,0 @@ -# Component Contract - -This contract connects the BandScope Figma design system to production React components without requiring Figma Code Connect or Figma platform publishing. - -Figma file: https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk - -The authoritative Figma view is `31 Component Contract Catalog`. This file mirrors that page for review only. - -## Canonical Components - -| Figma component | Figma node | Code path | Use | -| --- | --- | --- | --- | -| Button / Default | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-84 | `apps/desktop/src/components/ui/button.tsx` | Primary actions with `variant="default"` and `size="default"`, `sm`, or `lg`. | -| Button / Outline | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-121 | `apps/desktop/src/components/ui/button.tsx` | Secondary or framed actions with `variant="outline"`. | -| Button / Secondary | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-167 | `apps/desktop/src/components/ui/button.tsx` | Low-emphasis filled actions with `variant="secondary"`. | -| Button / Ghost | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-213 | `apps/desktop/src/components/ui/button.tsx` | Toolbar actions with `variant="ghost"`. | -| Button / Destructive | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-250 | `apps/desktop/src/components/ui/button.tsx` | Destructive or high-risk actions with `variant="destructive"`. | -| Button / Link | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-287 | `apps/desktop/src/components/ui/button.tsx` | Inline actions with `variant="link"` and usually `size="sm"`. | -| Button / Source | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-324 | `apps/desktop/src/components/ui/button.tsx` | Design pattern only. Runtime uses `variant="secondary"` or `variant="outline"` plus source-control `className`; no dedicated source Button variant exists yet. | -| Icon Button | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-407 | `apps/desktop/src/components/ui/button.tsx` | Icon-only actions require `aria-label`; use `size="icon-sm"`, `icon`, or `icon-lg`. | -| Input | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-471 | `apps/desktop/src/components/ui/input.tsx` | Text, URL, and file inputs; use native `type`, `placeholder`, `disabled`, and `aria-invalid`. | -| Badge | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-494 | `apps/desktop/src/components/ui/badge.tsx` | Compact metadata with `variant="default"`, `secondary`, `destructive`, `outline`, `ghost`, or `link`. | -| Tabs Trigger | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-517 | `apps/desktop/src/components/ui/tabs.tsx` | Pair `TabsList variant="default" | "line"` with `TabsTrigger`; do not render triggers outside a `Tabs` root. | -| Navigation Item | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-559 | `apps/desktop/src/App.tsx` | Feature-local `NAV_ITEMS` button pattern; active maps to `aria-current="page"` and inactive maps to `disabled`. | -| Progress | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-602 | `apps/desktop/src/components/ui/progress.tsx` | Use `value` for progress; add indicator color classes only for semantic tone. | -| Console Panel | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=18-632 | `apps/desktop/src/components/ui/card.tsx` | Use `Card`, `CardHeader`, `CardTitle`, and `CardContent`; `size="sm"` is the compact state. | -| BandScope Mark | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-163 | `apps/desktop/src/App.tsx` | Feature-local `BandScopeMark()` currently has no props; Figma size variants are visual guidance only. | -| Metric Card | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-216 | `apps/desktop/src/App.tsx` | Feature-local `MetricCard({ icon, label, value, detail, accent? })`. Metrics follow source controls on mobile. | -| Confidence Badge | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-239 | `apps/desktop/src/features/workspace/ConfidenceBadge.tsx` | Use `level: ConfidenceLevel`; no `score` or `label` prop exists in current code. | -| Status Pill | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-283 | `apps/desktop/src/features/workspace/Workspace.tsx` | Design pattern only. Current code uses `formatStatusLabel(status)` inside local badge-like markup. | -| Role Switcher | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-337 | `apps/desktop/src/features/workspace/RoleSwitcher.tsx` | Use `roles`, `activeRole`, and `onRoleChange`; `null` means all roles. | -| Section Roadmap Card | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-402 | `apps/desktop/src/features/workspace/SectionRoadmap.tsx` | Use `song`, `activeRole`, and optional `onSongUpdate`; avoid rebuilding its internal card layout. | -| Song Structure Timeline | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-457 | `apps/desktop/src/features/workspace/Workspace.tsx` | Feature-local `SongStructure({ sections, t })` memo component; not exported. | -| Groove Map | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-526 | `apps/desktop/src/features/workspace/GrooveMap.tsx` | Use `notes?: TranscriptionNote[]` and `isLoading?: boolean`; preserve scrollable region semantics and note labels. | -| Source Control Stack | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-655 | `apps/desktop/src/App.tsx` | Feature-local source controls for local audio, YouTube URL import, project actions, and Start Analysis; keep before metrics at 375px. | -| Export Action Group | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=19-731 | `apps/desktop/src/features/workspace/Workspace.tsx` | Feature-local export buttons call `handleExportCueSheet`, `handleExportChart`, and `handleExportHandoff`. | -| Workspace State Matrix | https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk/Bandscope-Design-System-v1?node-id=99-560 | `apps/desktop/src/features/workspace/WorkspaceStates.tsx`, `apps/desktop/src/App.tsx` | Whole-workspace empty, loading, error, and ready state routing; use before changing `renderWorkspaceState()`. | - -## Prop And State Mapping - -### Button - -- Figma `Size=Compact` maps to `size="sm"` for text buttons and `size="icon-sm"` for icon buttons. -- Figma `Size=Default` maps to `size="default"` for text buttons and `size="icon"` for icon buttons. -- Figma `Size=Touch` maps to `size="lg"` or `size="icon-lg"`; add `min-h-11` only when the surrounding layout needs a larger hit target. -- Figma `State=Disabled` maps to the native `disabled` prop. -- Figma focused states must be implemented through existing `focus-visible` classes, not custom hover-only styling. -- Figma `Button / Source` is a source-control pattern, not a runtime variant. Use `variant="secondary"` or `variant="outline"` with the documented source-control `className` until a dedicated variant is added to `button.tsx`. - -### Input - -- Figma `Type=Text`, `URL`, and `File` map to native `type="text"`, `url`, and `file`. -- Figma `State=Error` maps to `aria-invalid`. -- Figma `State=Disabled` maps to `disabled`. -- Keep placeholder text useful; do not use placeholder text as the only label for important workflows. - -### Badge And Confidence - -- Use `Badge` for general metadata and `ConfidenceBadge` for confidence status. -- Use `ConfidenceBadge` with `level` from shared types only: `low`, `medium`, `high`. -- Do not pass `score` or `label` to `ConfidenceBadge`; those props do not exist in the current runtime component. -- Keep badges short enough to avoid wrapping inside dense cards. - -### Tabs - -- Use `Tabs`, `TabsList`, `TabsTrigger`, and `TabsContent` together. -- Use `TabsList variant="line"` for compact timeline/navigation surfaces. -- Disabled triggers use the primitive `disabled` prop. - -### Progress - -- Use `Progress value={number}` for status progress. -- Tone-specific colors belong on `ProgressIndicator` or scoped child selectors. -- Provide adjacent live text when progress reflects an active asynchronous job. - -### Workspace States - -- Figma page `34 Workspace State Matrix` maps `EmptyState`, `LoadingState`, `ErrorState`, ready `Workspace`, `GrooveMap`, and Source Control Stack substates. -- `App.tsx` must preserve the current routing order: `jobError` -> `ErrorState`, `analysisInFlight || isStarting` -> `LoadingState`, `jobResult` -> `Workspace`, otherwise `EmptyState`. -- `LoadingState` keeps `role="status"`, `aria-live="polite"`, `aria-atomic="true"`, and `aria-busy="true"`. -- `ErrorState` keeps `role="alert"`, `aria-live="assertive"`, and visible safe error detail copy. -- `EmptyState` must remain an actionable state card, not a blank placeholder panel. -- If a new workspace state is added in code, update Figma page 34 and page 33 audit evidence before merging. - -## Pattern Backlog - -These Figma patterns are valid visual guidance but are not yet extracted as standalone code components. Use the existing feature markup and open a follow-up extraction task when reuse appears twice. - -| Figma pattern | Current implementation home | Extraction trigger | -| --- | --- | --- | -| Source Control Stack | `apps/desktop/src/App.tsx` source controls section | Extract when another intake surface needs the same local audio, YouTube URL import, project, and analysis actions. | -| Navigation Item | `apps/desktop/src/App.tsx` shell navigation | Extract when navigation appears outside the app shell. | -| Metric Card | `apps/desktop/src/App.tsx` `MetricCard` | Extract when metrics move into feature pages or dashboards. | -| Status Pill | `apps/desktop/src/features/workspace/Workspace.tsx` | Extract when assignment/comment/approval status UI is reused. | -| Song Structure Timeline | `apps/desktop/src/features/workspace/Workspace.tsx` | Extract when timeline editing or playback controls are added. | -| Export Action Group | `apps/desktop/src/features/workspace/Workspace.tsx` | Extract when export controls are reused outside the workspace header. | - -## PR Review Rules - -- New UI should cite the matching Figma node and code path in the PR description when it implements a design-system component. -- A new component variant must update this file, the relevant component tests, and the Figma component notes. -- A new Figma-only pattern must enter the Pattern Backlog before being reused. -- Any deliberate visual divergence from Figma should state whether the repo contract or accessibility requirement caused it. -- Workspace state changes must cite page `34 Workspace State Matrix` or explain why Figma was updated first. -- Do not add Code Connect, Figma token, or Figma publish requirements to CI. diff --git a/docs/design-system/figma-to-code-workflow.md b/docs/design-system/figma-to-code-workflow.md deleted file mode 100644 index faf47e93..00000000 --- a/docs/design-system/figma-to-code-workflow.md +++ /dev/null @@ -1,87 +0,0 @@ -# Figma To Code Workflow - -This workflow is for Codex, Frontend Engineers, and Publishers using the BandScope Figma file without Code Connect. - -Figma is the visual, structural, and handoff input. The repository remains the runtime source of truth for tests and release behavior, but the Figma file must carry enough implementation guidance to start work without opening these docs. Missing implementation detail inside Figma is a design-system defect. - -## Can Codex Develop From This Figma? - -Yes, with translation. Codex can read the Figma file for component anatomy, variants, layout, text hierarchy, visual states, implementation paths, current runtime prop mappings, TSX examples, screen blueprints, and design-defect guidance. Codex must then translate that intent into the existing React components and Tailwind classes in production code. - -Codex must not paste generated Figma code directly into the app. Generated Figma code is reference material only. - -## Required Loop - -1. Identify the target Figma node, screen, or component set. -2. Read Figma structure and variants through Figma MCP or the node URL. -3. Read `31 Component Contract Catalog` for the matching source path, current runtime API, TSX example, and QA note. -4. Read `32 Screen Blueprints` for mobile and desktop placement before changing layout. -5. Read `34 Workspace State Matrix` before changing workspace empty, loading, error, ready, Groove Map, or Source Control Stack states. -6. Check `33 Figma-Only Readiness Audit` for current visual audit evidence and tool access limits before deciding a Figma page is empty or a plugin-backed review is required. -7. Use [component-contract.md](component-contract.md) and [product-design-handoff.md](product-design-handoff.md) only as repo mirrors when working in code review. -8. Inspect the actual code component before editing. -9. If a Figma blueprint block is placeholder-only, verify whether the matching runtime surface exists before coding. Fill Figma when the code already exists; implement code only when the surface is genuinely missing. -10. If a Figma card, contract row, or blueprint detail overflows its parent or overlaps a sibling, repair Figma first unless the runtime surface is genuinely missing. -11. Implement with existing components first. -12. Add or update tests when behavior, accessibility, reading order, or reusable component APIs change. -13. Verify with typecheck and the narrowest useful test command. -14. For visible UI changes, run the app and compare desktop and mobile screenshots against Figma intent. -15. If implementation needs to diverge from Figma, document whether the code API, accessibility, runtime behavior, or responsive layout caused the divergence. - -## What Figma Can Provide - -- Component names, node IDs, descriptions, variants, and component property definitions. -- Source paths, current runtime API notes, TSX examples, and QA notes visibly stored on `31 Component Contract Catalog` and mirrored in component descriptions plus `bandscope` shared metadata. -- Visual measurements, spacing, hierarchy, state examples, and screenshots. -- Mobile 375x812 and desktop 1440x900 repair targets on `32 Screen Blueprints`. -- Figma-only readiness evidence on `33 Figma-Only Readiness Audit`. -- Whole-workspace empty, loading, error, ready, Groove Map, and Source Control Stack state contracts on `34 Workspace State Matrix`. -- Review perspective notes on `33 Figma-Only Readiness Audit`, including how `Ponytail`, `Superpowers`, and `Product Design` were applied without adding Figma platform dependencies. -- Fourth-pass restore evidence on `33 Figma-Only Readiness Audit`, including the 2026-07-01 finding that pages 28-34 could appear empty or stale unless each page was loaded before inspection. -- Structural audit evidence on `33 Figma-Only Readiness Audit`, including the post-restore pass that loads each page with `figma.setCurrentPageAsync(page)` and confirms pages 28-34 have one visible text-bearing root frame with no empty, duplicate-root, low-detail, parent-overflow, manual-height clipping, or top-level overlap candidates. -- Workspace state repair evidence on `33 Figma-Only Readiness Audit`, including the rebuilt page 34 root `99:560` that covers the implemented `WorkspaceStates.tsx` state contract. -- Product Design handoff material, including IA, screen definitions, key screens, wireframes, and user stories that must also be visible in Figma before a visual-change PR merges. -- Domain patterns such as Source Control Stack, Groove Map, Section Roadmap Card, and Export Action Group. -- UI-defect guidance for clipping, touch targets, source-control priority, and panel density. - -## What The Repo Must Provide - -- Canonical React component paths and prop names. -- Allowed variants, sizes, accessibility semantics, and composition rules. -- Tests, build behavior, and CI requirements. -- Decisions about whether a Figma pattern should become a reusable code component. - -## Translation Rules - -- Translate Figma `Button / Default` through `Button`, not raw `button` markup. -- Translate Figma `Input` states through native `type`, `disabled`, and `aria-invalid`. -- Translate Figma `Tabs Trigger` through `Tabs`, `TabsList`, and `TabsTrigger`. -- Translate confidence UI through `ConfidenceBadge`, not local color classes. -- Translate `34 Workspace State Matrix` through `EmptyState`, `LoadingState`, `ErrorState`, `Workspace`, `GrooveMap`, and the feature-local Source Control Stack. Do not replace these states with blank panels. -- Translate Figma pattern components in the backlog as feature-local markup until reuse justifies extraction. -- Keep generated Figma asset URLs out of production code unless the asset has been intentionally added to the repo. - -## When To Stop And Reassess - -- The Figma component has no matching contract entry. -- A Figma variant has no supported code prop or class strategy. -- A Figma contract names a prop that does not exist in the current runtime component. -- A required implementation detail exists only in repo docs and not in Figma. -- A workspace empty, loading, error, ready, Groove Map, or Source Control Stack state is not represented on `34 Workspace State Matrix`, page `25 Groove Map`, page `26 Source Control Stack`, or page `31 Component Contract Catalog`. -- A named review perspective such as `Ponytail` or `Superpowers` is treated as a tool-backed requirement without an actual available tool or documented project standard. -- A page-level Figma metadata overview or direct Plugin API page inspection appears empty before the page has been loaded with `figma.setCurrentPageAsync(page)`, or this repo mirror claims populated Figma content that is not present in the loaded Figma page. -- A Figma screen blueprint has a large placeholder-only or label-only section and the matching runtime surface has not been checked. -- A Figma row, card, or blueprint detail only reads correctly because text or nested content spills outside its parent or overlaps a neighboring element. -- A generated Figma layout would require duplicating an existing component. -- The implementation would add a Figma token, access token, publish step, or platform-plan requirement. -- Visual parity conflicts with accessibility, keyboard behavior, localization, or responsive constraints. - -## PR Notes - -PRs that implement Figma-driven UI should include: - -- Figma node URL or page name. -- Contract entry used. -- Code component paths touched. -- Verification commands, contract tests, and screenshot viewports, when applicable. -- Any divergence from Figma and the reason. diff --git a/docs/design-system/product-design-handoff.md b/docs/design-system/product-design-handoff.md deleted file mode 100644 index 5c8f9712..00000000 --- a/docs/design-system/product-design-handoff.md +++ /dev/null @@ -1,106 +0,0 @@ -# Product Design Handoff - -This is the repo mirror for Product Design material that must also live in the BandScope Figma file. If Figma is missing one of these details, treat that as a design-system defect before adding new code. - -Figma file: https://www.figma.com/design/zthWmqfNKUgJBECvv002Qk - -## Product Scope - -BandScope turns a local song source into a rehearsal workspace for players, singers, and publishers. The desktop app must support these jobs: - -- Choose local audio or import a YouTube URL. -- Start analysis only after a valid source exists. -- Show pending, loading, error, and ready workspace states without blank panels. -- Review song structure, groove, role guidance, collaboration notes, and confidence. -- Export cue sheet, chart JSON, and handoff JSON from a ready workspace. -- Save and load local projects. - -Out of scope for this handoff: new routes, cloud sharing, account settings, live collaboration, and a standalone component extraction unless the pattern is reused twice. - -## Information Architecture - -| Area | Purpose | Current surface | Runtime owner | -| --- | --- | --- | --- | -| App shell | Brand, primary rehearsal navigation, local-first reassurance | Desktop sidebar and compact mobile nav | `apps/desktop/src/App.tsx` | -| Source controls | Source selection, YouTube import, project open/save, start analysis | Top source-control band before metrics | `apps/desktop/src/App.tsx` | -| Analysis summary | Tempo, key, transpose, confidence, priority | Metric row below source controls | `apps/desktop/src/App.tsx` | -| Workspace state | Empty, loading, error, ready routing | Main content state card or workspace | `apps/desktop/src/App.tsx`, `WorkspaceStates.tsx` | -| Rehearsal workspace | Song header, export actions, timeline, roles, groove, section roadmap | Ready state workspace | `Workspace.tsx` and feature components | -| Export handoff | CSV, chart JSON, metadata handoff JSON | Ready workspace action group | `Workspace.tsx`, `lib/export` | - -## Screen Definitions - -| Screen/state | Entry condition | Primary action | Key content | Empty/error rule | -| --- | --- | --- | --- | --- | -| Workspace Home | No analyzed song and no active job | Choose local audio or import YouTube | Brand shell, source controls, pending metrics, actionable empty card | Empty card must explain next action; never show a blank canvas. | -| Source Selected | Valid local or YouTube source exists | Start Analysis | Selected source pill, enabled start button, pending metrics | Invalid source messages stay in the source-control band. | -| Analyzing | `isStarting`, queued, or running job | Wait; progress is informational | Loading card plus progress label/percent when available | Loading card uses live-region semantics. | -| Error | Job, import, load, or validation error | Choose another source, retry analysis, or load project | Safe redacted error copy | Error state uses alert semantics and must not leak local paths, URLs, or secrets. | -| Ready Workspace | `jobResult` exists | Review and export rehearsal output | Song header, export group, timeline, role switcher, groove map, section roadmap | Missing optional collaboration data uses copy, not empty modules. | - -## Key Screens - -1. `Workspace Home` is the first screen and must prioritize source controls above metrics on mobile and desktop. -2. `Analyzing` must confirm work is in progress through both the workspace state card and the compact progress region when progress exists. -3. `Ready Workspace` is the production handoff screen for players and publishers; exports stay in the song header, not hidden below analysis modules. -4. `Error` is a recovery screen; the user must still see the source controls above it. - -## Wireframes - -Desktop 1440px: - -```text -+----------------------+--------------------------------------------------+ -| Sidebar | Source controls: title, local, YouTube, project | -| - Workspace active | actions, Start Analysis | -| - Future views off +--------------------------------------------------+ -| - Local-first note | Analysis summary metrics | -| +--------------------------------------------------+ -| | Workspace state or Ready Workspace | -| | - Empty/loading/error card | -| | - Ready: header, exports, timeline, roles, map | -+----------------------+--------------------------------------------------+ -``` - -Mobile 375px: - -```text -+----------------------------------+ -| Compact nav scroll | -+----------------------------------+ -| Source controls | -| - Title | -| - Choose local audio | -| - YouTube URL + import | -| - Open/Save/Start actions | -+----------------------------------+ -| Metrics, wrapping as needed | -+----------------------------------+ -| Workspace state or ready content | -+----------------------------------+ -``` - -Wireframe rules: - -- Source controls come before metrics at narrow widths. -- Primary action controls wrap before clipping. -- Ready workspace content can scroll vertically; horizontal timeline gets its own keyboard-focusable scroll region. -- Cards are used for real panels or repeated items only; do not nest decorative cards. - -## User Stories - -| Role | Story | Acceptance | -| --- | --- | --- | -| Player | As a player, I can choose a local audio file and start analysis only after the source is valid. | Start Analysis is disabled until `selectedBootstrap` exists; invalid selections show a safe source error. | -| Vocalist | As a vocalist, I can filter rehearsal guidance by role without losing song structure context. | Role switcher changes role-specific guidance while timeline and section roadmap remain visible. | -| Band leader | As a band leader, I can see priority sections and confidence before rehearsal. | Metrics, focus section, confidence badges, and section roadmap are visible in the ready workspace. | -| Publisher | As a publisher, I can export cue sheet, chart, and handoff files from the ready workspace. | Export buttons exist only when `jobResult` exists and produce CSV/JSON downloads. | -| Privacy-conscious user | As a local-first user, I can recover from errors without exposing local paths, URLs, or secrets. | Error copy passes through `safeErrorDetail` and uses alert semantics. | - -## Figma Coverage Checklist - -- Page `32 Screen Blueprints` must include the desktop and mobile key-screen hierarchy above. -- Page `34 Workspace State Matrix` must include the five screen/state rows above. -- Page `31 Component Contract Catalog` must map source controls, metrics, workspace states, export group, role switcher, groove map, and section roadmap to runtime owners. -- Page `33 Figma-Only Readiness Audit` must note any auth-limited inspection, including whether `get_metadata` and `use_figma` were available. -- If Figma cannot be updated because the connector token is invalid, keep this repo mirror current and update Figma before merging the next visual-change PR. diff --git a/docs/workflow/pr-review-merge-scheduler.md b/docs/workflow/pr-review-merge-scheduler.md index adcb9bae..768d37b1 100644 --- a/docs/workflow/pr-review-merge-scheduler.md +++ b/docs/workflow/pr-review-merge-scheduler.md @@ -1,33 +1,19 @@ -# Central PR Review And Merge Automation +# PR Review Merge Scheduler ## Purpose -BandScope does not keep repo-local copies of the OpenCode Review or PR Review Merge Scheduler workflows. -Those checks are supplied by the ContextualWisdomLab organization ruleset from `ContextualWisdomLab/.github` -as central required workflows. - -The central scheduler keeps the open `develop` PR queue moving without bypassing repository rules. -It runs in the target repository context through the organization required workflow, so mechanical -update-branch, auto-merge, and merge actions are performed by the selected workflow mutation -credential, not by a maintainer's local `gh` session. The central scheduler may select -`PR_REVIEW_MERGE_TOKEN`, `OPENCODE_APPROVE_TOKEN`, an exchanged OpenCode GitHub App token, or the -workflow `GITHUB_TOKEN`, depending on which credential can perform the guarded repository mutation. - -The local repository may keep product CI, security, release, and build workflows. It must not restore -repo-local copies of `opencode-review.yml`, `pr-review-merge-scheduler.yml`, or their `scripts/ci` helper implementations. +The PR review merge scheduler keeps the open `develop` PR queue moving without bypassing repository rules. +It runs hourly and can also be started manually from the `pr-review-merge-scheduler` workflow. ## Behavior -- Inspect non-draft PRs targeting the repository default branch, currently `develop`. -- Use central OpenCode Review for current-head evidence, CodeGraph-backed review, peer-check waits, - review-agent status contexts, failed-check explanation, provider/runtime failures, OpenCode runtime - evidence, and approval publication failures. Publication failures are automation evidence, not - source-backed repository findings, and they must be summarized as OpenCode runtime evidence. -- Keep provider failure, external failed-check classification, and Strix evidence lookup diagnostics - in the central workflow. Strix evidence lookup failures must mention missing Actions read access - when that is the actual GitHub API scope problem. +- Inspect up to 20 open, non-draft PRs targeting `develop` by default. - Skip PRs with unresolved review threads. +- Request one CodeRabbit review per head SHA when a PR has zero unresolved threads but is not approved. - Check only GitHub-required checks before merge actions. +- Retry transient GitHub CLI/API read failures and skip only the affected PR when review-thread + state remains unavailable after retries, while keeping command stdout separate from retry + diagnostics so parsed JSON, counts, and booleans stay clean. - Update approved PRs that are behind `develop` and wait for fresh checks. - Merge only PRs that are approved, thread-clean, conflict-free, and passing required checks. - Fall back to GitHub auto-merge only when a direct normal merge does not complete. @@ -39,19 +25,12 @@ repo-local copies of `opencode-review.yml`, `pr-review-merge-scheduler.yml`, or - It does not resolve review threads. - It does not use admin merge or ruleset bypass. - It does not weaken required checks, branch protection, or repository rulesets. -- It does not require BandScope to carry repo-local OpenCode or scheduler workflow/helper copies. -- It does not move central token permissions into this repository. ## Security Notes -- Attack surface: organization required workflows with write access to PR comments, PR branch updates, and normal merges. +- Attack surface: scheduled GitHub Actions automation with write access to PR comments, PR branch updates, and normal merges. - Trust boundary touched: GitHub repository governance, PR review state, status checks, and CodeRabbit review requests. - Realistic threats: spammed review comments, merging a PR with unresolved conversations, merging without required checks, or hiding conflicts behind automation. -- Mitigations: central required workflow source pinning, idempotent per-head review comment marker, - explicit unresolved-thread check, retry-bounded GitHub API reads, required-check verification - through GitHub, conflict skip, guarded merge with `--match-head-commit`, and no admin bypass path. +- Mitigations: idempotent per-head review comment marker, explicit unresolved-thread check, retry-bounded GitHub API reads, required-check verification through GitHub, conflict skip, normal merge only, and no admin bypass path. - Remaining risk: CodeRabbit and GitHub check state can be delayed or stale; the scheduler therefore only advances eligible PRs and leaves code-fix work to agents or maintainers. -- Test points: organization ruleset inheritance, current-head OpenCode approval, unresolved review - thread count, required-check rollup, approved behind PR, approved conflict-free PR, approved dirty PR, - external failed-check classification, provider/runtime failure summary, and Strix evidence lookup - scope diagnostics. +- Test points: `workflow_dispatch` dry run on a limited `max_prs`, transient GitHub API failure with stderr output, PR with unresolved thread, PR needing review, approved behind PR, approved conflict-free PR, and approved dirty PR. diff --git a/opencode.jsonc b/opencode.jsonc index 888aa237..a5ab3396 100644 --- a/opencode.jsonc +++ b/opencode.jsonc @@ -70,24 +70,6 @@ "context": 128000, "output": 4096 } - }, - "openai/o3": { - "name": "OpenAI o3", - "tool_call": true, - "reasoning": true, - "limit": { - "context": 200000, - "output": 100000 - } - }, - "openai/o4-mini": { - "name": "OpenAI o4-mini", - "tool_call": true, - "reasoning": true, - "limit": { - "context": 200000, - "output": 100000 - } } } } diff --git a/package-lock.json b/package-lock.json index 8a479475..4bff7d1e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,7 @@ ], "devDependencies": { "@eslint/js": "^10.0.1", - "eslint-plugin-jsdoc": "^63.0.7", + "eslint-plugin-jsdoc": "^63.0.5", "react": "^19.2.4", "react-dom": "^19.2.7" }, @@ -2755,9 +2755,9 @@ } }, "node_modules/eslint-plugin-jsdoc": { - "version": "63.0.7", - "resolved": "https://registry.npmjs.org/eslint-plugin-jsdoc/-/eslint-plugin-jsdoc-63.0.7.tgz", - "integrity": "sha512-pxrqGO733F7xmVYB5vQOiciiT9uddxqehawnbPjZmW2YaJR6fT5cP3UQd2BNoE85ATspCMtNL8w/a5WDGX3Qwg==", + "version": "63.0.5", + "resolved": "https://registry.npmjs.org/eslint-plugin-jsdoc/-/eslint-plugin-jsdoc-63.0.5.tgz", + "integrity": "sha512-AzI9bgKhV9li049/mIblX0c41DeWMMfH9qNsRasc+fAxwURRKChIp03Pk57M7UTf+Y6hifTJ89kQyCOoOLtEDw==", "dev": true, "license": "BSD-3-Clause", "dependencies": { diff --git a/package.json b/package.json index 974eadab..84946d84 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ }, "devDependencies": { "@eslint/js": "^10.0.1", - "eslint-plugin-jsdoc": "^63.0.7", + "eslint-plugin-jsdoc": "^63.0.5", "react": "^19.2.4", "react-dom": "^19.2.7" } diff --git a/packages/shared-types/src/index.ts b/packages/shared-types/src/index.ts index ff04618b..400f4d39 100644 --- a/packages/shared-types/src/index.ts +++ b/packages/shared-types/src/index.ts @@ -1807,7 +1807,7 @@ function validateSongRehearsalPack( if (value.song === undefined) return invalidField(`${path}.song`); const songError = validateRehearsalSong(value.song, options); if (songError) return songError; - } else { + } else if (value.packState === "failed") { const extraKey = unexpectedKey(value, ["id", "packState", "engineState", "sourceLabel", "error"], path); if (extraKey) return extraKey; if (value.error === undefined) return invalidField(`${path}.error`); diff --git a/packages/shared-types/test/index.test.ts b/packages/shared-types/test/index.test.ts index bd2bc6fa..b74189f6 100644 --- a/packages/shared-types/test/index.test.ts +++ b/packages/shared-types/test/index.test.ts @@ -221,7 +221,6 @@ describe("shared type helpers", () => { progressPercent: 0, cacheStatus: "disabled" }); - expect(parseAnalysisJobStatus(queuedStatus)).toEqual(queuedStatus); expect(isAnalysisJobStatus({ jobId: "job-1", state: "running", @@ -343,14 +342,6 @@ describe("shared type helpers", () => { updatedAt: "2026-03-12T00:00:00.000Z", error: { code: "not_found", message: "Missing", extraField: true } })).toBe(false); - expect(() => parseAnalysisJobStatus({ - jobId: "job-1", - state: "running", - requestedAt: "2026-03-12T00:00:00.000Z", - updatedAt: "2026-03-12T00:00:00.000Z", - cacheStatus: "warm" - })).toThrow("cacheStatus"); - expect(() => parseAnalysisJobStatus({ jobId: 7 })).toThrow("jobId"); }); it("validates local audio sources and bootstrap requests", () => { @@ -587,9 +578,7 @@ describe("shared type helpers", () => { { message: "sections[0].roleBuckets[0].id", payload: { ...artifact, sections: [{ ...artifact.sections[0], roleBuckets: [{ ...artifact.sections[0]!.roleBuckets[0], id: 3 }] }] } }, { message: "sections[0].roleBuckets[0].name", payload: { ...artifact, sections: [{ ...artifact.sections[0], roleBuckets: [{ ...artifact.sections[0]!.roleBuckets[0], name: 3 }] }] } }, { message: "sections[0].roleBuckets[0].roleType", payload: { ...artifact, sections: [{ ...artifact.sections[0], roleBuckets: [{ ...artifact.sections[0]!.roleBuckets[0], roleType: "drums" }] }] } }, - { message: "sections[0].roleBuckets[0].extraField", payload: { ...artifact, sections: [{ ...artifact.sections[0], roleBuckets: [{ ...artifact.sections[0]!.roleBuckets[0], extraField: true }] }] } }, { message: "sections[0].roleBuckets[0].rehearsalPriority", payload: { ...artifact, sections: [{ ...artifact.sections[0], roleBuckets: [{ ...artifact.sections[0]!.roleBuckets[0], rehearsalPriority: "urgent" }] }] } }, - { message: "sections[0].extraField", payload: { ...artifact, sections: [{ ...artifact.sections[0], extraField: true }] } }, { message: "sourceAssets", payload: { ...artifact, sourceAssets: "not-an-array" } }, { message: "sourceAssets[0]", payload: { ...artifact, sourceAssets: [null] } }, { message: "sourceAssets[0].referenceKind", payload: { ...artifact, sourceAssets: [{ ...artifact.sourceAssets[0], referenceKind: "stem" }] } }, @@ -1116,58 +1105,6 @@ describe("shared type helpers", () => { song.sections[0]!.roles[0]!.transpositionPlan = 2 as never; }) }, - { - message: "sections[0].roles[0].transcription", - payload: createInvalidSong((song) => { - song.sections[0]!.roles[0]!.transcription = "not-an-array" as never; - }) - }, - { - message: "sections[0].roles[0].transcription[0]", - payload: createInvalidSong((song) => { - song.sections[0]!.roles[0]!.transcription = [null as never]; - }) - }, - { - message: "sections[0].roles[0].transcription[0].extraField", - payload: createInvalidSong((song) => { - song.sections[0]!.roles[0]!.transcription = [ - { pitch: "E2", onset: 0, offset: 1, velocity: 0.7, extraField: true } as never - ]; - }) - }, - { - message: "sections[0].roles[0].transcription[0].pitch", - payload: createInvalidSong((song) => { - song.sections[0]!.roles[0]!.transcription = [ - { pitch: 42, onset: 0, offset: 1, velocity: 0.7 } as never - ]; - }) - }, - { - message: "sections[0].roles[0].transcription[0].onset", - payload: createInvalidSong((song) => { - song.sections[0]!.roles[0]!.transcription = [ - { pitch: "E2", onset: "0", offset: 1, velocity: 0.7 } as never - ]; - }) - }, - { - message: "sections[0].roles[0].transcription[0].offset", - payload: createInvalidSong((song) => { - song.sections[0]!.roles[0]!.transcription = [ - { pitch: "E2", onset: 0, offset: "1", velocity: 0.7 } as never - ]; - }) - }, - { - message: "sections[0].roles[0].transcription[0].velocity", - payload: createInvalidSong((song) => { - song.sections[0]!.roles[0]!.transcription = [ - { pitch: "E2", onset: 0, offset: 1, velocity: "loud" } as never - ]; - }) - }, { message: "sections[0].roles[2].manualOverrides[0]", payload: createInvalidSong((song) => { @@ -1276,162 +1213,24 @@ describe("shared type helpers", () => { song.collaboration!.syncMode = "shared_drive" as never; }) }, - { - message: "collaboration", - payload: createInvalidSong((song) => { - song.collaboration = null as never; - }) - }, - { - message: "collaboration.extraField", - payload: createInvalidSong((song) => { - (song.collaboration as unknown as Record).extraField = true; - }) - }, { message: "collaboration.syncNote", payload: createInvalidSong((song) => { song.collaboration!.syncNote = 2 as never; }) }, - { - message: "collaboration.assignments", - payload: createInvalidSong((song) => { - song.collaboration!.assignments = "not-an-array" as never; - }) - }, - { - message: "collaboration.assignments[0]", - payload: createInvalidSong((song) => { - song.collaboration!.assignments = [null as never]; - }) - }, - { - message: "collaboration.assignments[0].extraField", - payload: createInvalidSong((song) => { - (song.collaboration!.assignments[0] as unknown as Record).extraField = true; - }) - }, - { - message: "collaboration.assignments[0].id", - payload: createInvalidSong((song) => { - song.collaboration!.assignments[0]!.id = 2 as never; - }) - }, { message: "collaboration.assignments[0].assignee", payload: createInvalidSong((song) => { song.collaboration!.assignments[0]!.assignee = 2 as never; }) }, - { - message: "collaboration.assignments[0].summary", - payload: createInvalidSong((song) => { - song.collaboration!.assignments[0]!.summary = 2 as never; - }) - }, - { - message: "collaboration.assignments[0].sectionId", - payload: createInvalidSong((song) => { - song.collaboration!.assignments[0]!.sectionId = 2 as never; - }) - }, - { - message: "collaboration.assignments[0].roleId", - payload: createInvalidSong((song) => { - song.collaboration!.assignments[0]!.roleId = 2 as never; - }) - }, - { - message: "collaboration.comments", - payload: createInvalidSong((song) => { - song.collaboration!.comments = "not-an-array" as never; - }) - }, - { - message: "collaboration.comments[0]", - payload: createInvalidSong((song) => { - song.collaboration!.comments = [null as never]; - }) - }, - { - message: "collaboration.comments[0].extraField", - payload: createInvalidSong((song) => { - (song.collaboration!.comments[0] as unknown as Record).extraField = true; - }) - }, - { - message: "collaboration.comments[0].id", - payload: createInvalidSong((song) => { - song.collaboration!.comments[0]!.id = 2 as never; - }) - }, - { - message: "collaboration.comments[0].author", - payload: createInvalidSong((song) => { - song.collaboration!.comments[0]!.author = 2 as never; - }) - }, - { - message: "collaboration.comments[0].body", - payload: createInvalidSong((song) => { - song.collaboration!.comments[0]!.body = 2 as never; - }) - }, - { - message: "collaboration.comments[0].sectionId", - payload: createInvalidSong((song) => { - song.collaboration!.comments[0]!.sectionId = 2 as never; - }) - }, - { - message: "collaboration.comments[0].roleId", - payload: createInvalidSong((song) => { - song.collaboration!.comments[0]!.roleId = 2 as never; - }) - }, { message: "collaboration.comments[0].status", payload: createInvalidSong((song) => { song.collaboration!.comments[0]!.status = "pending" as never; }) }, - { - message: "collaboration.approvals", - payload: createInvalidSong((song) => { - song.collaboration!.approvals = "not-an-array" as never; - }) - }, - { - message: "collaboration.approvals[0]", - payload: createInvalidSong((song) => { - song.collaboration!.approvals = [null as never]; - }) - }, - { - message: "collaboration.approvals[0].extraField", - payload: createInvalidSong((song) => { - (song.collaboration!.approvals[0] as unknown as Record).extraField = true; - }) - }, - { - message: "collaboration.approvals[0].id", - payload: createInvalidSong((song) => { - song.collaboration!.approvals[0]!.id = 2 as never; - }) - }, - { - message: "collaboration.approvals[0].scope", - payload: createInvalidSong((song) => { - song.collaboration!.approvals[0]!.scope = 2 as never; - }) - }, - { - message: "collaboration.approvals[0].owner", - payload: createInvalidSong((song) => { - song.collaboration!.approvals[0]!.owner = 2 as never; - }) - }, { message: "collaboration.approvals[0].status", payload: createInvalidSong((song) => { @@ -1443,14 +1242,6 @@ describe("shared type helpers", () => { for (const testCase of cases) { expect(() => parseRehearsalSong(testCase.payload)).toThrow(testCase.message); } - - const songWithTranscription = createDemoRehearsalSong(); - songWithTranscription.sections[0]!.roles[0]!.transcription = [ - { pitch: "E2", onset: 0, offset: 1, velocity: 0.7 } - ]; - expect(parseRehearsalSong(songWithTranscription).sections[0]?.roles[0]?.transcription).toEqual([ - { pitch: "E2", onset: 0, offset: 1, velocity: 0.7 } - ]); }); it("validates SongRehearsalPack and RehearsalWorkspace", () => { @@ -1468,39 +1259,10 @@ describe("shared type helpers", () => { workspaceVersion: 1, songs: [validPack] }; - const queuedPack: SongRehearsalPack = { - id: "pack-queued", - packState: "queued", - engineState: "queued", - sourceLabel: "Queued Song" - }; - const analyzingPack: SongRehearsalPack = { - id: "pack-analyzing", - packState: "analyzing", - engineState: "running", - sourceLabel: "Analyzing Song" - }; - const failedPack: SongRehearsalPack = { - id: "pack-failed", - packState: "failed", - engineState: "failed", - sourceLabel: "Failed Song", - error: { code: "engine_unavailable", message: "Engine unavailable" } - }; expect(parseSongRehearsalPack(validPack)).toEqual(validPack); - expect(parseSongRehearsalPack(queuedPack)).toEqual(queuedPack); - expect(parseSongRehearsalPack(analyzingPack)).toEqual(analyzingPack); - expect(parseSongRehearsalPack(failedPack)).toEqual(failedPack); expect(isRehearsalWorkspace(validWorkspace)).toBe(true); expect(parseRehearsalWorkspace(validWorkspace)).toEqual(validWorkspace); - expect(parseRehearsalWorkspace({ - ...validWorkspace, - songs: [queuedPack, failedPack] - })).toEqual({ - ...validWorkspace, - songs: [queuedPack, failedPack] - }); const legacyNestedSong = createDemoRehearsalSong() as unknown as { sections: Array>; @@ -1523,23 +1285,6 @@ describe("shared type helpers", () => { // Invalid packs expect(() => parseSongRehearsalPack({ ...validPack, packState: "invalid" })).toThrow("packState"); expect(() => parseSongRehearsalPack({ ...validPack, extraField: true })).toThrow("extraField"); - expect(() => parseSongRehearsalPack({ - id: "pack-ready-missing-song", - packState: "ready", - sourceLabel: "Ready Song" - })).toThrow("song"); - expect(() => parseSongRehearsalPack({ ...queuedPack, extraField: true })).toThrow("extraField"); - expect(() => parseSongRehearsalPack({ - id: "pack-queued-missing-engine", - packState: "queued", - sourceLabel: "Queued Song" - })).toThrow("engineState"); - expect(() => parseSongRehearsalPack({ ...failedPack, extraField: true })).toThrow("extraField"); - expect(() => parseSongRehearsalPack({ - id: "pack-failed-missing-error", - packState: "failed", - sourceLabel: "Failed Song" - })).toThrow("error"); // Invalid workspaces expect(isRehearsalWorkspace({ ...validWorkspace, songs: [{...validPack, packState: "bad"}] })).toBe(false); diff --git a/scripts/checks/verify_supply_chain.py b/scripts/checks/verify_supply_chain.py index 19a6b33a..ca4864c6 100644 --- a/scripts/checks/verify_supply_chain.py +++ b/scripts/checks/verify_supply_chain.py @@ -1216,30 +1216,23 @@ def is_blocking_required_step(block_lines: list[str], block_indent: int) -> bool return [] -def _verify_ci_coverage(missing: list[str]) -> None: +def verify_workflow_coverage() -> list[str]: + """Return workflow trigger and artifact coverage violations.""" + missing: list[str] = [] ci = read_workflow(Path(".github/workflows/ci.yml"), "ci", missing) for token in ["develop", "main", "pull_request", "push", "ci / build-and-test"]: if ci and token not in ci: missing.append(f"ci workflow missing token: {token}") - - -def _verify_sbom_coverage(missing: list[str]) -> None: sbom = read_workflow(Path(".github/workflows/sbom.yml"), "sbom", missing) for token in ["develop", "main", "pull_request", "release:", "tags:"]: if sbom and token not in sbom: missing.append(f"sbom workflow missing trigger token: {token}") - - -def _verify_dependency_review_coverage(missing: list[str]) -> None: review = read_workflow( Path(".github/workflows/dependency-review.yml"), "dependency review", missing ) for token in ["develop", "main", "pull_request"]: if review and token not in review: missing.append(f"dependency review workflow missing trigger token: {token}") - - -def _verify_security_audit_coverage(missing: list[str]) -> None: audit = read_workflow( Path(".github/workflows/security-audit.yml"), "security audit", missing ) @@ -1266,16 +1259,10 @@ def _verify_security_audit_coverage(missing: list[str]) -> None: missing.append( f"security audit workflow missing vulnerability audit token: {token}" ) - - -def _verify_codeql_coverage(missing: list[str]) -> None: codeql = read_workflow(Path(".github/workflows/codeql.yml"), "codeql", missing) for token in ["develop", "main", "pull_request", "push", "codeql"]: if codeql and token not in codeql: missing.append(f"codeql workflow missing token: {token}") - - -def _verify_release_coverage(missing: list[str]) -> None: release = read_workflow(Path(".github/workflows/release.yml"), "release", missing) for token in [ "develop", @@ -1287,18 +1274,12 @@ def _verify_release_coverage(missing: list[str]) -> None: ]: if release and token not in release: missing.append(f"release workflow missing token: {token}") - - -def _verify_secret_scan_coverage(missing: list[str]) -> None: secret_scan = read_workflow( Path(".github/workflows/secret-scan-gate.yml"), "secret scan", missing ) for token in ["develop", "main", "pull_request", "push", "secret-scan-gate"]: if secret_scan and token not in secret_scan: missing.append(f"secret scan workflow missing token: {token}") - - -def _verify_build_coverage(missing: list[str]) -> None: build = read_workflow( Path(".github/workflows/build-baseline.yml"), "build baseline", missing ) @@ -1336,9 +1317,14 @@ def _verify_build_coverage(missing: list[str]) -> None: missing.append( "build workflow should not rely on macos-latest for architecture coverage" ) - - -def _verify_scorecard_coverage(missing: list[str], workflow_paths: list[Path]) -> None: + workflow_paths = sorted(Path(".github/workflows").glob("*.yml")) + sorted( + Path(".github/workflows").glob("*.yaml") + ) + for workflow_path in workflow_paths: + workflow_content = workflow_path.read_text(encoding="utf-8") + missing.extend( + release_artifact_download_decompression_violations(workflow_content) + ) scorecard = read_workflow( Path(".github/workflows/ossf-scorecard.yml"), "ossf scorecard", missing ) @@ -1381,31 +1367,6 @@ def _verify_scorecard_coverage(missing: list[str], workflow_paths: list[Path]) - workflow_content, workflow_path ) ) - - -def verify_workflow_coverage() -> list[str]: - """Return workflow trigger and artifact coverage violations.""" - missing: list[str] = [] - _verify_ci_coverage(missing) - _verify_sbom_coverage(missing) - _verify_dependency_review_coverage(missing) - _verify_security_audit_coverage(missing) - _verify_codeql_coverage(missing) - _verify_release_coverage(missing) - _verify_secret_scan_coverage(missing) - _verify_build_coverage(missing) - - workflow_paths = sorted(Path(".github/workflows").glob("*.yml")) + sorted( - Path(".github/workflows").glob("*.yaml") - ) - for workflow_path in workflow_paths: - workflow_content = workflow_path.read_text(encoding="utf-8") - missing.extend( - release_artifact_download_decompression_violations(workflow_content) - ) - - _verify_scorecard_coverage(missing, workflow_paths) - return missing diff --git a/scripts/ci/classify_failed_check_evidence.py b/scripts/ci/classify_failed_check_evidence.py new file mode 100644 index 00000000..1ecf342a --- /dev/null +++ b/scripts/ci/classify_failed_check_evidence.py @@ -0,0 +1,311 @@ +#!/usr/bin/env python3 +"""Classify failed-check evidence before OpenCode changes PR review state.""" + +from __future__ import annotations + +import json +import re +import sys +from pathlib import Path +from typing import Any + + +FAILED_CHECK_HEADING = re.compile(r"^## Failed check:\s*(.+)$", re.MULTILINE) +UPLOAD_ARTIFACT_STEP = re.compile( + r"^- step \d+:\s+Upload .+ artifact \(failure\)$", + re.IGNORECASE | re.MULTILINE, +) +BUILD_NATIVE_SHELL_STEP = re.compile( + r"^- step \d+:\s+Build native shell \(failure\)$", + re.IGNORECASE | re.MULTILINE, +) +SETUP_UV_STEP = re.compile( + r"^- step \d+:\s+Run astral-sh/setup-uv@.+ \(failure\)$", + re.IGNORECASE | re.MULTILINE, +) +ARTIFACT_UPLOAD_INFRA_PATTERNS = ( + ( + "artifact upload finalize request reset", + re.compile( + r"Failed to FinalizeArtifact:\s+Unable to make request:\s+ECONNRESET", + re.IGNORECASE, + ), + ), + ( + "artifact service request reset", + re.compile(r"Unable to make request:\s+ECONNRESET", re.IGNORECASE), + ), +) +ARTIFACT_UPLOAD_CONFIRMATION_PATTERNS = ( + re.compile(r"actions/upload-artifact@", re.IGNORECASE), + re.compile(r"Finished uploading artifact content", re.IGNORECASE), + re.compile(r"Finalizing artifact upload", re.IGNORECASE), +) +TAURI_BINARY_RELEASE_DOWNLOAD_PATTERNS = ( + re.compile( + r"Downloading https://github\.com/tauri-apps/binary-releases/", + re.IGNORECASE, + ), +) +TAURI_BUNDLE_INFRA_PATTERNS = ( + ( + "tauri binary release download server error", + re.compile( + r"failed to bundle project `http status:\s*50[0-9]`", + re.IGNORECASE, + ), + ), +) +SETUP_UV_MANIFEST_FETCH_PATTERNS = ( + re.compile( + r"Fetching manifest data from " + r"https://raw\.githubusercontent\.com/astral-sh/versions/", + re.IGNORECASE, + ), +) +SETUP_UV_INFRA_PATTERNS = ( + ( + "setup-uv manifest fetch failed", + re.compile(r"##\[error\]fetch failed", re.IGNORECASE), + ), +) +BUILD_OR_PACKAGE_SUCCESS_PATTERNS = ( + re.compile(r"Finished `release` profile", re.IGNORECASE), + re.compile(r"Built application at:", re.IGNORECASE), + re.compile(r"Packaged .+ to artifacts/", re.IGNORECASE), +) + + +def unknown(reason: str, *, signals: list[str] | None = None) -> dict[str, Any]: + """Return the default actionable-or-unknown classification.""" + return { + "classification": "actionable_or_unknown", + "reason": reason, + "signals": signals or [], + } + + +def external(reason: str, *, signals: list[str]) -> dict[str, Any]: + """Return a classification for failures outside repository source control.""" + return { + "classification": "external_infrastructure", + "reason": reason, + "signals": signals, + } + + +def matching_evidence_lines( + evidence_text: str, patterns: tuple[re.Pattern[str], ...] +) -> list[str]: + """Return concrete evidence lines matched by the given patterns.""" + matches: list[str] = [] + for pattern in patterns: + for line in evidence_text.splitlines(): + if pattern.search(line): + matches.append(line.strip()) + break + return matches + + +def matching_labeled_evidence_lines( + evidence_text: str, patterns: tuple[tuple[str, re.Pattern[str]], ...] +) -> list[str]: + """Return labeled concrete evidence lines matched by the given patterns.""" + matches: list[str] = [] + matched_lines: set[str] = set() + for label, pattern in patterns: + for line in evidence_text.splitlines(): + if pattern.search(line): + matched_line = line.strip() + if matched_line not in matched_lines: + matches.append(f"{label}: {matched_line}") + matched_lines.add(matched_line) + break + return matches + + +def classify_failed_check_evidence(evidence_text: str) -> dict[str, Any]: + """Classify whether failed check evidence is safe to withhold as non-source.""" + failed_checks = FAILED_CHECK_HEADING.findall(evidence_text) + if not failed_checks: + return unknown("no failed check headings were present") + if len(failed_checks) != 1: + return unknown( + "multiple failed checks require per-check source diagnosis", + signals=failed_checks, + ) + + failed_check = failed_checks[0].strip() + upload_step_match = UPLOAD_ARTIFACT_STEP.search(evidence_text) + build_success_signals = matching_evidence_lines( + evidence_text, + BUILD_OR_PACKAGE_SUCCESS_PATTERNS, + ) + if upload_step_match is not None: + matched_infra_signals = matching_labeled_evidence_lines( + evidence_text, + ARTIFACT_UPLOAD_INFRA_PATTERNS, + ) + if not matched_infra_signals: + return unknown( + "no known external artifact upload infrastructure signal was present", + signals=[failed_check, upload_step_match.group(0)], + ) + + if not any( + pattern.search(evidence_text) + for pattern in ARTIFACT_UPLOAD_CONFIRMATION_PATTERNS + ): + return unknown( + "artifact upload context was missing from the failed-check evidence", + signals=[ + failed_check, + upload_step_match.group(0), + *matched_infra_signals, + ], + ) + + if not build_success_signals: + return unknown( + "build or package success was not visible before artifact upload failed", + signals=[ + failed_check, + upload_step_match.group(0), + *matched_infra_signals, + ], + ) + + return external( + ( + "the only failed check is a GitHub artifact upload " + "finalization/network failure after build/package output was " + "produced; rerun the failed workflow job instead of requesting " + "source changes" + ), + signals=[ + failed_check, + upload_step_match.group(0), + *matched_infra_signals, + *build_success_signals, + ], + ) + + setup_uv_step_match = SETUP_UV_STEP.search(evidence_text) + if setup_uv_step_match is not None: + matched_infra_signals = matching_labeled_evidence_lines( + evidence_text, + SETUP_UV_INFRA_PATTERNS, + ) + if not matched_infra_signals: + return unknown( + "no known external setup-uv infrastructure signal was present", + signals=[failed_check, setup_uv_step_match.group(0)], + ) + + setup_uv_fetch_signals = matching_evidence_lines( + evidence_text, + SETUP_UV_MANIFEST_FETCH_PATTERNS, + ) + if not setup_uv_fetch_signals: + return unknown( + "setup-uv manifest fetch context was missing from the evidence", + signals=[ + failed_check, + setup_uv_step_match.group(0), + *matched_infra_signals, + ], + ) + + return external( + ( + "the only failed check is a setup-uv manifest fetch failure " + "before repository build steps ran; rerun the failed workflow " + "job instead of requesting source changes" + ), + signals=[ + failed_check, + setup_uv_step_match.group(0), + *matched_infra_signals, + *setup_uv_fetch_signals, + ], + ) + + native_shell_step_match = BUILD_NATIVE_SHELL_STEP.search(evidence_text) + if native_shell_step_match is None: + return unknown( + "no known external failed job step pattern was present", + signals=[failed_check], + ) + + matched_infra_signals = matching_labeled_evidence_lines( + evidence_text, + TAURI_BUNDLE_INFRA_PATTERNS, + ) + if not matched_infra_signals: + return unknown( + "no known external native-shell infrastructure signal was present", + signals=[failed_check, native_shell_step_match.group(0)], + ) + + tauri_download_signals = matching_evidence_lines( + evidence_text, + TAURI_BINARY_RELEASE_DOWNLOAD_PATTERNS, + ) + if not tauri_download_signals: + return unknown( + "Tauri binary release download context was missing from the evidence", + signals=[ + failed_check, + native_shell_step_match.group(0), + *matched_infra_signals, + ], + ) + + if not build_success_signals: + return unknown( + "build success was not visible before native-shell bundling failed", + signals=[ + failed_check, + native_shell_step_match.group(0), + *matched_infra_signals, + *tauri_download_signals, + ], + ) + + return external( + ( + "the only failed check is a Tauri binary release download server " + "error after the native app binary was built; rerun the failed " + "workflow job instead of requesting source changes" + ), + signals=[ + failed_check, + native_shell_step_match.group(0), + *matched_infra_signals, + *tauri_download_signals, + *build_success_signals, + ], + ) + + +def main(argv: list[str]) -> int: + """Classify a failed-check evidence file and print JSON.""" + if len(argv) != 2: + print( + "usage: classify_failed_check_evidence.py ", file=sys.stderr + ) + return 64 + + evidence_file = Path(argv[1]) + try: + evidence_text = evidence_file.read_text(encoding="utf-8") + except OSError as exc: + print(f"cannot read failed-check evidence file: {exc}", file=sys.stderr) + return 65 + + print(json.dumps(classify_failed_check_evidence(evidence_text), ensure_ascii=True)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv)) diff --git a/scripts/ci/collect_failed_check_evidence.sh b/scripts/ci/collect_failed_check_evidence.sh new file mode 100755 index 00000000..b7d1023c --- /dev/null +++ b/scripts/ci/collect_failed_check_evidence.sh @@ -0,0 +1,425 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [ "$#" -ne 1 ]; then + echo "usage: $0 " >&2 + exit 2 +fi + +: "${GH_REPOSITORY:?GH_REPOSITORY is required}" +: "${PR_NUMBER:?PR_NUMBER is required}" +: "${HEAD_SHA:?HEAD_SHA is required}" + +OUTPUT_FILE="$1" +FAILED_CHECK_LOG_LINES="${FAILED_CHECK_LOG_LINES:-180}" + +strip_ansi() { + perl -pe 's/\x1b\[[0-9;?]*[A-Za-z]//g' +} + +emit_bounded_file() { + local file_path="$1" + local max_lines="$2" + local total_lines + local head_lines + local tail_lines + + total_lines="$(wc -l <"$file_path" | tr -d '[:space:]')" + if [ -z "$total_lines" ] || [ "$total_lines" -le "$max_lines" ]; then + sed -n "1,${max_lines}p" "$file_path" + return 0 + fi + + head_lines=$((max_lines / 2)) + tail_lines=$((max_lines - head_lines)) + sed -n "1,${head_lines}p" "$file_path" + printf '\n... truncated %s middle log lines ...\n\n' "$((total_lines - max_lines))" + tail -n "$tail_lines" "$file_path" +} + +emit_failure_signal_summary() { + local log_file="$1" + local summary_tmp + + summary_tmp="$(mktemp)" + tmp_files+=("$summary_tmp") + + awk ' + /FAIL:/ || + /::error::/ || + /##\[error\]/ || + /Process completed with exit code/ || + /LLM CONNECTION FAILED/ || + /RateLimitError/ || + /Too many requests/ || + /budget limit/ || + /Configured model and fallback models were unavailable/ || + /provider infrastructure/ || + /[Ff]atal/ || + /[Dd]enied/ || + /[Tt]imeout/ || + /[Ww]arn/ { + if (!seen[$0]++) { + print + } + } + ' "$log_file" >"$summary_tmp" + + if [ ! -s "$summary_tmp" ]; then + return 1 + fi + + printf '### Failed log signal summary\n\n' + printf '```text\n' + emit_bounded_file "$summary_tmp" 120 + printf '\n```\n\n' +} + +emit_strix_vulnerability_evidence() { + local log_file="$1" + local summary_tmp + local ranges_tmp + local merged_ranges_tmp + local report_index=0 + local start_line + local end_line + + summary_tmp="$(mktemp)" + ranges_tmp="$(mktemp)" + merged_ranges_tmp="$(mktemp)" + tmp_files+=("$summary_tmp" "$ranges_tmp" "$merged_ranges_tmp") + + awk ' + /Strix run failed for model/ || + /Primary model unavailable; retrying with fallback/ || + /Strix fallback model/ || + /LLM CONNECTION FAILED/ || + /RateLimitError/ || + /Too many requests/ || + /budget limit/ || + /Configured model and fallback models were unavailable/ || + /Below-threshold findings detected/ || + /Unable to map Strix findings/ || + /Model [[:alnum:]_.\/-]+/ || + /Vulnerabilities[[:space:]]+[0-9]/ || + /Vulnerabilities[[:space:]]+.*Total/ || + /(CRITICAL|HIGH|MEDIUM|LOW):[[:space:]]+[0-9]/ { + if (!seen[$0]++) { + print + } + } + ' "$log_file" >"$summary_tmp" + + awk ' + /Vulnerability Report/ { + start = NR - 12 + if (start < 1) { + start = 1 + } + end = NR + 190 + print start, end + } + ' "$log_file" >"$ranges_tmp" + + if [ ! -s "$summary_tmp" ] && [ ! -s "$ranges_tmp" ]; then + return 1 + fi + + printf '### Strix model attempt and finding summary\n\n' + if [ -s "$summary_tmp" ]; then + printf '```text\n' + emit_bounded_file "$summary_tmp" 180 + printf '\n```\n\n' + else + printf 'No model summary lines were detected in the failed Strix log.\n\n' + fi + + if [ ! -s "$ranges_tmp" ]; then + printf 'No Strix vulnerability report windows were detected in the failed log.\n\n' + return 0 + fi + + awk ' + NR == 1 { + start = $1 + end = $2 + next + } + $1 <= end + 5 { + if ($2 > end) { + end = $2 + } + next + } + { + print start, end + start = $1 + end = $2 + } + END { + if (start != "") { + print start, end + } + } + ' "$ranges_tmp" >"$merged_ranges_tmp" + + while read -r start_line end_line; do + report_index=$((report_index + 1)) + printf '### Strix vulnerability report window %s (log lines %s-%s)\n\n' "$report_index" "$start_line" "$end_line" + printf '```text\n' + sed -n "${start_line},${end_line}p" "$log_file" + printf '\n```\n\n' + done <"$merged_ranges_tmp" +} + +owner="${GH_REPOSITORY%%/*}" +repo="${GH_REPOSITORY#*/}" +failed_contexts="$(mktemp)" +workflow_run_contexts="$(mktemp)" +tmp_files=("$failed_contexts" "$workflow_run_contexts") +cleanup() { + rm -f "${tmp_files[@]}" +} +trap cleanup EXIT + +gh api graphql \ + -f owner="$owner" \ + -f name="$repo" \ + -F number="$PR_NUMBER" \ + -f query=' + query($owner:String!,$name:String!,$number:Int!) { + repository(owner:$owner,name:$name) { + pullRequest(number:$number) { + potentialMergeCommit { + oid + } + statusCheckRollup { + contexts(first: 100) { + nodes { + __typename + ... on CheckRun { + databaseId + name + status + conclusion + detailsUrl + checkSuite { + commit { + oid + } + workflowRun { + databaseId + workflow { + name + } + } + } + } + ... on StatusContext { + context + state + targetUrl + } + } + } + } + } + } + } + ' | + jq -r --arg head_sha "$HEAD_SHA" ' + (.data.repository.pullRequest.potentialMergeCommit.oid // "") as $merge_sha + | (.data.repository.pullRequest.statusCheckRollup.contexts.nodes // []) + | map( + if .__typename == "CheckRun" then + select((.checkSuite.commit.oid // "") as $check_sha | $check_sha == $head_sha or ($merge_sha != "" and $check_sha == $merge_sha)) + | select((.status // "") == "COMPLETED") + | select((.conclusion // "" | ascii_upcase) as $c | ["FAILURE","TIMED_OUT","ACTION_REQUIRED","CANCELLED","STARTUP_FAILURE"] | index($c)) + | [ + "check_run", + (((.checkSuite.workflowRun.workflow.name // "") + "/" + (.name // "check")) | gsub("^/"; "")), + (.conclusion // "unknown"), + (.detailsUrl // ""), + ((.checkSuite.workflowRun.databaseId // "") | tostring), + ((.databaseId // "") | tostring) + ] + elif .__typename == "StatusContext" then + select((.state // "" | ascii_upcase) as $s | ["FAILURE","ERROR"] | index($s)) + | [ + "status_context", + (.context // "status"), + (.state // "unknown"), + (.targetUrl // ""), + "", + "" + ] + else + empty + end + ) + | .[] + | @tsv + ' >"$failed_contexts" + + gh run list \ + --repo "$GH_REPOSITORY" \ + --commit "$HEAD_SHA" \ + --limit 100 \ + --json databaseId,workflowName,status,conclusion,url,event,headSha | + jq -r --arg head_sha "$HEAD_SHA" ' + .[] + | select((.event // "") == "pull_request_target" or (.event // "") == "workflow_dispatch") + | select((.headSha // "") == $head_sha) + | select((.workflowName // "") == "Strix Security Scan" or (.workflowName // "") == "Strix") + | select((.status // "") == "completed") + | select((.conclusion // "" | ascii_downcase) as $c | ["failure","timed_out","action_required","cancelled","startup_failure"] | index($c)) + | [ + "workflow_run", + (if (.workflowName // "") != "" then .workflowName else "workflow run" end), + (.conclusion // "unknown"), + (.url // ""), + ((.databaseId // "") | tostring), + "" + ] + | @tsv + ' >"$workflow_run_contexts" + +while IFS=$'\t' read -r kind label conclusion details_url run_id check_run_id; do + if [ -z "$run_id" ]; then + continue + fi + if awk -F '\t' -v run_id="$run_id" '$5 == run_id { found = 1 } END { exit found ? 0 : 1 }' "$failed_contexts"; then + continue + fi + printf '%s\t%s\t%s\t%s\t%s\t%s\n' "$kind" "$label" "$conclusion" "$details_url" "$run_id" "$check_run_id" >>"$failed_contexts" +done <"$workflow_run_contexts" + +{ + printf '# Failed GitHub Check Evidence\n\n' + printf -- '- PR: #%s\n' "$PR_NUMBER" + printf -- '- Head SHA: `%s`\n' "$HEAD_SHA" + printf -- '- Repository: `%s`\n\n' "$GH_REPOSITORY" + printf '## Line-specific repair contract\n\n' + printf -- '- Treat the check logs and annotations below as diagnostic evidence, not as a complete review.\n' + printf -- '- For each actionable failed check, inspect the local source or diff and identify the exact file line that must change.\n' + printf -- '- OpenCode `REQUEST_CHANGES` findings must include `path`, `line`, `root_cause`, `fix_direction`, `regression_test_direction`, and `suggested_diff`.\n' + printf -- '- Do not request changes with only a GitHub Actions URL or a generic check name.\n\n' + printf -- '- When Strix logs contain multiple `Vulnerability Report` or `Model ... Vulnerabilities ...` sections, include every model-reported vulnerability in the review evidence and findings, including model name, title, severity, endpoint, and Code Locations/path:line evidence when present.\n' + printf -- '- Create one OpenCode finding per Strix model vulnerability report; do not satisfy two model reports with one combined finding, even when titles or locations match.\n\n' + + if [ ! -s "$failed_contexts" ]; then + printf 'No completed failed GitHub Checks were present when evidence was collected.\n' + exit 0 + fi + + while IFS=$'\t' read -r kind label conclusion details_url run_id check_run_id; do + printf '## Failed check: %s\n\n' "$label" + printf -- '- Type: `%s`\n' "$kind" + printf -- '- Conclusion: `%s`\n' "$conclusion" + if [ -n "$details_url" ]; then + printf -- '- Details URL: %s\n' "$details_url" + fi + if [ -n "$run_id" ]; then + printf -- '- Workflow run id: `%s`\n' "$run_id" + fi + if [ -n "$check_run_id" ]; then + printf -- '- Check run id: `%s`\n' "$check_run_id" + fi + printf '\n' + + if [ "$kind" = "workflow_run" ] && [ -n "$run_id" ]; then + log_file="$(mktemp)" + stripped_log_file="$(mktemp)" + tmp_files+=("$log_file" "$stripped_log_file") + if gh run view "$run_id" --repo "$GH_REPOSITORY" --log-failed >"$log_file" 2>&1; then + strip_ansi <"$log_file" >"$stripped_log_file" + if [ -s "$stripped_log_file" ]; then + emit_failure_signal_summary "$stripped_log_file" || true + printf '### Failed workflow run log excerpt\n\n' + printf '```text\n' + emit_bounded_file "$stripped_log_file" "$FAILED_CHECK_LOG_LINES" + printf '\n```\n\n' + if [[ "$label" == *Strix* ]]; then + emit_strix_vulnerability_evidence "$stripped_log_file" || true + fi + else + printf 'No GitHub Actions job log is available for this failed workflow run.\n\n' + if [ "$conclusion" = "cancelled" ]; then + printf 'The workflow run completed as cancelled before GitHub emitted a failed job log. Treat this as missing current-head security evidence, not as a source-code vulnerability report.\n\n' + fi + fi + else + strip_ansi <"$log_file" >"$stripped_log_file" + printf 'No GitHub Actions job log is available for this failed workflow run.\n\n' + printf '```text\n' + emit_bounded_file "$stripped_log_file" 60 + printf '\n```\n\n' + fi + continue + fi + + if [ "$kind" != "check_run" ] || [ -z "$check_run_id" ]; then + printf 'No GitHub Actions job log is available for this status context.\n\n' + continue + fi + + job_json="$(mktemp)" + tmp_files+=("$job_json") + if gh api -X GET "repos/${GH_REPOSITORY}/actions/jobs/${check_run_id}" >"$job_json" 2>/dev/null; then + failed_steps="$( + jq -r ' + (.steps // []) + | map(select((.conclusion // "" | ascii_downcase) as $c | ["failure","timed_out","cancelled","startup_failure"] | index($c))) + | .[] + | "- step " + ((.number // 0) | tostring) + ": " + (.name // "step") + " (" + (.conclusion // "unknown") + ")" + ' "$job_json" + )" + if [ -n "$failed_steps" ]; then + printf '### Failed job steps\n\n' + printf '%s\n\n' "$failed_steps" + fi + fi + + annotations_tmp="$(mktemp)" + tmp_files+=("$annotations_tmp") + if gh api -X GET "repos/${GH_REPOSITORY}/check-runs/${check_run_id}/annotations" --paginate \ + --jq ' + .[]? + | "- " + (.path // "unknown") + ":" + ((.start_line // 0) | tostring) + "-" + ((.end_line // .start_line // 0) | tostring) + " [" + (.annotation_level // "annotation") + "] " + ((.message // .title // "") | gsub("\r|\n"; " ")) + ' >"$annotations_tmp" 2>/dev/null; then + if [ -s "$annotations_tmp" ]; then + printf '### Check annotations\n\n' + emit_bounded_file "$annotations_tmp" 40 + printf '\n' + fi + fi + + log_raw="$(mktemp)" + log_clean="$(mktemp)" + tmp_files+=("$log_raw" "$log_clean") + if [ -n "$run_id" ] && gh run view "$run_id" \ + --repo "$GH_REPOSITORY" \ + --job "$check_run_id" \ + --log-failed >"$log_raw" 2>&1; then + strip_ansi <"$log_raw" >"$log_clean" + if [ -s "$log_clean" ]; then + emit_failure_signal_summary "$log_clean" || true + if emit_strix_vulnerability_evidence "$log_clean"; then + printf '\n' + fi + printf '### Failed log excerpt\n\n' + printf '```text\n' + emit_bounded_file "$log_clean" "$FAILED_CHECK_LOG_LINES" + printf '\n```\n\n' + fi + else + printf '### Failed log excerpt\n\n' + printf 'The failed job log could not be collected with `gh run view --log-failed`.\n\n' + if [ -s "$log_raw" ]; then + printf '```text\n' + strip_ansi <"$log_raw" | sed -n '1,40p' + printf '\n```\n\n' + fi + fi + done <"$failed_contexts" +} >"$OUTPUT_FILE" diff --git a/scripts/ci/emit_opencode_failed_check_fallback_findings.sh b/scripts/ci/emit_opencode_failed_check_fallback_findings.sh new file mode 100755 index 00000000..96775b16 --- /dev/null +++ b/scripts/ci/emit_opencode_failed_check_fallback_findings.sh @@ -0,0 +1,434 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [ "$#" -lt 1 ] || [ "$#" -gt 2 ]; then + echo "usage: $0 [repo-root]" >&2 + exit 64 +fi + +EVIDENCE_FILE="$1" +REPO_ROOT="${2:-${GITHUB_WORKSPACE:-$PWD}}" +finding_index=0 +tmp_files=() + +cleanup() { + rm -f "${tmp_files[@]}" +} +trap cleanup EXIT + +normalize_source_path() { + local raw_path="$1" + local candidate + + candidate="$(printf '%s' "$raw_path" | sed -E 's#^/workspace/[^/]+/##; s#^/tmp/strix-pr-scope\.[^/]+/##; s#^\./##; s#^/##')" + case "$candidate" in + services/*.py) + candidate="backend/$candidate" + ;; + src/*) + if [ -e "${REPO_ROOT%/}/frontend/$candidate" ]; then + candidate="frontend/$candidate" + fi + ;; + esac + printf '%s' "$candidate" +} + +first_existing_line() { + local path="$1" + local pattern="${2:-}" + local match="" + + if [ ! -f "${REPO_ROOT%/}/$path" ]; then + printf '1' + return 0 + fi + if [ -n "$pattern" ]; then + match="$(grep -nE -- "$pattern" "${REPO_ROOT%/}/$path" | head -n 1 || true)" + if [ -n "$match" ]; then + printf '%s' "${match%%:*}" + return 0 + fi + fi + printf '1' +} + +derive_location_from_report() { + local title="$1" + local endpoint="$2" + local target="$3" + local raw_location="$4" + local clean_location="" + local path="" + local line="" + local line_range="" + + if [ -n "$raw_location" ]; then + clean_location="$(normalize_source_path "$raw_location")" + path="${clean_location%:*}" + line_range="${clean_location##*:}" + line="${line_range%%-*}" + if [ -f "${REPO_ROOT%/}/$path" ] && [[ "$line" =~ ^[0-9]+$ ]]; then + printf '%s\t%s\t%s' "$path" "$line" "$raw_location" + return 0 + fi + fi + + if [[ "$target" =~ (backend/[^[:space:]]+|frontend/[^[:space:]]+|\.github/[^[:space:]]+|scripts/[^[:space:]]+) ]]; then + path="$(normalize_source_path "${BASH_REMATCH[1]}")" + elif [[ "$endpoint" =~ ^/services/.*\.py$ ]]; then + path="$(normalize_source_path "${endpoint#/}")" + fi + + if [ -n "$path" ] && [ -f "${REPO_ROOT%/}/$path" ]; then + line="$(first_existing_line "$path")" + printf '%s\t%s\t%s' "$path" "$line" "target/endpoint: ${target:-$endpoint}" + return 0 + fi + + case "$title" in + *"docker_entrypoint.sh"*|*"Docker Runtime Failure"*) + path="Dockerfile" + line="$(first_existing_line "$path" '^CMD \["/app/scripts/docker_entrypoint\.sh"\]|^ENTRYPOINT .*docker_entrypoint\.sh')" + ;; + *"Path Traversal"*Attachment*|*"attachment"*filename*) + path="backend/services/email_parser.py" + line="$(first_existing_line "$path" 'filename = part\.get_filename\(\)|"filename":')" + ;; + *"OIDC"*|*"session token"*|*"Session Token"*) + path="frontend/src/lib/oidc-session.ts" + line="$(first_existing_line "$path" 'sessionStorage\.setItem')" + ;; + *"Prompt"*Studio*|*"Prompt Injection"*) + path="frontend/src/app/prompt-studio/page.tsx" + line="$(first_existing_line "$path" "apiClient\\.post|testResult|setTestResult")" + ;; + *"Frontend Security Issues"*|*"Hardcoded Credentials"*|*"Insecure Data Handling"*) + path="frontend/next.config.ts" + line="$(first_existing_line "$path" 'const nextConfig|headers|Content-Security-Policy')" + if [ ! -f "${REPO_ROOT%/}/$path" ]; then + path="frontend/src/app/page.tsx" + line="$(first_existing_line "$path")" + fi + ;; + *"Content Security Policy"*|*"security headers"*|*"Security Headers"*) + path="frontend/next.config.ts" + line="$(first_existing_line "$path" 'const nextConfig|headers')" + ;; + *"JWT"*|*"Authentication"*) + path="backend/api/auth.py" + line="$(first_existing_line "$path" 'jwt\.decode|JWT_DECODE_REQUIRED_CLAIMS|_build_oidc_jwks_client')" + ;; + esac + + if [ -n "$path" ] && [ -f "${REPO_ROOT%/}/$path" ] && [[ "$line" =~ ^[0-9]+$ ]]; then + printf '%s\t%s\t%s' "$path" "$line" "derived from Strix title: $title" + return 0 + fi + + printf 'unknown\t1\tStrix report did not include a mappable Code Location' +} + +extract_strix_failed_check_block() { + local source_file="$1" + local output_file="$2" + + awk ' + /^## Failed check: / { + in_strix = ($0 ~ /^## Failed check: .*Strix/) + } + in_strix { print } + ' "$source_file" >"$output_file" +} + +extract_strix_reports() { + local source_file="$1" + perl -CS -ne ' + sub clean { + my ($line) = @_; + $line =~ s/\r//g; + $line =~ s/\x1b\[[0-9;?]*[A-Za-z]//g; + if ($line =~ /│/) { + $line =~ s/^.*?│[[:space:]]*//; + $line =~ s/[[:space:]]*│.*$//; + } else { + $line =~ s/^.*?[0-9]Z[[:space:]]+//; + } + $line =~ s/[[:space:]]+/ /g; + $line =~ s/^[[:space:]]+|[[:space:]]+$//g; + return $line; + } + sub starts_new_field { + my ($line) = @_; + return $line =~ /^(Title|Severity|CVSS Score|CVSS Vector|Target|Endpoint|Method|Description|Impact|Technical Analysis|PoC Description|PoC Code|Code Locations|Remediation)\b/i; + } + sub finish_report { + return unless defined $title && length $title; + push @reports, { + model => $report_model, + title => $title, + severity => $severity, + endpoint => $endpoint, + method => $method, + target => $target, + location => $location, + }; + ($report_model, $title, $severity, $endpoint, $method, $target, $location) = ("", "", "", "", "", "", ""); + } + sub finish_window { + finish_report(); + for my $report (@reports) { + my $model = $report->{model} || $window_model || $current_model || "unknown-model"; + for my $field ($model, @$report{qw(title severity endpoint method target location)}) { + $field //= ""; + $field =~ s/\t/ /g; + } + print join("\x1f", $model, @$report{qw(title severity endpoint method target location)}), "\n"; + } + @reports = (); + $window_model = ""; + } + my $line = clean($_); + if ($line =~ /^### Strix vulnerability report window/i) { + finish_window(); + $in_window = 1; + if ($line =~ m{(?:model|for model)[[:space:]]+((?:github[-_]models|openai|deepseek|vertex_ai)/[A-Za-z0-9._/-]+)}i) { + $window_model = $1; + $current_model = $1; + } + next; + } + if ($line =~ m{(?:^|[[:space:]])Model[[:space:]]+((?:github[-_]models|openai|deepseek|vertex_ai)/[A-Za-z0-9._/-]+)}i || + $line =~ m{Strix run failed for model '\''([^'\'']+)'\''}) { + $current_model = $1; + $window_model ||= $1 if $in_window; + $report_model = $1 if defined $title && length $title; + } + next unless $in_window; + if (defined $continuation_field && length $continuation_field) { + if (!length $line) { + $continuation_field = ""; + } elsif (!starts_new_field($line) && $line !~ /^[╭╰─]+/ && $line !~ /^Vulnerability Report$/i) { + if ($continuation_field eq "title") { + $title .= " " . $line; + } elsif ($continuation_field eq "endpoint") { + $endpoint .= " " . $line; + } elsif ($continuation_field eq "target") { + $target .= " " . $line; + } + next; + } else { + $continuation_field = ""; + } + } + if ($line =~ /^Title:[[:space:]]+(.+)/i) { + finish_report(); + $title = $1; + $report_model = $window_model || $current_model || ""; + $continuation_field = "title"; + next; + } + if ($line =~ /^Severity:[[:space:]]+(CRITICAL|HIGH|MEDIUM|LOW|NONE)\b/i) { + $severity = uc($1); + next; + } + if ($line =~ /^Endpoint:[[:space:]]+(.+)/i) { + $endpoint = $1; + $continuation_field = "endpoint"; + next; + } + if ($line =~ /^Method:[[:space:]]+(.+)/i) { + $method = $1; + $continuation_field = ""; + next; + } + if ($line =~ /^Target:[[:space:]]+(.+)/i) { + $target = $1; + $continuation_field = "target"; + next; + } + if ($line =~ /(?:Code[[:space:]]+)?Location(?:s)?(?:[[:space:]]+[0-9]+)?[[:space:]]*:[[:space:]]*(.+?:[0-9]+(?:-[0-9]+)?)/i) { + $location ||= $1; + next; + } + END { + finish_window(); + } + ' "$source_file" +} + +emit_known_missing_string_finding() { + local evidence_file="$1" + local needle="$2" + local title="$3" + local preferred_path + local match="" + local path="" + local line="" + + if ! grep -Fq -- "$needle" "$evidence_file"; then + return 0 + fi + + shift 3 + for preferred_path in "$@"; do + if [ -f "${REPO_ROOT%/}/$preferred_path" ]; then + match="$(grep -nF -- "$needle" "${REPO_ROOT%/}/$preferred_path" | head -n 1 || true)" + if [ -n "$match" ]; then + path="$preferred_path" + line="${match%%:*}" + break + fi + fi + done + + finding_index=$((finding_index + 1)) + if [ -n "$path" ] && [ -n "$line" ]; then + printf '### %s. HIGH %s:%s - %s\n' "$finding_index" "$path" "$line" "$title" + printf -- '- Problem: Strix failed because the trusted self-test log reported missing "%s".\n' "$needle" + printf -- '- Root cause: The failed check is executing trusted-base workflow material, so this exact line must exist in the trusted workflow/test contract before the check can pass.\n' + printf -- '- Fix: Keep or add the current-head line at "%s:%s" so trusted-base Strix/OpenCode evidence contains "%s".\n' "$path" "$line" "$needle" + printf -- '- Regression test: Keep scripts/ci/test_strix_quick_gate.sh assertions covering this exact string.\n\n' + else + printf '### %s. HIGH unknown:1 - %s\n' "$finding_index" "$title" + printf -- '- Problem: Strix failed because the trusted self-test log reported missing "%s".\n' "$needle" + printf -- '- Root cause: No current-head line containing this exact string was found in the expected workflow/test files.\n' + printf -- '- Fix: Add the exact string "%s" to the relevant workflow or test contract line.\n' "$needle" + printf -- '- Regression test: Add a static assertion for this exact string.\n\n' + fi +} + +emit_strix_report_findings() { + local strix_evidence_file="$1" + local reports_file + local model + local title + local severity + local endpoint + local method + local target + local location + local mapped + local path + local line + local source_detail + + if ! grep -Fq "Strix vulnerability report window" "$strix_evidence_file"; then + return 0 + fi + + reports_file="$(mktemp)" + tmp_files+=("$reports_file") + extract_strix_reports "$strix_evidence_file" >"$reports_file" + + while IFS=$'\037' read -r model title severity endpoint method target location; do + if [ -z "$title" ] || [ "$severity" = "NONE" ]; then + continue + fi + mapped="$(derive_location_from_report "$title" "$endpoint" "$target" "$location")" + IFS=$'\t' read -r path line source_detail <<<"$mapped" + if [ "$path" = "unknown" ]; then + path=".github/workflows/strix.yml" + line="$(first_existing_line "$path" 'STRIX_FAIL_ON_MIN_SEVERITY|STRIX_FALLBACK_MODELS')" + source_detail="$source_detail; fallback anchored to Strix workflow because the report omitted a repository Code Location" + fi + + finding_index=$((finding_index + 1)) + printf '### %s. %s %s:%s - Strix report from %s: %s\n' "$finding_index" "${severity:-HIGH}" "$path" "$line" "$model" "$title" + printf -- '- Problem: Strix Security Scan failed and %s reported "%s" with severity %s. Endpoint: %s. Method: %s. Code location evidence: %s.\n' "$model" "$title" "${severity:-UNKNOWN}" "${endpoint:-N/A}" "${method:-N/A}" "$source_detail" + printf -- '- Root cause: The failed Strix evidence contains a distinct model vulnerability report, so OpenCode must not collapse it into provider-quota or generic check-failure text.\n' + printf -- '- Fix: Inspect and patch %s:%s for this exact report before approval; apply the remediation described by Strix for "%s" and keep the review finding tied to this line.\n' "$path" "$line" "$title" + printf -- '- Regression test: Add or update coverage that exercises the reported endpoint/path and proves the %s finding cannot recur.\n\n' "${severity:-Strix}" + done <"$reports_file" +} + +emit_strix_provider_failure_finding() { + local strix_evidence_file="$1" + local match="" + local path=".github/workflows/strix.yml" + local line="1" + + if ! grep -Eq "LLM CONNECTION FAILED|RateLimitError|Too many requests|budget limit|Configured model and fallback models were unavailable|provider infrastructure|Below-threshold findings detected|Unable to map Strix findings" "$strix_evidence_file"; then + return 0 + fi + + if [ -f "${REPO_ROOT%/}/$path" ]; then + match="$(grep -nE -- "^[[:space:]]*STRIX_FALLBACK_MODELS:" "${REPO_ROOT%/}/$path" | head -n 1 || true)" + if [ -n "$match" ]; then + line="${match%%:*}" + fi + fi + + finding_index=$((finding_index + 1)) + if grep -Fq "Strix vulnerability report window" "$strix_evidence_file"; then + printf '### %s. HIGH %s:%s - Strix provider signal left current-head security evidence incomplete\n' "$finding_index" "$path" "$line" + printf -- '- Problem: Strix produced one or more vulnerability report windows, then the failed log still reported provider infrastructure/failure-signal output such as LLM CONNECTION FAILED, RateLimitError, budget-limit, "Below-threshold findings detected", "Unable to map Strix findings", or fallback provider signal.\n' + printf -- '- Root cause: The scanner evidence is incomplete even after model reports were emitted; OpenCode must include every model report above and must not approve until a clean current-head Strix run or equivalent manual evidence exists.\n' + printf -- '- Fix: Re-run Strix after GitHub Models capacity recovers or run an explicitly configured manual provider evidence scan with valid credentials; keep %s:%s aligned with the approved fallback model list.\n' "$path" "$line" + printf -- '- Regression test: Keep failed-check evidence and validation covering provider-signal failures after vulnerability reports so partial reports cannot be downgraded to approval.\n\n' + else + printf '### %s. HIGH %s:%s - Strix provider quota blocked current-head security evidence\n' "$finding_index" "$path" "$line" + printf -- '- Problem: Strix failed before producing vulnerability reports. The failed log reported LLM CONNECTION FAILED, RateLimitError or Too many requests for the primary model, budget-limit output for the DeepSeek fallbacks, and Configured model and fallback models were unavailable.\n' + printf -- '- Root cause: The configured GitHub Models primary/fallback provider capacity or budget was exhausted for this run; no Strix Vulnerability Report window was produced, so there is no application source line to patch from this evidence.\n' + printf -- '- Fix: Do not approve from this failed scan. Re-run Strix after GitHub Models quota recovers or run an explicitly configured manual provider evidence scan with valid credentials; keep the configured fallback line at %s:%s aligned with the approved model list.\n' "$path" "$line" + printf -- '- Regression test: Keep the failed-check evidence collector preserving RateLimitError, budget-limit, provider infrastructure, and unavailable-model lines so OpenCode reviews can distinguish external provider blockers from code vulnerabilities.\n\n' + fi +} + +emit_strix_cancelled_without_log_finding() { + local strix_evidence_file="$1" + local match="" + local path=".github/workflows/strix.yml" + local line="1" + + if ! grep -Fq "Conclusion:" "$strix_evidence_file" || + ! grep -Fq "cancelled" "$strix_evidence_file" || + ! grep -Fq "No GitHub Actions job log is available for this failed workflow run." "$strix_evidence_file"; then + return 0 + fi + + if [ -f "${REPO_ROOT%/}/$path" ]; then + match="$(grep -nF -- "cancel-in-progress: false" "${REPO_ROOT%/}/$path" | head -n 1 || true)" + if [ -n "$match" ]; then + line="${match%%:*}" + fi + fi + + finding_index=$((finding_index + 1)) + printf '### %s. HIGH %s:%s - Current-head Strix evidence is missing because the workflow run was cancelled before logs\n' "$finding_index" "$path" "$line" + printf -- '- Problem: Strix Security Scan reported a current-head workflow_run conclusion of cancelled, but GitHub emitted no failed job log and no Strix Vulnerability Report window.\n' + printf -- '- Root cause: The security gate has no usable Strix evidence for this head SHA. This is a workflow execution/queue state, not an application vulnerability finding, so OpenCode must not invent a source-code fix.\n' + printf -- '- Fix: Do not approve from this cancelled run. Re-run the current-head Strix Security Scan after stale runs complete or are cancelled, then review the resulting job log; keep the workflow concurrency line at %s:%s so stale runs do not silently replace current-head evidence.\n' "$path" "$line" + printf -- '- Regression test: Keep failed-check evidence collection explicit for cancelled workflow runs with no job log so reviewers see that the blocker is missing scanner evidence.\n\n' +} + +strix_evidence_file="$(mktemp)" +tmp_files+=("$strix_evidence_file") +extract_strix_failed_check_block "$EVIDENCE_FILE" "$strix_evidence_file" + +emit_known_missing_string_finding \ + "$EVIDENCE_FILE" \ + "github.event.inputs.strix_llm || 'openai/gpt-5'" \ + "Strix PR scans must default to GitHub Models GPT-5" \ + ".github/workflows/strix.yml" \ + "scripts/ci/test_strix_quick_gate.sh" +emit_known_missing_string_finding \ + "$EVIDENCE_FILE" \ + "STRIX_LLM must select GitHub Models openai/gpt-5 or newer, direct OpenAI GPT-5.4 or newer, or an approved organization Vertex AI model" \ + "Strix unsupported-model errors must name the allowed providers" \ + ".github/workflows/strix.yml" \ + "scripts/ci/test_strix_quick_gate.sh" +emit_known_missing_string_finding \ + "$EVIDENCE_FILE" \ + "MODEL: github-models/openai/gpt-5" \ + "OpenCode review must try GitHub Models GPT-5 first" \ + ".github/workflows/opencode-review.yml" \ + "scripts/ci/test_strix_quick_gate.sh" + +emit_strix_report_findings "$strix_evidence_file" +emit_strix_provider_failure_finding "$strix_evidence_file" +emit_strix_cancelled_without_log_finding "$strix_evidence_file" + +if [ "$finding_index" -eq 0 ]; then + printf 'No deterministic missing-string markers or Strix report locations were recognized. Use the failed-check evidence below to map each failed check to exact local source lines before approving.\n\n' +fi diff --git a/scripts/ci/opencode_review_approve_gate.sh b/scripts/ci/opencode_review_approve_gate.sh new file mode 100755 index 00000000..c6f10694 --- /dev/null +++ b/scripts/ci/opencode_review_approve_gate.sh @@ -0,0 +1,235 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [ $# -ne 4 ] && [ $# -ne 5 ]; then + echo "usage: $0 [normalized_json_file]" >&2 + exit 64 +fi + +SCRIPT_DIR="$( + CDPATH='' + cd -P -- "$(dirname -- "$0")" + pwd -P +)" +NORMALIZER="$SCRIPT_DIR/opencode_review_normalize_output.py" +EXPECTED_HEAD_SHA="$1" +EXPECTED_RUN_ID="$2" +EXPECTED_RUN_ATTEMPT="$3" +COMMENT_FILE="$4" +NORMALIZED_JSON_FILE="${5:-}" + +if [ ! -r "$COMMENT_FILE" ]; then + echo "error: cannot read comment body file: $COMMENT_FILE" >&2 + exit 65 +fi + +SENTINEL_LINE="$( + grep -E '' \ + "$COMMENT_FILE" | head -1 || true +)" + +if [ -z "$SENTINEL_LINE" ]; then + echo "MISSING_SENTINEL" + exit 2 +fi + +SENTINEL_HEAD_SHA="$(echo "$SENTINEL_LINE" | sed -nE 's/.*head_sha=([^[:space:]]+).*/\1/p')" +SENTINEL_RUN_ID="$(echo "$SENTINEL_LINE" | sed -nE 's/.*run_id=([^[:space:]]+).*/\1/p')" +SENTINEL_RUN_ATTEMPT="$(echo "$SENTINEL_LINE" | sed -nE 's/.*run_attempt=([^[:space:]]+).*/\1/p')" + +if [ "$SENTINEL_HEAD_SHA" != "$EXPECTED_HEAD_SHA" ]; then + echo "SHA_MISMATCH" + exit 3 +fi + +if [ -z "$SENTINEL_RUN_ID" ] || [ -z "$SENTINEL_RUN_ATTEMPT" ]; then + echo "MISSING_SENTINEL" + exit 2 +fi + +if [ "$EXPECTED_RUN_ID" != "-" ] && [ "$SENTINEL_RUN_ID" != "$EXPECTED_RUN_ID" ]; then + echo "MISSING_SENTINEL" + exit 2 +fi + +if [ "$EXPECTED_RUN_ATTEMPT" != "-" ] && [ "$SENTINEL_RUN_ATTEMPT" != "$EXPECTED_RUN_ATTEMPT" ]; then + echo "MISSING_SENTINEL" + exit 2 +fi + +CONTROL_JSON="$( + awk ' + /^[[:space:]]*$/ { exit } + in_block { print } + ' "$COMMENT_FILE" +)" + +if [ -z "$CONTROL_JSON" ]; then + echo "NO_CONCLUSION" + exit 4 +fi + +TMP_JSON="$(mktemp)" +trap 'rm -f "$TMP_JSON" "${TMP_JSON}.normalized"' EXIT +printf '%s\n' "$CONTROL_JSON" >"$TMP_JSON" + +if ! jq -e . "$TMP_JSON" >/dev/null 2>&1; then + echo "NO_CONCLUSION" + exit 4 +fi + +CONTROL_HEAD_SHA="$(jq -r '.head_sha // empty' "$TMP_JSON")" +CONTROL_RUN_ID="$(jq -r '.run_id // empty' "$TMP_JSON")" +CONTROL_RUN_ATTEMPT="$(jq -r '.run_attempt // empty' "$TMP_JSON")" +RESULT="$(jq -r '.result // empty' "$TMP_JSON")" + +if [ "$RESULT" = "APPROVE" ]; then + TMP_NORMALIZED_JSON="${TMP_JSON}.normalized" + jq '.findings = (.findings // [])' "$TMP_JSON" >"$TMP_NORMALIZED_JSON" + mv "$TMP_NORMALIZED_JSON" "$TMP_JSON" +fi + +if [ "$CONTROL_HEAD_SHA" != "$EXPECTED_HEAD_SHA" ]; then + echo "SHA_MISMATCH" + exit 3 +fi + +if [ "$EXPECTED_RUN_ID" != "-" ] && [ "$CONTROL_RUN_ID" != "$EXPECTED_RUN_ID" ]; then + echo "MISSING_SENTINEL" + exit 2 +fi + +if [ "$EXPECTED_RUN_ATTEMPT" != "-" ] && [ "$CONTROL_RUN_ATTEMPT" != "$EXPECTED_RUN_ATTEMPT" ]; then + echo "MISSING_SENTINEL" + exit 2 +fi + +if ! jq -e ' + type == "object" + and (.head_sha | type == "string" and length > 0) + and (.run_id | type == "string" and length > 0) + and (.run_attempt | type == "string" and length > 0) + and (.result == "APPROVE" or .result == "REQUEST_CHANGES") + and (.reason | type == "string" and length > 0) + and (.summary | type == "string" and length > 0) + and (.findings | type == "array") + and ( + if .result == "REQUEST_CHANGES" then (.findings | length > 0) + else (.findings | length == 0) + end + ) + and all(.findings[]; + (.path | type == "string" and length > 0) + and ((.path | ascii_downcase) as $p | ($p != "n/a" and $p != "unknown")) + and (.line | type == "number" and . > 0 and floor == .) + and (.severity | type == "string" and length > 0) + and (.title | type == "string" and length > 0) + and (.problem | type == "string" and length > 0) + and (.root_cause | type == "string" and length > 0) + and (.fix_direction | type == "string" and length > 0) + and (.regression_test_direction | type == "string" and length > 0) + and (.suggested_diff | type == "string" and length > 0) + and ((.suggested_diff | ascii_downcase) as $d | (($d | startswith("n/a")) | not) and (($d | startswith("cannot provide diff")) | not)) + ) +' "$TMP_JSON" >/dev/null; then + echo "NO_CONCLUSION" + exit 4 +fi + +if ! python3 "$NORMALIZER" --check-structural-approval "$TMP_JSON" >/dev/null; then + echo "NO_CONCLUSION" + exit 4 +fi + +SOURCE_ROOT="${GITHUB_WORKSPACE:-$PWD}" +if ! python3 - "$SOURCE_ROOT" "$TMP_JSON" <<'PY' +from __future__ import annotations + +import json +import os +import sys +from pathlib import Path + + +source_root = Path(sys.argv[1]).resolve() +control_file = Path(sys.argv[2]) +control = json.loads(control_file.read_text(encoding="utf-8")) + +if control.get("result") != "REQUEST_CHANGES": + raise SystemExit(0) + + +def normalized_line(value: str) -> str: + return " ".join(value.strip().split()) + + +def finding_is_source_backed(finding: dict[str, object]) -> bool: + path_value = str(finding.get("path", "")) + if ( + not path_value + or path_value.startswith("/") + or path_value == "." + or ".." in Path(path_value).parts + ): + return False + + source_file = (source_root / path_value).resolve() + try: + source_file.relative_to(source_root) + except ValueError: + return False + if not source_file.is_file(): + return False + + try: + source_lines = source_file.read_text(encoding="utf-8").splitlines() + except UnicodeDecodeError: + return False + + line_number = finding.get("line") + if not isinstance(line_number, int) or line_number < 1 or line_number > len(source_lines): + return False + + source_line_set = { + normalized_line(line) + for line in source_lines + if normalized_line(line) + } + suggested_diff = str(finding.get("suggested_diff", "")) + removed_lines = [] + added_lines = [] + for raw_line in suggested_diff.splitlines(): + if raw_line.startswith("--- ") or raw_line.startswith("+++ "): + continue + if raw_line.startswith("-"): + stripped = normalized_line(raw_line[1:]) + if stripped: + removed_lines.append(stripped) + elif raw_line.startswith("+"): + stripped = normalized_line(raw_line[1:]) + if stripped: + added_lines.append(stripped) + + if not removed_lines and not added_lines: + return False + for removed_line in removed_lines: + if removed_line not in source_line_set: + return False + return True + + +if not all(finding_is_source_backed(finding) for finding in control.get("findings", [])): + raise SystemExit(1) +PY +then + echo "NO_CONCLUSION" + exit 4 +fi + +if [ -n "$NORMALIZED_JSON_FILE" ]; then + jq -c '{head_sha, run_id, run_attempt, result, reason, summary, findings}' "$TMP_JSON" >"$NORMALIZED_JSON_FILE" +fi + +echo "$RESULT" +exit 0 diff --git a/scripts/ci/opencode_review_normalize_output.py b/scripts/ci/opencode_review_normalize_output.py new file mode 100755 index 00000000..c7bd9c41 --- /dev/null +++ b/scripts/ci/opencode_review_normalize_output.py @@ -0,0 +1,249 @@ +#!/usr/bin/env python3 +"""Normalize OpenCode review output into the strict approval-gate contract.""" + +from __future__ import annotations + +import json +import re +import sys +from pathlib import Path +from typing import Any + + +STRUCTURAL_FAILURE_PHRASES = ( + "structural exploration was not possible", + "structural exploration not possible", + "structural exploration is not required", + "structural exploration not required", + "structural analysis is not required", + "structural analysis not required", + "structural review is not required", + "structural review not required", + "no structural exploration required", + "no structural analysis required", + "no structural review required", + "structural exploration is unnecessary", + "structural analysis is unnecessary", + "structural review is unnecessary", + "could not be reviewed", + "could not inspect", + "could not be inspected", + "changed files could not be inspected", + "source files could not be inspected", + "required files could not be inspected", + "could not access changed files", + "could not access the changed files", + "could not access source files", + "could not access the source files", + "could not access required files", + "could not access required evidence", + "file access issues", + "file inaccessibility", + "evidence was truncated", + "not provided in evidence", + "truncated evidence", + "unable to inspect", + "insufficient evidence", +) + +STRUCTURAL_FAILURE_PATTERNS = ( + re.compile( + r"\b(?:could not|cannot|can't|unable to)\s+" + r"(?:inspect|access|review)\s+(?:the\s+)?" + r"(?:changed|source|required)\s+files?\b" + ), + re.compile( + r"\b(?:changed|source|required)\s+files?\s+" + r"(?:could not|cannot|can't|were not|was not)\s+" + r"(?:be\s+)?(?:inspected|accessed|reviewed)\b" + ), + re.compile( + r"\b(?:structural\s+(?:exploration|analysis|review))\s+" + r"(?:was\s+)?(?:unavailable|incomplete|blocked|not possible)\b" + ), +) + + +def admits_missing_structural_review(reason: str, summary: str) -> bool: + """Return whether an approval admits it did not inspect required structure.""" + combined = f"{reason}\n{summary}".casefold() + return any(phrase in combined for phrase in STRUCTURAL_FAILURE_PHRASES) or any( + pattern.search(combined) for pattern in STRUCTURAL_FAILURE_PATTERNS + ) + + +def check_structural_approval(control_file: Path) -> int: + """Reject approvals whose control JSON admits missing structural review.""" + try: + value = json.loads(control_file.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError) as exc: + print(f"cannot read OpenCode control JSON: {exc}", file=sys.stderr) + return 65 + + if not isinstance(value, dict): + print("NO_CONCLUSION", file=sys.stderr) + return 4 + + if value.get("result") == "APPROVE" and admits_missing_structural_review( + str(value.get("reason", "")), + str(value.get("summary", "")), + ): + print("NO_CONCLUSION", file=sys.stderr) + return 4 + + return 0 + + +def valid_control( + value: Any, + *, + expected_head_sha: str, + expected_run_id: str, + expected_run_attempt: str, +) -> dict[str, Any] | None: + """Return a normalized review control object when all gate fields are valid.""" + if not isinstance(value, dict): + return None + + if value.get("head_sha") != expected_head_sha: + return None + if value.get("run_id") != expected_run_id: + return None + if value.get("run_attempt") != expected_run_attempt: + return None + + result = value.get("result") + if result not in {"APPROVE", "REQUEST_CHANGES"}: + return None + + if not isinstance(value.get("reason"), str) or not value["reason"].strip(): + return None + if not isinstance(value.get("summary"), str) or not value["summary"].strip(): + return None + reason = value["reason"].strip() + summary = value["summary"].strip() + + findings = value.get("findings") + if findings is None and result == "APPROVE": + findings = [] + if not isinstance(findings, list): + return None + if result == "APPROVE" and findings: + return None + if result == "REQUEST_CHANGES" and not findings: + return None + if result == "APPROVE" and admits_missing_structural_review(reason, summary): + return None + + required_finding_fields = ( + "path", + "severity", + "title", + "problem", + "root_cause", + "fix_direction", + "regression_test_direction", + "suggested_diff", + ) + for finding in findings: + if not isinstance(finding, dict): + return None + if not isinstance(finding.get("line"), int) or finding["line"] <= 0: + return None + for field in required_finding_fields: + if not isinstance(finding.get(field), str) or not finding[field].strip(): + return None + + return { + "head_sha": value["head_sha"], + "run_id": value["run_id"], + "run_attempt": value["run_attempt"], + "result": result, + "reason": reason, + "summary": summary, + "findings": findings, + } + + +def iter_json_objects(text: str) -> list[Any]: + """Extract JSON objects from possibly noisy OpenCode output text.""" + decoder = json.JSONDecoder() + values: list[Any] = [] + + try: + values.append(json.loads(text)) + except json.JSONDecodeError: + # OpenCode exports may contain prose around the JSON control object. + pass + + for index, character in enumerate(text): + if character != "{": + continue + try: + value, _ = decoder.raw_decode(text[index:]) + except json.JSONDecodeError: + continue + values.append(value) + + return values + + +def main(argv: list[str]) -> int: + """Normalize an OpenCode output file for the shell approval gate.""" + if len(argv) == 3 and argv[1] == "--check-structural-approval": + return check_structural_approval(Path(argv[2])) + + if len(argv) != 5: + print( + "usage: opencode_review_normalize_output.py " + " \n" + " or: opencode_review_normalize_output.py --check-structural-approval ", + file=sys.stderr, + ) + return 64 + + expected_head_sha, expected_run_id, expected_run_attempt, output_file_arg = argv[1:] + output_file = Path(output_file_arg) + try: + output_text = output_file.read_text(encoding="utf-8") + except OSError as exc: + print(f"cannot read OpenCode output file: {exc}", file=sys.stderr) + return 65 + + for value in iter_json_objects(output_text): + control = valid_control( + value, + expected_head_sha=expected_head_sha, + expected_run_id=expected_run_id, + expected_run_attempt=expected_run_attempt, + ) + if control is None: + continue + + normalized_json = json.dumps(control, separators=(",", ":"), ensure_ascii=False) + output_file.write_text( + "\n".join( + [ + ( + "" + ), + "", + "", + "", + ] + ), + encoding="utf-8", + ) + return 0 + + print("NO_CONCLUSION", file=sys.stderr) + return 4 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv)) diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py new file mode 100644 index 00000000..1c30dd96 --- /dev/null +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -0,0 +1,429 @@ +#!/usr/bin/env python3 +"""Inspect open pull requests and enable safe OpenCode-gated auto-merge.""" + +from __future__ import annotations + +import argparse +import json +import os +import subprocess +import sys +from dataclasses import dataclass +from typing import Any + + +OPEN_PRS_QUERY = """\ +query($owner: String!, $name: String!, $pageSize: Int!, $cursor: String) { + repository(owner: $owner, name: $name) { + pullRequests(first: $pageSize, after: $cursor, states: OPEN, orderBy: {field: CREATED_AT, direction: ASC}) { + pageInfo { hasNextPage endCursor } + nodes { + number + title + isDraft + mergeable + reviewDecision + baseRefName + baseRefOid + headRefName + headRefOid + headRepository { nameWithOwner } + autoMergeRequest { enabledAt } + reviewThreads(first: 100) { + nodes { isResolved isOutdated } + } + reviews(last: 50) { + nodes { + state + body + submittedAt + author { login } + commit { oid } + } + } + statusCheckRollup { + contexts(first: 100) { + nodes { + __typename + ... on CheckRun { + name + status + conclusion + checkSuite { + workflowRun { + workflow { name } + } + } + } + ... on StatusContext { + context + state + } + } + } + } + } + } + } +} +""" + + +@dataclass +class Decision: + """Scheduler action selected for a pull request.""" + + pr: int + action: str + reason: str + + +def run(args: list[str], *, stdin: str | None = None) -> str: + """Run a command and return stdout.""" + + process = subprocess.run(args, input=stdin, capture_output=True, text=True) + if process.returncode != 0: + raise RuntimeError( + f"Command failed ({process.returncode}): {' '.join(args)}\n{process.stderr}" + ) + return process.stdout + + +def split_repo(repo: str) -> tuple[str, str]: + """Split an owner/name repository string.""" + + try: + owner, name = repo.split("/", 1) + except ValueError as exc: + raise ValueError(f"repo must be owner/name, got {repo!r}") from exc + if not owner or not name: + raise ValueError(f"repo must be owner/name, got {repo!r}") + return owner, name + + +def gh_graphql(query: str, **fields: str | int) -> dict[str, Any]: + """Execute a GitHub GraphQL query through gh.""" + + cmd = ["gh", "api", "graphql", "-F", "query=@-"] + for key, value in fields.items(): + flag = "-F" if isinstance(value, int) else "-f" + cmd.extend([flag, f"{key}={value}"]) + return json.loads(run(cmd, stdin=query)) + + +def fetch_open_prs(repo: str, max_prs: int) -> list[dict[str, Any]]: + """Fetch open pull requests from GitHub.""" + + owner, name = split_repo(repo) + prs: list[dict[str, Any]] = [] + cursor: str | None = None + + while len(prs) < max_prs: + page_size = min(100, max_prs - len(prs)) + fields: dict[str, str | int] = { + "owner": owner, + "name": name, + "pageSize": page_size, + } + if cursor: + fields["cursor"] = cursor + payload = gh_graphql(OPEN_PRS_QUERY, **fields) + pr_page = payload["data"]["repository"]["pullRequests"] + prs.extend(pr_page.get("nodes") or []) + if not pr_page["pageInfo"]["hasNextPage"]: + break + cursor = pr_page["pageInfo"]["endCursor"] + + return prs + + +def context_nodes(pr: dict[str, Any]) -> list[dict[str, Any]]: + """Return status context nodes for a pull request.""" + + rollup = pr.get("statusCheckRollup") or {} + contexts = rollup.get("contexts") or {} + return contexts.get("nodes") or [] + + +def is_opencode_context(node: dict[str, Any]) -> bool: + """Return whether a status node belongs to OpenCode review.""" + + if node.get("__typename") == "CheckRun": + workflow = ( + ((node.get("checkSuite") or {}).get("workflowRun") or {}).get("workflow") + or {} + ) + return node.get("name") == "opencode-review" or workflow.get("name") == "OpenCode Review" + return node.get("context") == "opencode-review" + + +def opencode_in_progress(pr: dict[str, Any]) -> bool: + """Return whether OpenCode review is still running.""" + + for node in context_nodes(pr): + if not is_opencode_context(node): + continue + status = (node.get("status") or node.get("state") or "").upper() + if status and status not in {"COMPLETED", "SUCCESS", "FAILURE", "ERROR"}: + return True + return False + + +def unresolved_thread_count(pr: dict[str, Any]) -> int: + """Count active unresolved review threads.""" + + threads = ((pr.get("reviewThreads") or {}).get("nodes") or []) + return sum(1 for thread in threads if not thread.get("isResolved") and not thread.get("isOutdated")) + + +def review_author_login(review: dict[str, Any]) -> str: + """Return a review author's normalized login.""" + + return ((review.get("author") or {}).get("login") or "").lower() + + +def is_opencode_review(review: dict[str, Any]) -> bool: + """Return whether a review was authored by OpenCode.""" + + login = review_author_login(review) + body = review.get("body") or "" + return login.startswith("opencode-agent") or "opencode" in login or "OpenCode Agent" in body + + +def current_head_review_state(pr: dict[str, Any], state: str) -> bool: + """Return whether the current head has a matching OpenCode review state.""" + + head = pr.get("headRefOid") + for review in reversed((pr.get("reviews") or {}).get("nodes") or []): + if not is_opencode_review(review): + continue + if (review.get("state") or "").upper() != state: + continue + commit = (review.get("commit") or {}).get("oid") + if commit == head: + return True + return False + + +def has_current_head_approval(pr: dict[str, Any]) -> bool: + """Return whether the current head has OpenCode approval.""" + + return current_head_review_state(pr, "APPROVED") + + +def has_current_head_changes_requested(pr: dict[str, Any]) -> bool: + """Return whether the current head has OpenCode changes requested.""" + + return current_head_review_state(pr, "CHANGES_REQUESTED") + + +def enable_auto_merge(repo: str, pr: dict[str, Any], *, dry_run: bool) -> None: + """Enable GitHub auto-merge for an approved pull request.""" + + number = str(pr["number"]) + head = pr["headRefOid"] + if dry_run: + return + run(["gh", "pr", "merge", number, "--repo", repo, "--auto", "--merge", "--match-head-commit", head]) + + +def dispatch_opencode_review(repo: str, workflow: str, pr: dict[str, Any], *, dry_run: bool) -> None: + """Dispatch the OpenCode review workflow for a pull request.""" + + if dry_run: + return + run( + [ + "gh", + "workflow", + "run", + workflow, + "--repo", + repo, + "--ref", + pr["baseRefName"], + "-f", + f"pr_number={pr['number']}", + "-f", + f"pr_base_ref={pr['baseRefName']}", + "-f", + f"pr_base_sha={pr['baseRefOid']}", + "-f", + f"pr_head_ref={pr['headRefName']}", + "-f", + f"pr_head_sha={pr['headRefOid']}", + ] + ) + + +def inspect_pr( + repo: str, + pr: dict[str, Any], + *, + dry_run: bool, + trigger_reviews: bool, + enable_auto_merge_flag: bool, + workflow: str, + base_branch: str, +) -> Decision: + """Inspect a pull request and select the scheduler action.""" + + number = pr["number"] + head_repo = (pr.get("headRepository") or {}).get("nameWithOwner") + base_ref = pr.get("baseRefName") + + if pr.get("isDraft"): + return Decision(number, "skip", "draft PR") + if base_ref != base_branch: + return Decision(number, "skip", f"base branch is {base_ref}; expected {base_branch}") + if head_repo != repo: + return Decision(number, "skip", f"fork or external head repo: {head_repo}") + + unresolved = unresolved_thread_count(pr) + if unresolved: + return Decision(number, "block", f"{unresolved} unresolved review thread(s)") + + if has_current_head_changes_requested(pr): + return Decision(number, "block", "current-head OpenCode review requested changes") + + if has_current_head_approval(pr): + if pr.get("autoMergeRequest"): + return Decision(number, "wait", "current head is approved; auto-merge already enabled") + if not enable_auto_merge_flag: + return Decision(number, "wait", "current head is approved; auto-merge disabled by scheduler inputs") + enable_auto_merge(repo, pr, dry_run=dry_run) + return Decision(number, "auto_merge", "current head is approved; auto-merge enabled") + + if opencode_in_progress(pr): + return Decision(number, "wait", "OpenCode review is already in progress") + + if trigger_reviews: + dispatch_opencode_review(repo, workflow, pr, dry_run=dry_run) + return Decision(number, "review_dispatch", "current head has no OpenCode approval") + + return Decision(number, "block", "current head has no OpenCode approval") + + +def print_summary( + decisions: list[Decision], + *, + dry_run: bool, + base_branch: str, + project_flow: str, +) -> None: + """Print human-readable and machine-readable scheduler results.""" + + counts: dict[str, int] = {} + for decision in decisions: + counts[decision.action] = counts.get(decision.action, 0) + 1 + print(f"PR #{decision.pr}: {decision.action}: {decision.reason}") + print( + json.dumps( + { + "base_branch": base_branch, + "dry_run": dry_run, + "inspected": len(decisions), + "counts": counts, + "project_flow": project_flow, + }, + sort_keys=True, + ) + ) + + +def self_test() -> None: + """Run scheduler behavior smoke tests.""" + + sample = { + "number": 1, + "headRefOid": "abc", + "isDraft": False, + "headRepository": {"nameWithOwner": "owner/repo"}, + "reviewDecision": "REVIEW_REQUIRED", + "reviewThreads": {"nodes": []}, + "reviews": { + "nodes": [ + { + "state": "APPROVED", + "author": {"login": "opencode-agent"}, + "body": "OpenCode Agent approved this head.", + "commit": {"oid": "abc"}, + } + ] + }, + "statusCheckRollup": {"contexts": {"nodes": []}}, + } + assert has_current_head_approval(sample) + assert not has_current_head_changes_requested(sample) + sample["reviews"]["nodes"].append( + { + "state": "CHANGES_REQUESTED", + "author": {"login": "opencode-agent"}, + "commit": {"oid": "old"}, + } + ) + assert not has_current_head_changes_requested(sample) + sample["statusCheckRollup"]["contexts"]["nodes"].append( + {"__typename": "CheckRun", "name": "opencode-review", "status": "IN_PROGRESS"} + ) + assert opencode_in_progress(sample) + print("self-test passed") + + +def parse_args(argv: list[str]) -> argparse.Namespace: + """Parse scheduler command-line arguments.""" + + parser = argparse.ArgumentParser() + parser.add_argument("--repo", default=os.environ.get("GITHUB_REPOSITORY", "")) + parser.add_argument("--base-branch", default=os.environ.get("DEFAULT_BRANCH", "")) + parser.add_argument("--project-flow", default=os.environ.get("PROJECT_FLOW", "")) + parser.add_argument("--max-prs", type=int, default=100) + parser.add_argument("--dry-run", action="store_true") + parser.add_argument("--trigger-reviews", action=argparse.BooleanOptionalAction, default=True) + parser.add_argument("--enable-auto-merge", action=argparse.BooleanOptionalAction, default=True) + parser.add_argument("--review-workflow", default="OpenCode Review") + parser.add_argument("--self-test", action="store_true") + return parser.parse_args(argv) + + +def main(argv: list[str]) -> int: + """Run the PR review merge scheduler.""" + + args = parse_args(argv) + if args.self_test: + self_test() + return 0 + if not args.repo: + raise SystemExit("--repo is required") + if not args.base_branch: + raise SystemExit("--base-branch is required") + if not args.project_flow: + raise SystemExit("--project-flow is required") + prs = fetch_open_prs(args.repo, args.max_prs) + decisions = [ + inspect_pr( + args.repo, + pr, + dry_run=args.dry_run, + trigger_reviews=args.trigger_reviews, + enable_auto_merge_flag=args.enable_auto_merge, + workflow=args.review_workflow, + base_branch=args.base_branch, + ) + for pr in prs + ] + print_summary( + decisions, + dry_run=args.dry_run, + base_branch=args.base_branch, + project_flow=args.project_flow, + ) + return 0 + + +if __name__ == "__main__": + try: + raise SystemExit(main(sys.argv[1:])) + except RuntimeError as exc: + print(str(exc), file=sys.stderr) + raise SystemExit(1) from exc diff --git a/scripts/ci/test_opencode_fact_gate_contract.sh b/scripts/ci/test_opencode_fact_gate_contract.sh new file mode 100755 index 00000000..1624f122 --- /dev/null +++ b/scripts/ci/test_opencode_fact_gate_contract.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +set -euo pipefail + +repo_root="$( + CDPATH='' + cd -P -- "$(dirname -- "$0")/../.." + pwd -P +)" +workflow_file="$repo_root/.github/workflows/opencode-review.yml" + +check_contains() { + local needle="$1" + if ! grep -Fq -- "$needle" "$workflow_file"; then + printf 'missing OpenCode fact-gate contract: %s\n' "$needle" >&2 + exit 1 + fi +} + +check_contains '## Changed docs repository tree evidence' +check_contains 'git ls-tree -r --name-only HEAD -- "$docs_dir"' +check_contains 'Do not claim repository docs, images, or reference assets are unavailable, missing, or absent unless the changed docs repository tree evidence proves it.' +check_contains 'collect_unresolved_human_review_threads()' +check_contains 'reviewThreads(first: 100)' +check_contains 'Latest unresolved human review thread evidence' +check_contains 'OpenCode reviewed the current-head evidence but found unresolved human review threads before approval.' + +printf 'OpenCode fact-gate contract OK\n' diff --git a/scripts/ci/validate_opencode_failed_check_review.sh b/scripts/ci/validate_opencode_failed_check_review.sh new file mode 100755 index 00000000..137d8691 --- /dev/null +++ b/scripts/ci/validate_opencode_failed_check_review.sh @@ -0,0 +1,391 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [ "$#" -ne 3 ]; then + echo "usage: $0 " >&2 + exit 64 +fi + +CONTROL_JSON_FILE="$1" +FAILED_CHECKS_FILE="$2" +FAILED_CHECK_EVIDENCE_FILE="$3" + +if [ ! -r "$CONTROL_JSON_FILE" ] || [ ! -r "$FAILED_CHECKS_FILE" ] || [ ! -r "$FAILED_CHECK_EVIDENCE_FILE" ]; then + echo "FAILED_CHECK_EVIDENCE_NOT_REFERENCED" + exit 4 +fi + +if [ ! -s "$FAILED_CHECKS_FILE" ]; then + exit 0 +fi + +review_text="$( + jq -r ' + [ + (.summary // ""), + (.reason // ""), + ( + .findings[]? + | [ + (.path // ""), + ((.line // "") | tostring), + (.severity // ""), + (.title // ""), + (.problem // ""), + (.root_cause // ""), + (.fix_direction // ""), + (.regression_test_direction // ""), + (.suggested_diff // "") + ] + | join("\n") + ) + ] + | join("\n") + ' "$CONTROL_JSON_FILE" +)" + +contains_review_text() { + local needle="$1" + if [ -z "$needle" ]; then + return 0 + fi + grep -Fqi -- "$needle" <<<"$review_text" +} + +extract_strix_required_markers() { + perl -CS -ne ' + s/\r//g; + s/\x1b\[[0-9;?]*[A-Za-z]//g; + if (/│/) { + s/^.*?│[[:space:]]*//; + s/[[:space:]]*│.*$//; + } else { + s/^.*?[0-9]Z[[:space:]]+//; + } + s/[[:space:]]+/ /g; + s/^[[:space:]]+|[[:space:]]+$//g; + + if (/^Title:[[:space:]]+(.+)/) { + print "$1\n"; + } + if (/^Severity:[[:space:]]+(CRITICAL|HIGH|MEDIUM|LOW)\b/) { + print "Severity: $1\n"; + } + if (/^Endpoint:[[:space:]]+(.+)/) { + print "$1\n"; + } + if (/^Method:[[:space:]]+(.+)/) { + print "Method: $1\n"; + } + if (/^Location[[:space:]]+[0-9]+:[[:space:]]+(.+:[0-9]+(?:-[0-9]+)?)/) { + print "$1\n"; + } + ' "$FAILED_CHECK_EVIDENCE_FILE" +} + +extract_strix_title_markers() { + perl -CS -ne ' + s/\r//g; + s/\x1b\[[0-9;?]*[A-Za-z]//g; + if (/│/) { + s/^.*?│[[:space:]]*//; + s/[[:space:]]*│.*$//; + } else { + s/^.*?[0-9]Z[[:space:]]+//; + } + s/[[:space:]]+/ /g; + s/^[[:space:]]+|[[:space:]]+$//g; + if (/^Title:[[:space:]]+(.+)/) { + print "$1\n"; + } + ' "$FAILED_CHECK_EVIDENCE_FILE" +} + +count_strix_review_findings() { + jq -r ' + [ + (.findings // [])[] + | [ + .title, + .problem, + .root_cause, + .fix_direction, + .regression_test_direction, + .suggested_diff + ] + | map(. // "") + | join("\n") + | select(test("strix|github[-_]models/|deepseek/|openai/gpt-|vertex_ai/|Vulnerability Report"; "i")) + ] + | length + ' "$CONTROL_JSON_FILE" +} + +validate_distinct_strix_report_findings() { + python3 - "$CONTROL_JSON_FILE" "$FAILED_CHECK_EVIDENCE_FILE" <<'PY' +from __future__ import annotations + +import json +import re +import sys +from pathlib import Path + + +control_file = Path(sys.argv[1]) +evidence_file = Path(sys.argv[2]) +control = json.loads(control_file.read_text(encoding="utf-8")) +evidence_text = evidence_file.read_text(encoding="utf-8", errors="replace") + +ansi_re = re.compile(r"\x1b\[[0-9;?]*[A-Za-z]") +model_re = re.compile( + r"(?:^|[\s])Model\s+((?:github[-_]models|openai|deepseek|vertex_ai)/[A-Za-z0-9._/-]+)", + re.IGNORECASE, +) +failed_model_re = re.compile(r"Strix run failed for model '([^']+)'") +location_re = re.compile( + r"(?:Code\s+)?Locations?(?:\s+[0-9]+)?\s*:\s*(.+?:[0-9]+(?:-[0-9]+)?)", + re.IGNORECASE, +) + + +def clean(raw_line: str) -> str: + line = ansi_re.sub("", raw_line).replace("\r", "") + if "│" in line: + line = re.sub(r"^.*?│\s*", "", line) + line = re.sub(r"\s*│.*$", "", line) + else: + line = re.sub(r"^.*?[0-9]Z\s+", "", line) + line = re.sub(r"\s+", " ", line).strip() + return line + + +def starts_new_field(line: str) -> bool: + return bool( + re.match( + r"^(Title|Severity|CVSS Score|CVSS Vector|Target|Endpoint|Method|Description|Impact|Technical Analysis|PoC Description|PoC Code|Code Locations|Remediation)\b", + line, + re.IGNORECASE, + ) + ) + + +def parse_reports(text: str) -> list[dict[str, str]]: + reports: list[dict[str, str]] = [] + in_window = False + window_model = "" + current_model = "" + report_model = "" + title = "" + severity = "" + endpoint = "" + method = "" + target = "" + location = "" + continuation = "" + + def finish_report() -> None: + nonlocal report_model, title, severity, endpoint, method, target, location + if title: + reports.append( + { + "model": report_model or window_model or current_model or "unknown-model", + "title": title, + "severity": severity, + "endpoint": endpoint, + "method": method, + "target": target, + "location": location, + } + ) + report_model = title = severity = endpoint = method = target = location = "" + + for raw_line in text.splitlines(): + line = clean(raw_line) + if line.lower().startswith("### strix vulnerability report window"): + finish_report() + in_window = True + window_model = "" + match = re.search( + r"(?:model|for model)\s+((?:github[-_]models|openai|deepseek|vertex_ai)/[A-Za-z0-9._/-]+)", + line, + re.IGNORECASE, + ) + if match: + window_model = match.group(1) + current_model = match.group(1) + continuation = "" + continue + + match = model_re.search(line) or failed_model_re.search(line) + if match: + current_model = match.group(1) + if in_window and not window_model: + window_model = current_model + if title and not report_model: + report_model = current_model + + if not in_window: + continue + + if continuation: + if not line: + continuation = "" + elif not starts_new_field(line) and not re.match(r"^[╭╰─]+$", line) and line.lower() != "vulnerability report": + if continuation == "title": + title = f"{title} {line}".strip() + elif continuation == "endpoint": + endpoint = f"{endpoint} {line}".strip() + elif continuation == "target": + target = f"{target} {line}".strip() + continue + else: + continuation = "" + + if line.lower() == "vulnerability report": + continue + field_match = re.match(r"^Title:\s+(.+)", line, re.IGNORECASE) + if field_match: + finish_report() + title = field_match.group(1) + report_model = window_model or current_model + continuation = "title" + continue + field_match = re.match(r"^Severity:\s+(CRITICAL|HIGH|MEDIUM|LOW|NONE)\b", line, re.IGNORECASE) + if field_match: + severity = field_match.group(1).upper() + continue + field_match = re.match(r"^Endpoint:\s+(.+)", line, re.IGNORECASE) + if field_match: + endpoint = field_match.group(1) + continuation = "endpoint" + continue + field_match = re.match(r"^Method:\s+(.+)", line, re.IGNORECASE) + if field_match: + method = field_match.group(1) + continuation = "" + continue + field_match = re.match(r"^Target:\s+(.+)", line, re.IGNORECASE) + if field_match: + target = field_match.group(1) + continuation = "target" + continue + field_match = location_re.search(line) + if field_match and not location: + location = field_match.group(1) + + finish_report() + return [report for report in reports if report["title"] and report["severity"] != "NONE"] + + +def finding_text(finding: dict[str, object]) -> str: + fields = [ + "path", + "line", + "severity", + "title", + "problem", + "root_cause", + "fix_direction", + "regression_test_direction", + "suggested_diff", + ] + return "\n".join(str(finding.get(field, "")) for field in fields).lower() + + +def contains(text: str, marker: str) -> bool: + return not marker or marker.lower() in text + + +reports = parse_reports(evidence_text) +if not reports: + raise SystemExit(0) + +findings = [finding_text(finding) for finding in control.get("findings", []) if isinstance(finding, dict)] +used_findings: set[int] = set() + +for report in reports: + required_markers = [ + report["model"], + report["title"], + report["severity"], + report["endpoint"], + report["method"], + report["location"], + ] + for index, text in enumerate(findings): + if index in used_findings: + continue + if all(contains(text, marker) for marker in required_markers): + used_findings.add(index) + break + else: + raise SystemExit(1) +PY +} + +while IFS= read -r failed_check_line; do + case "$failed_check_line" in + "- "*) + failed_check_label="${failed_check_line#- }" + failed_check_label="${failed_check_label%%:*}" + if ! contains_review_text "$failed_check_label"; then + echo "FAILED_CHECK_EVIDENCE_NOT_REFERENCED" + exit 4 + fi + ;; + esac +done <"$FAILED_CHECKS_FILE" + +while IFS= read -r fail_marker; do + if ! contains_review_text "$fail_marker"; then + echo "FAILED_CHECK_EVIDENCE_NOT_REFERENCED" + exit 4 + fi +done < <(awk -F 'FAIL: ' 'NF > 1 { print $2 }' "$FAILED_CHECK_EVIDENCE_FILE" | sort -u) + +for evidence_marker in \ + "Self-test Strix gate script" \ + "github.event.inputs.strix_llm" \ + "STRIX_LLM must select" \ + "MODEL: github-models/openai/gpt-5" +do + if grep -Fq -- "$evidence_marker" "$FAILED_CHECK_EVIDENCE_FILE" && + ! contains_review_text "$evidence_marker"; then + echo "FAILED_CHECK_EVIDENCE_NOT_REFERENCED" + exit 4 + fi +done + +if grep -Fq "Strix vulnerability report window" "$FAILED_CHECK_EVIDENCE_FILE"; then + if ! validate_distinct_strix_report_findings; then + echo "FAILED_CHECK_EVIDENCE_NOT_REFERENCED" + exit 4 + fi + + strix_title_count="$(extract_strix_title_markers | sed '/^[[:space:]]*$/d' | wc -l | tr -d '[:space:]')" + finding_count="$(count_strix_review_findings)" + if [ -n "$strix_title_count" ] && [ "$strix_title_count" -gt 0 ] && + [ "$finding_count" -lt "$strix_title_count" ]; then + echo "FAILED_CHECK_EVIDENCE_NOT_REFERENCED" + exit 4 + fi + + while IFS= read -r model_name; do + if ! contains_review_text "$model_name"; then + echo "FAILED_CHECK_EVIDENCE_NOT_REFERENCED" + exit 4 + fi + done < <( + perl -ne 'while (m{(?:openai|deepseek|vertex_ai|github(?:_|-)models)/[A-Za-z0-9._/-]+}g) { print "$&\n" }' \ + "$FAILED_CHECK_EVIDENCE_FILE" | sort -u + ) + + while IFS= read -r strix_marker; do + if ! contains_review_text "$strix_marker"; then + echo "FAILED_CHECK_EVIDENCE_NOT_REFERENCED" + exit 4 + fi + done < <(extract_strix_required_markers) +fi + +exit 0 diff --git a/services/analysis-engine/src/bandscope_analysis/youtube.py b/services/analysis-engine/src/bandscope_analysis/youtube.py index aa27d6f7..612a8a23 100644 --- a/services/analysis-engine/src/bandscope_analysis/youtube.py +++ b/services/analysis-engine/src/bandscope_analysis/youtube.py @@ -5,6 +5,7 @@ """ import argparse +import glob import json import os import re @@ -59,9 +60,8 @@ def _find_downloaded_file(actual_filepath: str) -> Optional[str]: if not os.path.exists(actual_filepath): # Try to find the file with a different extension in case of conversion base_path = os.path.splitext(actual_filepath)[0] - for ext in SUPPORTED_AUDIO_EXTENSIONS: - match = base_path + ext - if os.path.exists(match): + for match in glob.iglob(glob.escape(base_path) + ".*"): + if match.endswith(SUPPORTED_AUDIO_EXTENSIONS): return match return None return actual_filepath diff --git a/services/analysis-engine/tests/test_priority.py b/services/analysis-engine/tests/test_priority.py index 5d155b97..dfe135e0 100644 --- a/services/analysis-engine/tests/test_priority.py +++ b/services/analysis-engine/tests/test_priority.py @@ -82,43 +82,3 @@ def test_calculate_priority_with_manual_override() -> None: "setupNote": "", } assert calculate_rehearsal_priority(cast(Any, role)) == RehearsalPriority.HIGH - - -def test_calculate_priority_empty_role() -> None: - """Test missing role fields fall back to LOW priority.""" - role = {} - assert calculate_rehearsal_priority(cast(Any, role)) == RehearsalPriority.LOW - - -def test_calculate_priority_missing_confidence_level() -> None: - """Test missing confidence level falls through as LOW without other signals.""" - role = { - "confidence": {}, - "overlapWarnings": [], - "manualOverrides": [], - "setupNote": "", - } - assert calculate_rehearsal_priority(cast(Any, role)) == RehearsalPriority.LOW - - -def test_calculate_priority_multiple_medium_conditions() -> None: - """Test multiple medium signals still yield MEDIUM priority.""" - role = { - "confidence": {"level": "medium"}, - "overlapWarnings": [], - "manualOverrides": [], - "setupNote": "Some note", - "simplification": "Some simplification", - } - assert calculate_rehearsal_priority(cast(Any, role)) == RehearsalPriority.MEDIUM - - -def test_calculate_priority_high_overrides_medium() -> None: - """Test high priority signals override medium priority signals.""" - role = { - "confidence": {"level": "medium"}, - "overlapWarnings": ["Warning"], - "manualOverrides": [], - "setupNote": "Note", - } - assert calculate_rehearsal_priority(cast(Any, role)) == RehearsalPriority.HIGH diff --git a/services/analysis-engine/tests/test_extractor.py b/services/analysis-engine/tests/test_sections.py similarity index 74% rename from services/analysis-engine/tests/test_extractor.py rename to services/analysis-engine/tests/test_sections.py index 16fa180d..768bef8e 100644 --- a/services/analysis-engine/tests/test_extractor.py +++ b/services/analysis-engine/tests/test_sections.py @@ -1,6 +1,6 @@ """Tests for the section extraction logic and models.""" -from bandscope_analysis.sections.extractor import _normalize_label, extract_sections +from bandscope_analysis.sections.extractor import extract_sections from bandscope_analysis.sections.model import CueAnchorStrategy @@ -81,29 +81,3 @@ def test_extract_sections_unrecognized_label() -> None: assert sections[1]["id"] == "random part-1" assert sections[1]["form_label"] == "random part" assert sections[1]["confidence_level"] == "low" - - -def test_normalize_label() -> None: - """Verify standard label normalization logic.""" - assert _normalize_label("VERSE 1") == "verse" - assert _normalize_label(" chorus 2 ") == "chorus" - assert _normalize_label("pre-chorus") == "pre-chorus" - assert _normalize_label("UNKNOWN") == "unknown" - assert _normalize_label("intro") == "intro" - assert _normalize_label(123) == "123" - - -def test_extract_sections_empty() -> None: - """Verify behavior with an empty arrangement.""" - result = extract_sections([]) - assert result["strategy_used"] == "count" - assert len(result["sections"]) == 0 - - -def test_extract_sections_missing_label() -> None: - """Verify behavior when a section is missing the label key.""" - arrangement = [{"groove": "standard"}] - result = extract_sections(arrangement) - assert len(result["sections"]) == 1 - assert result["sections"][0]["form_label"] == "unknown" - assert result["sections"][0]["id"] == "unknown-1" diff --git a/services/analysis-engine/tests/test_separation.py b/services/analysis-engine/tests/test_separation.py index 81e27cc1..33dceff6 100644 --- a/services/analysis-engine/tests/test_separation.py +++ b/services/analysis-engine/tests/test_separation.py @@ -97,7 +97,7 @@ def test_stem_separator_deduplicates() -> None: def test_stem_separator_invalid_role() -> None: """Test separator handles non-dict roles gracefully.""" separator = StemSeparator() - result = separator.separate( # type: ignore[arg-type] + result = separator.separate( [{"id": "bass", "name": "Bass", "roleType": "instrument"}, "invalid"] ) assert len(result["stems"]) == 1 @@ -135,16 +135,6 @@ def test_stem_separator_keyboard_name_match() -> None: assert result["stems"][0]["category"] == "keys" -def test_stem_separator_missing_id() -> None: - """Test separator handles roles with missing id by generating a fallback id.""" - separator = StemSeparator() - roles = [{"name": "Lead Vocal", "roleType": "vocal"}] - result = separator.separate(roles) - assert len(result["stems"]) == 1 - assert result["stems"][0]["stem_id"] == "stem-role-0" - assert result["stems"][0]["label"] == "Lead Vocal" - - def test_audio_stem_separator_splits_local_audio_into_chunked_stems(tmp_path) -> None: """Ensure local audio is separated into downstream-consumable canonical stems.""" sample_rate = 8_000 diff --git a/services/analysis-engine/tests/test_supply_chain_policy.py b/services/analysis-engine/tests/test_supply_chain_policy.py index 0fdb5e6b..c471caac 100644 --- a/services/analysis-engine/tests/test_supply_chain_policy.py +++ b/services/analysis-engine/tests/test_supply_chain_policy.py @@ -6,36 +6,27 @@ import json import re import stat +import subprocess import zipfile from pathlib import Path import pytest from conftest import load_module - -def central_required_workflow_policy_text() -> str: - """Return the repository policy text that delegates review automation centrally.""" - repo_root = Path(__file__).resolve().parents[3] - return (repo_root / "docs" / "workflow" / "pr-review-merge-scheduler.md").read_text( - encoding="utf-8" - ) - - -def assert_local_review_workflows_removed() -> None: - """Ensure this repository does not carry local copies of central review workflows.""" - repo_root = Path(__file__).resolve().parents[3] - assert not (repo_root / ".github" / "workflows" / "opencode-review.yml").exists() - assert not (repo_root / ".github" / "workflows" / "pr-review-merge-scheduler.yml").exists() - for helper in ( - "classify_failed_check_evidence.py", - "collect_failed_check_evidence.sh", - "emit_opencode_failed_check_fallback_findings.sh", - "opencode_review_approve_gate.sh", - "opencode_review_normalize_output.py", - "pr_review_merge_scheduler.py", - "validate_opencode_failed_check_review.sh", - ): - assert not (repo_root / "scripts" / "ci" / helper).exists() +OPTIONAL_STRUCTURAL_REVIEW_PHRASES = ( + "structural exploration is not required", + "structural exploration not required", + "structural analysis is not required", + "structural analysis not required", + "structural review is not required", + "structural review not required", + "no structural exploration required", + "no structural analysis required", + "no structural review required", + "structural exploration is unnecessary", + "structural analysis is unnecessary", + "structural review is unnecessary", +) def test_supply_chain_check_requires_multi_arch_runner_labels( @@ -1242,14 +1233,13 @@ def test_supply_chain_check_accepts_repo_ossf_pr_code_scanning_upload() -> None: def test_opencode_review_declares_top_level_token_permissions() -> None: - """Ensure OpenCode token posture is delegated to the central required workflow.""" - policy = central_required_workflow_policy_text() + """Ensure OpenCode review keeps workflow-level GITHUB_TOKEN restrictions.""" + repo_root = Path(__file__).resolve().parents[3] + workflow = (repo_root / ".github" / "workflows" / "opencode-review.yml").read_text( + encoding="utf-8" + ) - assert_local_review_workflows_removed() - assert "ContextualWisdomLab/.github" in policy - assert "opencode-review" in policy - assert "repo-local copies" in policy - assert "token permissions" in policy + assert "\npermissions: read-all\n" in workflow def test_supply_chain_check_rejects_unnormalized_scorecard_sarif_upload( @@ -4930,66 +4920,575 @@ def test_supply_chain_check_accepts_repo_workspace_exec_policy( def test_opencode_review_gate_ignores_review_agent_status_contexts() -> None: - """Ensure peer-check handling is delegated to the central OpenCode workflow.""" - policy = central_required_workflow_policy_text() + """Ensure OpenCode ignores review agents while waiting on regular peer checks.""" + repo_root = Path(__file__).resolve().parents[3] + workflow = (repo_root / ".github" / "workflows" / "opencode-review.yml").read_text( + encoding="utf-8" + ) - assert_local_review_workflows_removed() - assert "peer-check waits" in policy - assert "review-agent status contexts" in policy - assert "failed-check explanation" in policy + assert "def opencode_review_agent_status:" in workflow + assert '$context == "coderabbit"' in workflow + assert '$context == "copilot pull request reviewer"' in workflow + assert "current_peer_checks_still_running" in workflow + assert 'select((.name // "") != "opencode-review")' in workflow + assert ( + 'select((.checkSuite.workflowRun.workflow.name // "") != "OpenCode PR Review")' in workflow + ) + assert ( + 'select((.state // "" | ascii_upcase) as $s | ["PENDING","EXPECTED"] | index($s))' + in workflow + ) + assert "No completed failed GitHub Checks were present" in workflow + assert workflow.count("select(opencode_review_agent_status | not)") >= 2 def test_opencode_review_unavailable_reports_provider_errors() -> None: - """Ensure provider failure reporting is a central OpenCode workflow responsibility.""" - policy = central_required_workflow_policy_text() + """Ensure unavailable OpenCode reviews explain provider failures in the overview.""" + repo_root = Path(__file__).resolve().parents[3] + workflow = (repo_root / ".github" / "workflows" / "opencode-review.yml").read_text( + encoding="utf-8" + ) - assert_local_review_workflows_removed() - assert "provider/runtime failures" in policy - assert "OpenCode runtime evidence" in policy + assert "summarize_opencode_review_failures" in workflow + assert "OpenCode runtime evidence:" in workflow + assert ".error.data.statusCode // empty" in workflow + assert ".error.data.message // .error.message // .error.name // empty" in workflow + assert ".error.data.metadata.url // empty" in workflow def test_opencode_approval_write_failure_updates_overview_only() -> None: - """Ensure approval write failures remain central automation evidence.""" - policy = central_required_workflow_policy_text() + """Ensure approval write failures are not reported as source findings.""" + repo_root = Path(__file__).resolve().parents[3] + workflow = (repo_root / ".github" / "workflows" / "opencode-review.yml").read_text( + encoding="utf-8" + ) - assert_local_review_workflows_removed() - assert "approval publication failures" in policy - assert "automation evidence, not" in policy - assert "source-backed repository findings" in policy + assert "create_approval_or_report_unavailable" in workflow + assert "APPROVAL_REVIEW_UNAVAILABLE" in workflow + assert "not a source-backed code finding" in workflow + assert 'create_approval_or_report_unavailable "$body"' in workflow -def test_pr_review_merge_scheduler_uses_central_mutation_credential() -> None: - """Ensure mechanical PR queue handling uses the central mutation credential.""" +def test_pr_review_merge_scheduler_uses_github_token_fallback() -> None: + """Ensure scheduled queue handling still runs when the app token secret is absent.""" repo_root = Path(__file__).resolve().parents[3] - policy = central_required_workflow_policy_text() + workflow = (repo_root / ".github" / "workflows" / "pr-review-merge-scheduler.yml").read_text( + encoding="utf-8" + ) + + assert "contents: write" in workflow + assert "issues: write" in workflow + assert "pull-requests: write" in workflow + assert "GH_TOKEN: ${{ secrets.OPENCODE_APPROVE_TOKEN || github.token }}" in workflow + assert "scheduler token source=github-token" in workflow + assert "Configure OPENCODE_APPROVE_TOKEN before running the scheduler" not in workflow + + +def test_opencode_classifies_artifact_upload_reset_as_external() -> None: + """Ensure transient artifact upload finalization resets do not request changes.""" + classifier = load_module( + "scripts/ci/classify_failed_check_evidence.py", + "classify_failed_check_evidence", + ) + evidence = """ +# Failed GitHub Check Evidence + +## Failed check: build-baseline/build / macos / amd64 + +### Failed job steps + +- step 13: Upload macOS amd64 artifact (failure) + +### Failed log excerpt + +```text +Finished `release` profile [optimized] target(s) in 6m 56s +Packaged BandScope_0.1.3_x64.dmg to artifacts/bandscope-macos-amd64.dmg +Run actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a +Finished uploading artifact content to blob storage! +Finalizing artifact upload +##[error]Failed to FinalizeArtifact: Unable to make request: ECONNRESET +``` +""".strip() + + result = classifier.classify_failed_check_evidence(evidence) + + assert result["classification"] == "external_infrastructure" + assert "rerun the failed workflow job" in result["reason"] + assert "build-baseline/build / macos / amd64" in result["signals"] + assert "Packaged .+ to artifacts/" not in result["signals"] + artifact_finalize_signals = [ + signal + for signal in result["signals"] + if "Failed to FinalizeArtifact: Unable to make request: ECONNRESET" in signal + ] + assert artifact_finalize_signals == [ + "artifact upload finalize request reset: " + "##[error]Failed to FinalizeArtifact: Unable to make request: ECONNRESET" + ] + assert any( + "Failed to FinalizeArtifact: Unable to make request: ECONNRESET" in signal + for signal in result["signals"] + ) + assert ( + "Packaged BandScope_0.1.3_x64.dmg to artifacts/bandscope-macos-amd64.dmg" + in result["signals"] + ) - opencode_config = (repo_root / "opencode.jsonc").read_text(encoding="utf-8") - assert '"openai/o3"' in opencode_config - assert '"openai/o4-mini"' in opencode_config - assert_local_review_workflows_removed() - assert "selected workflow mutation" in policy - assert "credential, not by a maintainer's local `gh` session" in policy - assert "PR_REVIEW_MERGE_TOKEN" in policy - assert "OPENCODE_APPROVE_TOKEN" in policy - assert "OpenCode GitHub App token" in policy - assert "workflow `GITHUB_TOKEN`" in policy - assert "update-branch, auto-merge, and merge actions" in policy + +def test_opencode_classifies_tauri_binary_release_502_as_external() -> None: + """Ensure Tauri binary release server errors do not request source changes.""" + classifier = load_module( + "scripts/ci/classify_failed_check_evidence.py", + "classify_failed_check_evidence_tauri_binary_release", + ) + evidence = """ +# Failed GitHub Check Evidence + +## Failed check: build-baseline/build / windows / amd64 + +### Failed job steps + +- step 12: Build native shell (failure) + +### Failed log excerpt + +```text +Finished `release` profile [optimized] target(s) in 4m 53s +Built application at: D:\\a\\bandscope\\target\\release\\bandscope-desktop.exe +Downloading https://github.com/tauri-apps/binary-releases/releases/download/nsis-3.11/nsis-3.11.zip +failed to bundle project `http status: 502` +Error failed to bundle project `http status: 502` +``` +""".strip() + + result = classifier.classify_failed_check_evidence(evidence) + + assert result["classification"] == "external_infrastructure" + assert "Tauri binary release download server error" in result["reason"] + assert "build-baseline/build / windows / amd64" in result["signals"] + assert any("tauri-apps/binary-releases" in signal for signal in result["signals"]) + assert any( + "failed to bundle project `http status: 502`" in signal for signal in result["signals"] + ) + + +def test_opencode_classifies_setup_uv_manifest_fetch_as_external() -> None: + """Ensure setup-uv manifest fetch failures do not request source changes.""" + classifier = load_module( + "scripts/ci/classify_failed_check_evidence.py", + "classify_failed_check_evidence_setup_uv_fetch", + ) + evidence = """ +# Failed GitHub Check Evidence + +## Failed check: build-baseline/build / macos / amd64 + +### Failed job steps + +- step 5: Run astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 (failure) + +### Failed log excerpt + +```text +Fetching manifest data from https://raw.githubusercontent.com/astral-sh/versions/ +##[error]fetch failed +``` +""".strip() + + result = classifier.classify_failed_check_evidence(evidence) + + assert result["classification"] == "external_infrastructure" + assert "setup-uv manifest fetch failure" in result["reason"] + assert "build-baseline/build / macos / amd64" in result["signals"] + assert any("##[error]fetch failed" in signal for signal in result["signals"]) + assert any( + "raw.githubusercontent.com/astral-sh/versions" in signal for signal in result["signals"] + ) + + +def test_opencode_keeps_test_failures_actionable() -> None: + """Ensure ordinary failed checks still require source-backed diagnosis.""" + classifier = load_module( + "scripts/ci/classify_failed_check_evidence.py", + "classify_failed_check_evidence_actionable", + ) + evidence = """ +# Failed GitHub Check Evidence + +## Failed check: ci/ci / build-and-test + +### Failed job steps + +- step 7: Run tests (failure) + +### Failed log excerpt + +```text +FAIL apps/desktop/src/App.test.tsx +##[error]Process completed with exit code 1. +``` +""".strip() + + result = classifier.classify_failed_check_evidence(evidence) + + assert result["classification"] == "actionable_or_unknown" def test_opencode_review_stops_external_check_failures_without_review() -> None: - """Ensure external check failure handling is delegated to central review automation.""" - policy = central_required_workflow_policy_text() + """Ensure external check failures update overview instead of review state.""" + repo_root = Path(__file__).resolve().parents[3] + workflow = (repo_root / ".github" / "workflows" / "opencode-review.yml").read_text( + encoding="utf-8" + ) - assert_local_review_workflows_removed() - assert "external failed-check classification" in policy - assert "review state" in policy - assert "current-head evidence" in policy + assert "scripts/ci/classify_failed_check_evidence.py" in workflow + assert "stop_for_external_failed_check_if_needed" in workflow + assert 'stop_approval_without_review "EXTERNAL_CHECK_FAILURE"' in workflow + assert 'map(tostring | ltrimstr("- ") | "- " + .)' in workflow + assert 'if [ "$gate_status" -ne 0 ]; then' in workflow + assert "python3 scripts/ci/opencode_review_normalize_output.py" in workflow + assert '"$HEAD_SHA" "$RUN_ID" "$RUN_ATTEMPT" "$clean_output"' in workflow + assert 'if ! classification="$(' in workflow + assert "jq -r '.classification // empty' \"$classification_file\" 2>/dev/null" in workflow + + +def test_opencode_normalizer_defaults_missing_approve_findings(tmp_path: Path) -> None: + """Ensure APPROVE control payloads without findings normalize to findings:[].""" + normalizer = load_module( + "scripts/ci/opencode_review_normalize_output.py", + "opencode_review_normalize_output", + ) + output_file = tmp_path / "opencode-output.md" + output_file.write_text( + "\n".join( + [ + "review text", + '{"head_sha":"abc123","run_id":"456","run_attempt":"1",' + '"result":"APPROVE","reason":"checks and review passed",' + '"summary":"no source-backed blockers found"}', + ] + ), + encoding="utf-8", + ) + + result = normalizer.main( + [ + "opencode_review_normalize_output.py", + "abc123", + "456", + "1", + str(output_file), + ] + ) + + assert result == 0 + assert '"findings":[]' in output_file.read_text(encoding="utf-8") + + +def test_opencode_review_gate_defaults_missing_approve_findings(tmp_path: Path) -> None: + """Ensure approval gate accepts APPROVE payloads that omit empty findings.""" + repo_root = Path(__file__).resolve().parents[3] + comment_file = tmp_path / "comment.md" + normalized_file = tmp_path / "normalized.json" + comment_file.write_text( + "\n".join( + [ + "", + "", + "", + "", + ] + ), + encoding="utf-8", + ) + + result = subprocess.run( + [ + "bash", + str(repo_root / "scripts" / "ci" / "opencode_review_approve_gate.sh"), + "abc123", + "456", + "1", + str(comment_file), + str(normalized_file), + ], + cwd=repo_root, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0, result.stderr + assert result.stdout.strip() == "APPROVE" + assert json.loads(normalized_file.read_text(encoding="utf-8"))["findings"] == [] + + +def test_opencode_normalizer_rejects_approve_without_structural_review( + tmp_path: Path, +) -> None: + """Ensure OpenCode cannot approve after admitting structural review failed.""" + normalizer = load_module( + "scripts/ci/opencode_review_normalize_output.py", + "opencode_review_normalize_missing_structure", + ) + output_file = tmp_path / "opencode-output.md" + original_output = "\n".join( + [ + "review text", + '{"head_sha":"abc123","run_id":"456","run_attempt":"1",' + '"result":"APPROVE","reason":"no blockers found",' + '"summary":"No blockers found, but evidence was truncated",' + '"findings":[]}', + ] + ) + output_file.write_text(original_output, encoding="utf-8") + + result = normalizer.main( + [ + "opencode_review_normalize_output.py", + "abc123", + "456", + "1", + str(output_file), + ] + ) + + assert result == 4 + assert output_file.read_text(encoding="utf-8") == original_output + + +def test_opencode_normalizer_rejects_optional_structural_review_variants( + tmp_path: Path, +) -> None: + """Ensure optional structural-review phrasing cannot be normalized.""" + normalizer = load_module( + "scripts/ci/opencode_review_normalize_output.py", + "opencode_review_normalize_optional_structure", + ) + + assert set(OPTIONAL_STRUCTURAL_REVIEW_PHRASES).issubset(normalizer.STRUCTURAL_FAILURE_PHRASES) + + for field in ("reason", "summary"): + for phrase in OPTIONAL_STRUCTURAL_REVIEW_PHRASES: + output_file = tmp_path / f"{field}-{phrase.replace(' ', '-')}.md" + reason = phrase if field == "reason" else "no blockers found" + summary = phrase if field == "summary" else "structural exploration completed" + original_output = "\n".join( + [ + "review text", + '{"head_sha":"abc123","run_id":"456","run_attempt":"1",' + '"result":"APPROVE",' + f'"reason":"{reason}",' + f'"summary":"{summary}",' + '"findings":[]}', + ] + ) + output_file.write_text(original_output, encoding="utf-8") + + result = normalizer.main( + [ + "opencode_review_normalize_output.py", + "abc123", + "456", + "1", + str(output_file), + ] + ) + + assert result == 4 + assert output_file.read_text(encoding="utf-8") == original_output + + +def test_opencode_review_gate_rejects_approve_without_structural_review( + tmp_path: Path, +) -> None: + """Ensure approval gate rejects approvals that admit missing structure.""" + repo_root = Path(__file__).resolve().parents[3] + comment_file = tmp_path / "comment.md" + normalized_file = tmp_path / "normalized.json" + comment_file.write_text( + "\n".join( + [ + "", + "", + "", + "", + ] + ), + encoding="utf-8", + ) + + result = subprocess.run( + [ + "bash", + str(repo_root / "scripts" / "ci" / "opencode_review_approve_gate.sh"), + "abc123", + "456", + "1", + str(comment_file), + str(normalized_file), + ], + cwd=repo_root, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 4 + assert result.stdout.strip() == "NO_CONCLUSION" + assert not normalized_file.exists() + + +def test_opencode_review_gate_rejects_optional_structural_review_variants( + tmp_path: Path, +) -> None: + """Ensure approval gate rejects optional structural-review phrasing.""" + repo_root = Path(__file__).resolve().parents[3] + + for field in ("reason", "summary"): + for phrase in OPTIONAL_STRUCTURAL_REVIEW_PHRASES: + comment_file = tmp_path / f"{field}-{phrase.replace(' ', '-')}.md" + normalized_file = tmp_path / f"{field}-{phrase.replace(' ', '-')}.json" + reason = phrase if field == "reason" else "no blockers found" + summary = phrase if field == "summary" else "structural exploration completed" + comment_file.write_text( + "\n".join( + [ + "", + "", + "", + "", + ] + ), + encoding="utf-8", + ) + + result = subprocess.run( + [ + "bash", + str(repo_root / "scripts" / "ci" / "opencode_review_approve_gate.sh"), + "abc123", + "456", + "1", + str(comment_file), + str(normalized_file), + ], + cwd=repo_root, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 4 + assert result.stdout.strip() == "NO_CONCLUSION" + assert not normalized_file.exists() + + +def test_opencode_normalizer_accepts_completed_local_structural_fallback( + tmp_path: Path, +) -> None: + """Ensure normalizer accepts tool fallback when structural review completed.""" + normalizer = load_module( + "scripts/ci/opencode_review_normalize_output.py", + "opencode_review_normalize_structural_fallback", + ) + output_file = tmp_path / "opencode-output.md" + output_file.write_text( + "\n".join( + [ + "review text", + '{"head_sha":"abc123","run_id":"456","run_attempt":"1",' + '"result":"APPROVE","reason":"no blockers found",' + '"summary":"Could not access CodeGraph; performed focused local ' + 'source/diff inspection and completed structural exploration",' + '"findings":[]}', + ] + ), + encoding="utf-8", + ) + + result = normalizer.main( + [ + "opencode_review_normalize_output.py", + "abc123", + "456", + "1", + str(output_file), + ] + ) + + assert result == 0 + assert '"findings":[]' in output_file.read_text(encoding="utf-8") + + +def test_opencode_review_gate_accepts_completed_local_structural_fallback( + tmp_path: Path, +) -> None: + """Ensure tool access failures do not block approvals after local structure review.""" + repo_root = Path(__file__).resolve().parents[3] + comment_file = tmp_path / "comment.md" + normalized_file = tmp_path / "normalized.json" + comment_file.write_text( + "\n".join( + [ + "", + "", + "", + "", + ] + ), + encoding="utf-8", + ) + + result = subprocess.run( + [ + "bash", + str(repo_root / "scripts" / "ci" / "opencode_review_approve_gate.sh"), + "abc123", + "456", + "1", + str(comment_file), + str(normalized_file), + ], + cwd=repo_root, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0, result.stderr + assert result.stdout.strip() == "APPROVE" + assert json.loads(normalized_file.read_text(encoding="utf-8"))["findings"] == [] def test_opencode_strix_lookup_reports_missing_actions_read_scope() -> None: - """Ensure Strix lookup token-scope diagnostics stay in central workflow policy.""" - policy = central_required_workflow_policy_text() + """Ensure Strix lookup token-scope failures are diagnosable.""" + repo_root = Path(__file__).resolve().parents[3] + workflow = (repo_root / ".github" / "workflows" / "opencode-review.yml").read_text( + encoding="utf-8" + ) - assert_local_review_workflows_removed() - assert "Strix evidence lookup" in policy - assert "Actions read access" in policy + assert "HTTP 403|forbidden|resource not accessible" in workflow + assert "requires Actions read access" in workflow diff --git a/services/analysis-engine/tests/test_youtube.py b/services/analysis-engine/tests/test_youtube.py index 71d7d60d..bac281a2 100644 --- a/services/analysis-engine/tests/test_youtube.py +++ b/services/analysis-engine/tests/test_youtube.py @@ -151,10 +151,14 @@ def exists_side_effect(path: str) -> bool: mock_exists.side_effect = exists_side_effect mock_getsize.return_value = 10 * 1024 * 1024 - result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") + with patch("bandscope_analysis.youtube.glob.iglob") as mock_iglob: + mock_iglob.return_value = iter(["/tmp/abc123DEF45.opus"]) + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") assert result["ok"] is True assert result["metadata"]["filepath"] == "/tmp/abc123DEF45.opus" + mock_iglob.assert_called_once_with("/tmp/abc123DEF45.*") @patch("bandscope_analysis.youtube.os.path.exists") @@ -176,7 +180,10 @@ def test_download_youtube_audio_file_not_found( mock_ydl.prepare_filename.return_value = "/tmp/abc123DEF45.webm" mock_exists.return_value = False - result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") + with patch("bandscope_analysis.youtube.glob.iglob") as mock_iglob: + mock_iglob.return_value = iter(()) + + result = download_youtube_audio("https://youtube.com/watch?v=abc123DEF45", "/tmp") assert result["ok"] is False assert result["error"]["code"] == "file_not_found"