mirror of
https://github.com/nexu-io/open-design.git
synced 2026-10-27 09:51:12 +01:00
* feat(tools-pr): add maintainer PR-duty workspace
Adds `tools/pr` as the maintainer-only control plane for PR-duty work on
this repo. Thin `gh` wrapper that encodes repo-specific knowledge:
review lanes, forbidden surfaces, lane-specific checklists, validation
command derivation from touched packages.
Subcommands:
- `list` — triage open queue by lane and review-state bucket.
- `view <num>` — agent-friendly review brief for a single PR.
- `classify [num]` — emit script-level tags for one PR or the whole
open queue; full-queue JSON output lands under `.tmp/tools-pr/classify/`
with rate-limit telemetry per run.
- `assignment` — assigner-perspective view of PR ownership, idle time,
and blockers (derived from existing tags; no new judgments).
Tag dictionary (13 tags) covers: bot-only-approval, needs-rebase,
forbidden-surface, unlabeled, duplicate-title, non-ascii-slug,
maintainer-edits-disabled, org-member, unresolved-changes-requested,
stale-approval, and three awaiting-* timing tags. Each rule is
expressible as one factual sentence over `gh` data + repo paths — see
`tools/pr/AGENTS.md` for the full dictionary plus precision rules.
Templates in `tools/pr/templates/*.md` are aesthetic references for
recurring maintainer comments (duplicate-title ask, awaiting-author
nudge, agent-review brief shape). `templates/examples/` holds
frozen-in-time agent-review snapshots for three PR shapes.
Infrastructure:
- `gh()` wraps `execFile` with minimum-touch retry (2 attempts at 1s + 2s
backoff) on transient 5xx / network errors. Persistent failures still
surface — retry is anti-jitter, not an exponential-backoff resilience
layer.
- Heavy chunks (`reviews`, `comments`, `commits`, assignment timelines)
use cursor-paginated `gh api graphql` via `fetchPaginatedPrList` to
stay under GitHub's GraphQL server-side timeout. Light chunks stay on
`gh pr list --json`.
- `fetchOrgMembers` cached per process via `gh api orgs/<owner>/members
--paginate`.
Wiring:
- Root `package.json` adds `pnpm tools-pr` to the allowed root entry
points.
- `scripts/postinstall.mjs` builds `tools/pr` alongside other workspace
packages.
- `scripts/guard.ts` allowlists `tools/pr/bin/tools-pr.mjs` and
`tools/pr/esbuild.config.mjs`, and adds `pr/` to the `tools/` top-level
layout allowlist.
- Root `AGENTS.md` and `tools/AGENTS.md` document the new command
surface, root-command-boundary update, and per-tool ownership.
* docs(agents): brief tools-pr in root AGENTS.md, link to tools/pr/AGENTS.md
Adds a `PR-duty tooling` section to the root AGENTS.md summarising what
`pnpm tools-pr` is, listing the four common subcommands (list / view /
classify / assignment), and pointing readers to `tools/pr/AGENTS.md` for
the full tag dictionary, operational playbook, templates, and design
rules. The section keeps root-level guidance to high-level orientation
while details stay local to the tool's own AGENTS.md.
* fix(tools-pr): drop overly broad touches-root-package.json forbidden hit
`deriveForbidden` was flagging any change to root `package.json` as a
forbidden-surface hit, but AGENTS.md §Root command boundary only forbids
specific *lifecycle* aliases (pnpm dev / test / build / daemon / preview
/ start) — tools-control-plane entrypoints like `pnpm tools-pr` are
explicitly allowed. Distinguishing "forbidden alias" from "allowed
entry" requires reading the diff content, which is `pnpm guard`'s job
rather than a path-derived classify tag.
Dogfooded on this branch's own PR (#1259), which added the `pnpm
tools-pr` script and was incorrectly flagged. Removing the hit aligns
the `forbidden-surface` tag with what tools-pr can mechanically detect
from file paths alone (apps/nextjs/, packages/shared/).
* fix(tools-pr): paginate commits fetch, recognise ready-to-merge, escape title-index separator
Three review follow-ups on #1259, all factual fixes:
- `fetchOpenPrCommits` now uses `fetchPaginatedPrList` instead of a
one-shot `pullRequests(first: $first)` query. GitHub GraphQL caps
connection page size at 100, so the previous implementation would
fail at runtime when callers passed `--limit > 100`. The paginated
path makes the commits fetch consistent with the other heavy chunks
(reviews, comments, assignment timelines) and removes the artificial
ceiling entirely. The `limit` parameter is dropped from
`fetchOpenPrCommits`; the CLI `--limit` continues to bound the
`gh pr list --json` chunks.
- `deriveStatus` in `assignment.ts` now reads `facts.reviewDecision`
and `facts.mergeStateStatus`. When the PR is `APPROVED` with merge
state `CLEAN` or `UNSTABLE` and carries no blockers, status renders
as `ready to merge` instead of falling through to `in review`. The
assignment view loses its main triage signal without this — a clean
human-approved PR rendered identical to a REVIEW_REQUIRED one.
- `tags.ts:tagDuplicateTitle` and `tags.ts:buildContext` both
constructed the title-index key with a literal NUL byte between
author and title, which made the file appear as binary in `git diff`
/ review tooling. Replaced the literal byte with a Unicode escape
sequence in source; the runtime string value is identical, the
source stays plain text and round-trips through review tooling
cleanly.
* fix(tools-pr): raise default --limit to 1000 to cover the live open queue
mrcfps flagged that `tools-pr list` (and `classify --all`, `assignment`)
defaults to `--limit 100`, which silently drops every PR past the first
100 in the open queue. The repo currently sits at 104 open PRs, so the
out-of-the-box run was already omitting four PRs.
Raise the default to 1000 in `list.ts`, `classify.ts`, and `assignment.ts`,
and remove the now-pointless 200 ceiling — `gh pr list --limit N` paginates
internally, so a high cap is cheap. Users can still pass `--limit <small>`
for a truncated preview. CLI help text on the three subcommands updated to
match.
* fix(web): pass designTemplates to ProjectView render helper
#955 made `designTemplates` a required Prop on ProjectView, but the test
helper added in #1244 (`renderProjectView` in
`ProjectView.api-empty-response.test.tsx`) was never updated. The two
PRs landed on main without conflicting, leaving `apps/web` typecheck red
for every PR that rebases past b5eb8c16.
Pass `designTemplates={[] as SkillSummary[]}` alongside the existing
`skills={[] as SkillSummary[]}` so the helper compiles. The component
already treats the array shape (empty included) as a no-op fallback in
the empty-response paths the test exercises.
* fix(tools-pr): correct author signal + merge inline review comments
Two correctness gaps in the awaiting-* signal pipeline surfaced during
review of the new tools-pr commands:
1. `authorSignalAt` iterated every PR commit unconditionally. On
`maintainerCanModify=true` PRs a maintainer's follow-up push would
advance the author timestamp, masking a stalled author response.
Filter commits to those whose `authorLogin` matches `facts.author`,
mirroring the same filter already applied to comments.
2. `fetchOpenPrComments` (and `fetchView`) only fetched
`pullRequest.comments` / `gh pr view --json comments`, which is the
issue-conversation thread. Inline review-thread replies — where
authors and reviewers actually exchange most fix-up replies — live in
`reviewThreads.comments` / REST `pulls/{n}/comments`. Missing them let
`humanReviewerSignalAt` / `authorSignalAt` and the `view` brief point
at the wrong side after someone replied inline. Extend the list-mode
GraphQL to also sweep `reviewThreads(last: 20).comments(first: 20)`,
and add a parallel REST inline-comments fetch in `fetchView` that
merges into `GhView.comments`.
352 lines
11 KiB
TypeScript
352 lines
11 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
|
|
import type { ReactNode } from 'react';
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
|
|
|
import { ProjectView } from '../../src/components/ProjectView';
|
|
import { streamMessage } from '../../src/providers/anthropic';
|
|
import type { StreamHandlers } from '../../src/providers/anthropic';
|
|
import { patchPreviewCommentStatus, writeProjectTextFile } from '../../src/providers/registry';
|
|
import { listMessages, saveMessage } from '../../src/state/projects';
|
|
import type {
|
|
AgentEvent,
|
|
AgentInfo,
|
|
AppConfig,
|
|
ChatAttachment,
|
|
ChatCommentAttachment,
|
|
ChatMessage,
|
|
Conversation,
|
|
DesignSystemSummary,
|
|
Project,
|
|
SkillSummary,
|
|
} from '../../src/types';
|
|
|
|
const chatPaneMockState = vi.hoisted(() => ({
|
|
commentAttachments: [] as ChatCommentAttachment[],
|
|
}));
|
|
|
|
vi.mock('../../src/router', () => ({
|
|
navigate: vi.fn(),
|
|
}));
|
|
|
|
vi.mock('../../src/providers/anthropic', () => ({
|
|
streamMessage: vi.fn(),
|
|
}));
|
|
|
|
vi.mock('../../src/providers/daemon', () => ({
|
|
fetchChatRunStatus: vi.fn(),
|
|
listActiveChatRuns: vi.fn().mockResolvedValue([]),
|
|
reattachDaemonRun: vi.fn(),
|
|
streamViaDaemon: vi.fn(),
|
|
}));
|
|
|
|
vi.mock('../../src/providers/project-events', () => ({
|
|
useProjectFileEvents: vi.fn(),
|
|
}));
|
|
|
|
vi.mock('../../src/providers/registry', async () => {
|
|
const actual = await vi.importActual<typeof import('../../src/providers/registry')>(
|
|
'../../src/providers/registry',
|
|
);
|
|
return {
|
|
...actual,
|
|
deletePreviewComment: vi.fn(),
|
|
fetchDesignSystem: vi.fn().mockResolvedValue(null),
|
|
fetchLiveArtifacts: vi.fn().mockResolvedValue([]),
|
|
fetchPreviewComments: vi.fn().mockResolvedValue([]),
|
|
fetchProjectFiles: vi.fn().mockResolvedValue([]),
|
|
fetchSkill: vi.fn().mockResolvedValue(null),
|
|
patchPreviewCommentStatus: vi.fn(),
|
|
upsertPreviewComment: vi.fn(),
|
|
writeProjectTextFile: vi.fn(),
|
|
};
|
|
});
|
|
|
|
vi.mock('../../src/state/projects', async () => {
|
|
const actual = await vi.importActual<typeof import('../../src/state/projects')>(
|
|
'../../src/state/projects',
|
|
);
|
|
const mockConversation = (projectId: string): Conversation => ({
|
|
id: `conv-${projectId}`,
|
|
projectId,
|
|
title: null,
|
|
createdAt: 1,
|
|
updatedAt: 1,
|
|
});
|
|
return {
|
|
...actual,
|
|
createConversation: vi.fn().mockImplementation(async (projectId: string) => mockConversation(projectId)),
|
|
deleteConversation: vi.fn(),
|
|
getTemplate: vi.fn().mockResolvedValue(null),
|
|
listConversations: vi.fn().mockImplementation(async (projectId: string) => [mockConversation(projectId)]),
|
|
listMessages: vi.fn().mockResolvedValue([]),
|
|
loadTabs: vi.fn().mockResolvedValue({ tabs: [], active: null }),
|
|
patchConversation: vi.fn(),
|
|
patchProject: vi.fn(),
|
|
saveMessage: vi.fn(),
|
|
saveTabs: vi.fn(),
|
|
};
|
|
});
|
|
|
|
vi.mock('../../src/components/AppChromeHeader', () => ({
|
|
AppChromeHeader: ({ children }: { children: ReactNode }) => <header>{children}</header>,
|
|
}));
|
|
|
|
vi.mock('../../src/components/AvatarMenu', () => ({
|
|
AvatarMenu: () => null,
|
|
}));
|
|
|
|
vi.mock('../../src/components/FileWorkspace', () => ({
|
|
FileWorkspace: () => <div data-testid="file-workspace" />,
|
|
}));
|
|
|
|
vi.mock('../../src/components/Loading', () => ({
|
|
CenteredLoader: () => <div data-testid="loader" />,
|
|
}));
|
|
|
|
vi.mock('../../src/components/ChatPane', () => ({
|
|
ChatPane: ({
|
|
messages,
|
|
onSend,
|
|
}: {
|
|
messages: ChatMessage[];
|
|
onSend: (
|
|
prompt: string,
|
|
attachments: ChatAttachment[],
|
|
commentAttachments: ChatCommentAttachment[],
|
|
) => void;
|
|
}) => (
|
|
<div>
|
|
<button type="button" onClick={() => onSend('Create a login page', [], chatPaneMockState.commentAttachments)}>
|
|
send
|
|
</button>
|
|
{messages.map((message) => (
|
|
<article key={message.id} data-testid={`message-${message.role}`}>
|
|
<span>{message.content}</span>
|
|
<span>{message.runStatus ?? 'no-run-status'}</span>
|
|
{(message.events ?? []).map((event, index) => (
|
|
<span key={index}>
|
|
{event.kind === 'status' ? `${event.label}:${event.detail ?? ''}` : ''}
|
|
{event.kind === 'text' ? event.text : ''}
|
|
</span>
|
|
))}
|
|
</article>
|
|
))}
|
|
</div>
|
|
),
|
|
}));
|
|
|
|
const mockedStreamMessage = vi.mocked(streamMessage);
|
|
const mockedListMessages = vi.mocked(listMessages);
|
|
const mockedSaveMessage = vi.mocked(saveMessage);
|
|
const mockedWriteProjectTextFile = vi.mocked(writeProjectTextFile);
|
|
const mockedPatchPreviewCommentStatus = vi.mocked(patchPreviewCommentStatus);
|
|
|
|
const config: AppConfig = {
|
|
mode: 'api',
|
|
apiProtocol: 'openai',
|
|
apiKey: 'sk-test',
|
|
baseUrl: 'https://api.deepseek.com',
|
|
model: 'deepseek-chat',
|
|
agentId: null,
|
|
skillId: null,
|
|
designSystemId: null,
|
|
};
|
|
|
|
const project: Project = {
|
|
id: 'project-1',
|
|
name: 'Project',
|
|
skillId: null,
|
|
designSystemId: null,
|
|
createdAt: 1,
|
|
updatedAt: 1,
|
|
};
|
|
|
|
function renderProjectView() {
|
|
return render(
|
|
<ProjectView
|
|
project={project}
|
|
routeFileName={null}
|
|
config={config}
|
|
agents={[] as AgentInfo[]}
|
|
skills={[] as SkillSummary[]}
|
|
designTemplates={[] as SkillSummary[]}
|
|
designSystems={[] as DesignSystemSummary[]}
|
|
daemonLive
|
|
onModeChange={vi.fn()}
|
|
onAgentChange={vi.fn()}
|
|
onAgentModelChange={vi.fn()}
|
|
onRefreshAgents={vi.fn()}
|
|
onOpenSettings={vi.fn()}
|
|
onBack={vi.fn()}
|
|
onClearPendingPrompt={vi.fn()}
|
|
onTouchProject={vi.fn()}
|
|
onProjectChange={vi.fn()}
|
|
onProjectsRefresh={vi.fn()}
|
|
/>,
|
|
);
|
|
}
|
|
|
|
describe('ProjectView API empty response handling', () => {
|
|
beforeEach(() => {
|
|
chatPaneMockState.commentAttachments = [];
|
|
mockedStreamMessage.mockReset();
|
|
mockedListMessages.mockClear();
|
|
mockedSaveMessage.mockClear();
|
|
mockedWriteProjectTextFile.mockClear();
|
|
mockedPatchPreviewCommentStatus.mockClear();
|
|
});
|
|
|
|
afterEach(() => {
|
|
cleanup();
|
|
vi.clearAllMocks();
|
|
});
|
|
|
|
it('marks an empty API completion as a soft no-output state instead of succeeded', async () => {
|
|
mockedStreamMessage.mockImplementation(async (
|
|
_cfg: AppConfig,
|
|
_system: string,
|
|
_history: ChatMessage[],
|
|
_signal: AbortSignal,
|
|
handlers: StreamHandlers,
|
|
) => {
|
|
handlers.onDone('');
|
|
});
|
|
renderProjectView();
|
|
|
|
await sendTestPrompt();
|
|
|
|
await waitFor(() => {
|
|
expect(screen.getByText('empty_response:deepseek-chat')).toBeTruthy();
|
|
});
|
|
expect(screen.getByText(/provider ended the request/i)).toBeTruthy();
|
|
expect(screen.queryByText('succeeded')).toBeNull();
|
|
|
|
await waitFor(() => {
|
|
expect(
|
|
mockedSaveMessage.mock.calls.some((call) => {
|
|
const message = call[2] as ChatMessage;
|
|
return (
|
|
message.role === 'assistant' &&
|
|
message.runStatus === undefined &&
|
|
message.events?.some(
|
|
(event: AgentEvent) => event.kind === 'status' && event.label === 'empty_response',
|
|
)
|
|
);
|
|
}),
|
|
).toBe(true);
|
|
});
|
|
});
|
|
|
|
it('marks attached saved comments as failed when an API completion has no output', async () => {
|
|
chatPaneMockState.commentAttachments = [
|
|
{
|
|
id: 'comment-1',
|
|
order: 1,
|
|
filePath: 'index.html',
|
|
elementId: 'hero-title',
|
|
selector: '#hero-title',
|
|
label: 'Hero title',
|
|
comment: 'Make this clearer',
|
|
currentText: 'Old title',
|
|
pagePosition: { x: 0, y: 0, width: 100, height: 24 },
|
|
htmlHint: '<h1 id="hero-title">Old title</h1>',
|
|
source: 'saved-comment',
|
|
},
|
|
];
|
|
mockedStreamMessage.mockImplementation(async (
|
|
_cfg: AppConfig,
|
|
_system: string,
|
|
_history: ChatMessage[],
|
|
_signal: AbortSignal,
|
|
handlers: StreamHandlers,
|
|
) => {
|
|
handlers.onDone('');
|
|
});
|
|
renderProjectView();
|
|
|
|
await sendTestPrompt();
|
|
|
|
await waitFor(() => {
|
|
expect(mockedPatchPreviewCommentStatus).toHaveBeenCalledWith(
|
|
project.id,
|
|
'conv-project-1',
|
|
'comment-1',
|
|
'failed',
|
|
);
|
|
});
|
|
await waitFor(() => {
|
|
expect(hasSavedAssistantMessage((message) => (
|
|
message.runStatus === undefined &&
|
|
message.events?.some((event) => event.kind === 'status' && event.label === 'empty_response') === true
|
|
))).toBe(true);
|
|
});
|
|
});
|
|
|
|
it('keeps normal API text completions on the succeeded path', async () => {
|
|
mockedStreamMessage.mockImplementation(async (
|
|
_cfg: AppConfig,
|
|
_system: string,
|
|
_history: ChatMessage[],
|
|
_signal: AbortSignal,
|
|
handlers: StreamHandlers,
|
|
) => {
|
|
handlers.onDelta('hello');
|
|
handlers.onDone('hello');
|
|
});
|
|
renderProjectView();
|
|
|
|
await sendTestPrompt();
|
|
|
|
await waitFor(() => expect(screen.getAllByText('hello').length).toBeGreaterThan(0));
|
|
await waitFor(() => {
|
|
expect(hasSavedAssistantMessage((message) => message.runStatus === 'succeeded')).toBe(true);
|
|
});
|
|
expect(screen.queryByText(/provider ended the request/i)).toBeNull();
|
|
});
|
|
|
|
it('keeps API artifact completions on the succeeded path even when done text is empty', async () => {
|
|
const artifact =
|
|
'<artifact identifier="landing-page" type="text/html" title="Landing Page">' +
|
|
'<!doctype html><html><head><title>Landing</title></head><body><main><h1>Landing page</h1><p>Generated design artifact with enough structure to persist.</p></main></body></html>' +
|
|
'</artifact>';
|
|
mockedStreamMessage.mockImplementation(async (
|
|
_cfg: AppConfig,
|
|
_system: string,
|
|
_history: ChatMessage[],
|
|
_signal: AbortSignal,
|
|
handlers: StreamHandlers,
|
|
) => {
|
|
handlers.onDelta(artifact);
|
|
handlers.onDone('');
|
|
});
|
|
renderProjectView();
|
|
|
|
await sendTestPrompt();
|
|
|
|
await waitFor(() => {
|
|
expect(hasSavedAssistantMessage((message) => message.runStatus === 'succeeded')).toBe(true);
|
|
});
|
|
await waitFor(() => expect(mockedWriteProjectTextFile).toHaveBeenCalled());
|
|
expect(screen.queryByText(/provider ended the request/i)).toBeNull();
|
|
expect(screen.queryByText('empty_response:deepseek-chat')).toBeNull();
|
|
});
|
|
});
|
|
|
|
async function sendTestPrompt() {
|
|
await waitFor(() => {
|
|
expect(mockedListMessages).toHaveBeenCalledWith(project.id, 'conv-project-1');
|
|
});
|
|
await new Promise((resolve) => setTimeout(resolve, 0));
|
|
await waitFor(() => expect(screen.getByRole('button', { name: 'send' })).toBeTruthy());
|
|
fireEvent.click(screen.getByRole('button', { name: 'send' }));
|
|
}
|
|
|
|
function hasSavedAssistantMessage(predicate: (message: ChatMessage) => boolean): boolean {
|
|
return mockedSaveMessage.mock.calls.some((call) => {
|
|
const message = call[2] as ChatMessage;
|
|
return message.role === 'assistant' && predicate(message);
|
|
});
|
|
}
|