Files
open-design/apps/web/tests/components/ProjectView.api-empty-response.test.tsx
PerishFireandGitHub 8c0fb8dc01 feat(tools-pr): add maintainer PR-duty workspace (#1259)
* 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`.
2026-05-11 19:17:21 +08:00

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);
});
}