mirror of
https://github.com/nexu-io/open-design.git
synced 2026-05-31 19:04:39 +07:00
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`.
This commit is contained in:
PerishFire
GitHub
parent
b5eb8c1647
commit
8c0fb8dc01
28 changed files with 3504 additions and 3 deletions
27
AGENTS.md
27
AGENTS.md
|
|
@ -22,6 +22,7 @@ This file is the single source of truth for agents entering this repository. Rea
|
||||||
- `packages/sidecar-proto` owns the Open Design sidecar business protocol; `packages/sidecar` owns the generic sidecar runtime; `packages/platform` owns generic OS process primitives.
|
- `packages/sidecar-proto` owns the Open Design sidecar business protocol; `packages/sidecar` owns the generic sidecar runtime; `packages/platform` owns generic OS process primitives.
|
||||||
- `tools/dev` is the local development lifecycle control plane.
|
- `tools/dev` is the local development lifecycle control plane.
|
||||||
- `tools/pack` is the local packaged build/start/stop/logs control plane and mac beta release artifact preparation surface.
|
- `tools/pack` is the local packaged build/start/stop/logs control plane and mac beta release artifact preparation surface.
|
||||||
|
- `tools/pr` is the maintainer PR-duty control plane: a thin `gh` wrapper that encodes this repo's review-lane derivation, forbidden-surface flags, lane checklists, and validation-command suggestions.
|
||||||
- `e2e` owns user-level end-to-end smoke tests and Playwright UI automation; read `e2e/AGENTS.md` before editing its tests or commands.
|
- `e2e` owns user-level end-to-end smoke tests and Playwright UI automation; read `e2e/AGENTS.md` before editing its tests or commands.
|
||||||
|
|
||||||
## Inactive or placeholder directories
|
## Inactive or placeholder directories
|
||||||
|
|
@ -46,8 +47,8 @@ This file is the single source of truth for agents entering this repository. Rea
|
||||||
|
|
||||||
## Root command boundary
|
## Root command boundary
|
||||||
|
|
||||||
- Keep root scripts reserved for true repo-level checks and tools control-plane entrypoints: `pnpm guard`, `pnpm typecheck`, `pnpm tools-dev`, and `pnpm tools-pack`.
|
- Keep root scripts reserved for true repo-level checks and tools control-plane entrypoints: `pnpm guard`, `pnpm typecheck`, `pnpm tools-dev`, `pnpm tools-pack`, and `pnpm tools-pr`.
|
||||||
- Do not add root aggregate `pnpm build` or `pnpm test` aliases. Build/test commands must stay package-scoped (`pnpm --filter <package> ...`) or tool-scoped (`pnpm tools-pack ...`).
|
- Do not add root aggregate `pnpm build` or `pnpm test` aliases. Build/test commands must stay package-scoped (`pnpm --filter <package> ...`) or tool-scoped (`pnpm tools-pack ...` / `pnpm tools-pr ...`).
|
||||||
- Do not add root e2e aliases; e2e package commands and ownership rules live in `e2e/AGENTS.md`.
|
- Do not add root e2e aliases; e2e package commands and ownership rules live in `e2e/AGENTS.md`.
|
||||||
|
|
||||||
## Boundary constraints
|
## Boundary constraints
|
||||||
|
|
@ -79,6 +80,19 @@ This file is the single source of truth for agents entering this repository. Rea
|
||||||
- Blocking review feedback should focus on correctness, security/secrets, data integrity, repository boundary violations, contract/migration breakage, missing required validation, or high-risk maintainability issues.
|
- Blocking review feedback should focus on correctness, security/secrets, data integrity, repository boundary violations, contract/migration breakage, missing required validation, or high-risk maintainability issues.
|
||||||
- Only maintainers may close a PR instead of requesting changes, and only when the change is not salvageable on the existing branch (wrong target product, foreign test harness, DOM/API assumptions absent from this repo, or scripts that conflict with lifecycle rules).
|
- Only maintainers may close a PR instead of requesting changes, and only when the change is not salvageable on the existing branch (wrong target product, foreign test harness, DOM/API assumptions absent from this repo, or scripts that conflict with lifecycle rules).
|
||||||
|
|
||||||
|
## PR-duty tooling
|
||||||
|
|
||||||
|
`pnpm tools-pr` is the maintainer-only control plane for PR-duty work on this repo. It is a thin `gh` wrapper that encodes repo-specific knowledge — review-lane derivation, forbidden-surface flags, per-lane checklists, validation-command suggestions, and a fixed dictionary of factual classify tags (`bot-only-approval`, `needs-rebase`, `stale-approval`, `unresolved-changes-requested`, `awaiting-*` timing, `org-member`, etc.). The tool is read-only on the PR surface: it never approves, merges, comments, or closes; those side effects stay in explicit `gh` invocations the maintainer runs.
|
||||||
|
|
||||||
|
Common subcommands:
|
||||||
|
|
||||||
|
- `pnpm tools-pr list` — triage the open queue by lane and review-state bucket.
|
||||||
|
- `pnpm tools-pr view <num>` — factual review brief for a single PR.
|
||||||
|
- `pnpm tools-pr classify --all` — script-level tag JSON for the whole open queue (entry point for cron / digest consumers); per-PR `classify <num>` for spot checks.
|
||||||
|
- `pnpm tools-pr assignment` — assigner-perspective ownership + idle-time / blocker view across the queue.
|
||||||
|
|
||||||
|
For the full tag dictionary, operational playbook (direct merge / duplicate-title / awaiting-author / org-member / agent-review flows), comment templates, language-detection rules, and tool-design constraints (precision boundaries, factual-output rule, retry + pagination strategy), see [`tools/pr/AGENTS.md`](tools/pr/AGENTS.md).
|
||||||
|
|
||||||
## Validation strategy
|
## Validation strategy
|
||||||
|
|
||||||
- After package, workspace, or command-entry changes, run `pnpm install` so workspace links and generated dist entries stay fresh.
|
- After package, workspace, or command-entry changes, run `pnpm install` so workspace links and generated dist entries stay fresh.
|
||||||
|
|
@ -122,6 +136,14 @@ pnpm guard
|
||||||
pnpm typecheck
|
pnpm typecheck
|
||||||
```
|
```
|
||||||
|
|
||||||
|
```bash
|
||||||
|
pnpm tools-pr list
|
||||||
|
pnpm tools-pr list --bucket=merge-ready,approved-blocked
|
||||||
|
pnpm tools-pr list --lane=skill,contract --json
|
||||||
|
pnpm tools-pr view 1180
|
||||||
|
pnpm tools-pr view 1180 --json
|
||||||
|
```
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
pnpm --filter @open-design/web typecheck
|
pnpm --filter @open-design/web typecheck
|
||||||
pnpm --filter @open-design/web test
|
pnpm --filter @open-design/web test
|
||||||
|
|
@ -131,6 +153,7 @@ pnpm --filter @open-design/daemon build
|
||||||
pnpm --filter @open-design/desktop build
|
pnpm --filter @open-design/desktop build
|
||||||
pnpm --filter @open-design/tools-dev build
|
pnpm --filter @open-design/tools-dev build
|
||||||
pnpm --filter @open-design/tools-pack build
|
pnpm --filter @open-design/tools-pack build
|
||||||
|
pnpm --filter @open-design/tools-pr build
|
||||||
```
|
```
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
|
|
|
||||||
|
|
@ -171,6 +171,7 @@ function renderProjectView() {
|
||||||
config={config}
|
config={config}
|
||||||
agents={[] as AgentInfo[]}
|
agents={[] as AgentInfo[]}
|
||||||
skills={[] as SkillSummary[]}
|
skills={[] as SkillSummary[]}
|
||||||
|
designTemplates={[] as SkillSummary[]}
|
||||||
designSystems={[] as DesignSystemSummary[]}
|
designSystems={[] as DesignSystemSummary[]}
|
||||||
daemonLive
|
daemonLive
|
||||||
onModeChange={vi.fn()}
|
onModeChange={vi.fn()}
|
||||||
|
|
|
||||||
|
|
@ -13,6 +13,7 @@
|
||||||
"postinstall": "node ./scripts/postinstall.mjs",
|
"postinstall": "node ./scripts/postinstall.mjs",
|
||||||
"tools-dev": "pnpm exec tools-dev",
|
"tools-dev": "pnpm exec tools-dev",
|
||||||
"tools-pack": "pnpm exec tools-pack",
|
"tools-pack": "pnpm exec tools-pack",
|
||||||
|
"tools-pr": "pnpm exec tools-pr",
|
||||||
"guard": "tsx ./scripts/guard.ts",
|
"guard": "tsx ./scripts/guard.ts",
|
||||||
"i18n:check": "tsx ./scripts/i18n-check.ts",
|
"i18n:check": "tsx ./scripts/i18n-check.ts",
|
||||||
"sync:community-pets": "node --experimental-strip-types scripts/sync-community-pets.ts",
|
"sync:community-pets": "node --experimental-strip-types scripts/sync-community-pets.ts",
|
||||||
|
|
@ -24,6 +25,7 @@
|
||||||
"devDependencies": {
|
"devDependencies": {
|
||||||
"@open-design/tools-dev": "workspace:*",
|
"@open-design/tools-dev": "workspace:*",
|
||||||
"@open-design/tools-pack": "workspace:*",
|
"@open-design/tools-pack": "workspace:*",
|
||||||
|
"@open-design/tools-pr": "workspace:*",
|
||||||
"@types/node": "^20.17.10",
|
"@types/node": "^20.17.10",
|
||||||
"tsx": "4.21.0",
|
"tsx": "4.21.0",
|
||||||
"typescript": "^5.6.3"
|
"typescript": "^5.6.3"
|
||||||
|
|
|
||||||
|
|
@ -14,6 +14,9 @@ importers:
|
||||||
'@open-design/tools-pack':
|
'@open-design/tools-pack':
|
||||||
specifier: workspace:*
|
specifier: workspace:*
|
||||||
version: link:tools/pack
|
version: link:tools/pack
|
||||||
|
'@open-design/tools-pr':
|
||||||
|
specifier: workspace:*
|
||||||
|
version: link:tools/pr
|
||||||
'@types/node':
|
'@types/node':
|
||||||
specifier: ^20.17.10
|
specifier: ^20.17.10
|
||||||
version: 20.19.39
|
version: 20.19.39
|
||||||
|
|
@ -377,6 +380,25 @@ importers:
|
||||||
specifier: ^2.1.8
|
specifier: ^2.1.8
|
||||||
version: 2.1.9(@types/node@24.12.2)(jsdom@29.1.1)
|
version: 2.1.9(@types/node@24.12.2)(jsdom@29.1.1)
|
||||||
|
|
||||||
|
tools/pr:
|
||||||
|
dependencies:
|
||||||
|
cac:
|
||||||
|
specifier: 6.7.14
|
||||||
|
version: 6.7.14
|
||||||
|
devDependencies:
|
||||||
|
'@types/node':
|
||||||
|
specifier: 24.12.2
|
||||||
|
version: 24.12.2
|
||||||
|
esbuild:
|
||||||
|
specifier: 0.27.7
|
||||||
|
version: 0.27.7
|
||||||
|
tsx:
|
||||||
|
specifier: 4.21.0
|
||||||
|
version: 4.21.0
|
||||||
|
typescript:
|
||||||
|
specifier: 6.0.3
|
||||||
|
version: 6.0.3
|
||||||
|
|
||||||
packages:
|
packages:
|
||||||
|
|
||||||
7zip-bin@5.2.0:
|
7zip-bin@5.2.0:
|
||||||
|
|
|
||||||
|
|
@ -59,6 +59,8 @@ const residualAllowedExactPaths = new Set([
|
||||||
"tools/dev/esbuild.config.mjs",
|
"tools/dev/esbuild.config.mjs",
|
||||||
"tools/pack/bin/tools-pack.mjs",
|
"tools/pack/bin/tools-pack.mjs",
|
||||||
"tools/pack/esbuild.config.mjs",
|
"tools/pack/esbuild.config.mjs",
|
||||||
|
"tools/pr/bin/tools-pr.mjs",
|
||||||
|
"tools/pr/esbuild.config.mjs",
|
||||||
"tools/pack/resources/mac/notarize.cjs",
|
"tools/pack/resources/mac/notarize.cjs",
|
||||||
// electron-builder hook path; CJS compatibility entry used by tools-pack desktop builds.
|
// electron-builder hook path; CJS compatibility entry used by tools-pack desktop builds.
|
||||||
"tools/pack/resources/web-standalone-after-pack.cjs",
|
"tools/pack/resources/web-standalone-after-pack.cjs",
|
||||||
|
|
@ -360,6 +362,7 @@ const toolsRootAllowlist = new Map<string, "directory" | "file">([
|
||||||
["AGENTS.md", "file"],
|
["AGENTS.md", "file"],
|
||||||
["dev", "directory"],
|
["dev", "directory"],
|
||||||
["pack", "directory"],
|
["pack", "directory"],
|
||||||
|
["pr", "directory"],
|
||||||
]);
|
]);
|
||||||
|
|
||||||
async function checkToolsLayout(): Promise<boolean> {
|
async function checkToolsLayout(): Promise<boolean> {
|
||||||
|
|
@ -373,7 +376,7 @@ async function checkToolsLayout(): Promise<boolean> {
|
||||||
const repositoryPath = `tools/${entry.name}${entry.isDirectory() ? "/" : ""}`;
|
const repositoryPath = `tools/${entry.name}${entry.isDirectory() ? "/" : ""}`;
|
||||||
|
|
||||||
if (expected == null) {
|
if (expected == null) {
|
||||||
violations.push(`${repositoryPath} -> tools/ top-level entries are allowlisted; expected only AGENTS.md, dev/, and pack/`);
|
violations.push(`${repositoryPath} -> tools/ top-level entries are allowlisted; expected only AGENTS.md, dev/, pack/, and pr/`);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -13,6 +13,7 @@ const buildTargets = [
|
||||||
"packages/platform",
|
"packages/platform",
|
||||||
"tools/dev",
|
"tools/dev",
|
||||||
"tools/pack",
|
"tools/pack",
|
||||||
|
"tools/pr",
|
||||||
];
|
];
|
||||||
|
|
||||||
const jsExtensions = new Set([".js", ".cjs", ".mjs"]);
|
const jsExtensions = new Set([".js", ".cjs", ".mjs"]);
|
||||||
|
|
|
||||||
|
|
@ -9,6 +9,7 @@ Follow the root `AGENTS.md` first. This file only records module-level boundarie
|
||||||
- `pnpm tools-dev run web` runs foreground daemon + web for the Playwright webServer flow.
|
- `pnpm tools-dev run web` runs foreground daemon + web for the Playwright webServer flow.
|
||||||
- `pnpm tools-dev inspect desktop ...` inspects the desktop runtime through sidecar IPC.
|
- `pnpm tools-dev inspect desktop ...` inspects the desktop runtime through sidecar IPC.
|
||||||
- `tools/pack` provides `@open-design/tools-pack` and the `tools-pack` bin. The active slice is packaged artifact build/install/start/stop/logs/uninstall/cleanup/list/reset plus beta release artifact preparation for mac and Windows lanes, plus a Linux AppImage lane with optional containerized builds.
|
- `tools/pack` provides `@open-design/tools-pack` and the `tools-pack` bin. The active slice is packaged artifact build/install/start/stop/logs/uninstall/cleanup/list/reset plus beta release artifact preparation for mac and Windows lanes, plus a Linux AppImage lane with optional containerized builds.
|
||||||
|
- `tools/pr` provides `@open-design/tools-pr` and the `tools-pr` bin. It is the maintainer PR-duty control plane: a thin `gh` wrapper that encodes this repo's review-lane derivation, forbidden-surface flags, per-lane checklists, and validation-command suggestions. It must not perform side effects (approve / request changes / merge / close / push); those stay in explicit `gh` calls the maintainer runs.
|
||||||
|
|
||||||
## Packaging scope
|
## Packaging scope
|
||||||
|
|
||||||
|
|
@ -31,6 +32,8 @@ pnpm --filter @open-design/tools-dev typecheck
|
||||||
pnpm --filter @open-design/tools-dev build
|
pnpm --filter @open-design/tools-dev build
|
||||||
pnpm --filter @open-design/tools-pack typecheck
|
pnpm --filter @open-design/tools-pack typecheck
|
||||||
pnpm --filter @open-design/tools-pack build
|
pnpm --filter @open-design/tools-pack build
|
||||||
|
pnpm --filter @open-design/tools-pr typecheck
|
||||||
|
pnpm --filter @open-design/tools-pr build
|
||||||
pnpm tools-dev status --json
|
pnpm tools-dev status --json
|
||||||
pnpm tools-dev logs --json
|
pnpm tools-dev logs --json
|
||||||
pnpm tools-dev check
|
pnpm tools-dev check
|
||||||
|
|
@ -47,4 +50,8 @@ pnpm tools-pack linux install --headless
|
||||||
pnpm tools-pack linux start --headless
|
pnpm tools-pack linux start --headless
|
||||||
pnpm tools-pack linux stop --headless
|
pnpm tools-pack linux stop --headless
|
||||||
pnpm tools-pack linux build --containerized
|
pnpm tools-pack linux build --containerized
|
||||||
|
pnpm tools-pr list
|
||||||
|
pnpm tools-pr list --bucket=merge-ready,approved-blocked
|
||||||
|
pnpm tools-pr view <num>
|
||||||
|
pnpm tools-pr view <num> --json
|
||||||
```
|
```
|
||||||
|
|
|
||||||
214
tools/pr/AGENTS.md
Normal file
214
tools/pr/AGENTS.md
Normal file
|
|
@ -0,0 +1,214 @@
|
||||||
|
# tools/pr
|
||||||
|
|
||||||
|
Follow the root `AGENTS.md` and `tools/AGENTS.md` first. This tool owns the maintainer PR-duty command surface for nexu-io/open-design.
|
||||||
|
|
||||||
|
## Owns
|
||||||
|
|
||||||
|
- Repository-specific triage and review preparation on top of `gh`.
|
||||||
|
- Lane derivation from touched paths (default / contract / skill / design-system / craft / docs / multi), per `docs/code-review-guidelines.md` §4.
|
||||||
|
- Forbidden-surface detection on diff paths (e.g. `apps/nextjs/`, `packages/shared/`), per `docs/code-review-guidelines.md` §2.
|
||||||
|
- Per-lane rule citations matching the hard lines in `docs/code-review-guidelines.md` §4.x and `CONTRIBUTING.zh-CN.md`, with source noted inline.
|
||||||
|
- Validation command derivation from touched packages, citing `AGENTS.md §Validation strategy`.
|
||||||
|
- Factual brief assembly: de-noised top files, bot-stripped reviews/comments, CI rollup, body preview.
|
||||||
|
- Script-level tag emission via `classify`, with each tag carrying a sharp, mechanical rule and a source token (see §Tag dictionary).
|
||||||
|
|
||||||
|
## Does not own
|
||||||
|
|
||||||
|
- `gh` configuration, GitHub credentials, or authentication.
|
||||||
|
- PR side effects (approve / request changes / merge / close). Side effects stay in `gh` invocations the maintainer runs explicitly.
|
||||||
|
- Branch checkout, local rebase, or push operations.
|
||||||
|
- Sidecar protocol, runtime topology, packaged release, or app business logic.
|
||||||
|
|
||||||
|
## Rules
|
||||||
|
|
||||||
|
- **Output is strictly factual.** Every line of human or JSON output must be either (a) data observed from `gh` / the diff / repository paths, or (b) a direct citation of a rule from this repo's own docs (`AGENTS.md`, `docs/code-review-guidelines.md §X`, `CONTRIBUTING.zh-CN.md`, etc.) with the source noted inline. Tools-pr **does not** emit risk verdicts (LOW/MEDIUM/HIGH), merge recommendations, or directive language (`should`, `must`, `do not`, `recommended`, `encouraged`, `suggested`). Judgment belongs to the reviewer who consumes the brief, not to the tool.
|
||||||
|
- Stay a *thin* wrapper. Each subcommand corresponds to a real PR-duty action; do not introduce abstractions that have no caller.
|
||||||
|
- Keep dependencies minimal: `cac` for subcommand parsing, no GitHub SDKs. Use `gh` via `node:child_process`.
|
||||||
|
- `gh pr list` with 12+ JSON fields returns HTTP 502 across this repo's open queue. Chunk `--json` selections into multiple smaller calls and join by PR number (`src/gh.ts:fetchOpenPrs`).
|
||||||
|
- Heavy chunks (`reviews`, `comments`) use cursor-paginated `gh api graphql` via `fetchPaginatedPrList` with `PR_LIST_PAGE_SIZE = 30`. Light chunks (`meta`, `stats`, `files`) stay on `gh pr list --json` — they're already cheap. The split keeps per-page node count low and lets `gh` retry pages individually when the upstream gateway flakes.
|
||||||
|
- Transient gateway errors (HTTP 5xx, `EAI_AGAIN`, etc.) trigger a minimum-touch retry inside `gh()` (two attempts at 1s + 2s backoff). Anything else (4xx, auth failure, schema rejection, JSON parse) surfaces immediately — retry must not mask real problems.
|
||||||
|
- Lane and forbidden-surface rules track `docs/code-review-guidelines.md`. When that document changes, update `src/lane.ts` in the same PR.
|
||||||
|
- Per-lane rules must point at the hard lines that already exist in the review/contribution docs, with the source cited inline — do not invent new requirements here.
|
||||||
|
- Output formats are stable contracts: human report is for terminal eyes, `--json` is for downstream agents and future subcommands. Adding/removing JSON fields counts as a breaking change for the JSON consumer surface.
|
||||||
|
|
||||||
|
## Tag dictionary (v1)
|
||||||
|
|
||||||
|
Each tag has a single mechanical rule. Adding new tags requires the rule to be expressible as one factual sentence, derivable purely from `gh` data + file paths, and unlikely to false-positive in legitimate use. Patterns that fail this test (e.g. `missing-test-changes`, `contract-no-consumer-update`, `bulk-author`) are intentionally not implemented — see `feedback_tools_pr_precise_boundaries` in maintainer memory for the exclusion list.
|
||||||
|
|
||||||
|
| Tag | Rule | Data source |
|
||||||
|
|---|---|---|
|
||||||
|
| `bot-only-approval` | `reviewDecision === "APPROVED"` and every review with `state === "APPROVED"` matches `isBotAuthored()` | gh.reviewDecision + latestReviews |
|
||||||
|
| `needs-rebase` | `mergeStateStatus ∈ {DIRTY, BEHIND}` | gh.mergeStateStatus |
|
||||||
|
| `forbidden-surface` | A touched path matches the regex set in `lane.ts:deriveForbidden` (AGENTS.md §Forbidden surfaces) | files + lane.deriveForbidden |
|
||||||
|
| `unlabeled` | The PR is missing at least one of the `size/`, `risk/`, `type/` label prefixes | gh.labels |
|
||||||
|
| `duplicate-title` | Another open PR by the same author has a byte-for-byte identical `title` | cross-PR titleIndexByAuthor |
|
||||||
|
| `non-ascii-slug` | A design-system root touched by the PR has a slug that fails `/^[a-z0-9-]+$/` | files + lane.DESIGN_DIR |
|
||||||
|
| `maintainer-edits-disabled` | `maintainerCanModify === false` | gh.maintainerCanModify |
|
||||||
|
| `org-member` | PR author's GitHub login appears in `gh api orgs/<repo-owner>/members` | gh REST orgs members list |
|
||||||
|
| `unresolved-changes-requested` | Any reviewer's latest review has `state === "CHANGES_REQUESTED"` | gh.latestReviews[].state |
|
||||||
|
| `stale-approval` | Any `APPROVED` review's `commit.oid` differs from current `headRefOid` | gh.latestReviews[].commit.oid + gh.headRefOid |
|
||||||
|
| `awaiting-author-response-24h` | Latest human-reviewer signal time is newer than the latest author signal time and is ≥ 24h ago | latestReviews + comments + commits |
|
||||||
|
| `awaiting-reviewer-response-24h` | Latest author signal time is newer than the latest human-reviewer signal time, ≥ 24h ago, and at least one human-reviewer signal exists | latestReviews + comments + commits |
|
||||||
|
| `awaiting-first-review-24h` | No human review or non-author non-bot comment exists, and `createdAt` is ≥ 24h ago | latestReviews + comments + createdAt |
|
||||||
|
|
||||||
|
**Signal-time definitions** (used by the three `awaiting-*` tags):
|
||||||
|
|
||||||
|
- *author signal* = `max(commits[].committedDate) ∪ max(comments[?author.login==prAuthor].createdAt)`
|
||||||
|
- *human-reviewer signal* = `max(latestReviews[?author!=prAuthor && !isBotAuthored].submittedAt) ∪ max(comments[?author!=prAuthor && !isBotAuthored].createdAt)`
|
||||||
|
|
||||||
|
The three `awaiting-*` tags are mutually exclusive by construction. Each of them also sets `tag.awaitingHours` — the integer hour count between the awaiting-window start (latest reviewer signal / latest author signal / `createdAt` respectively) and the classify-run moment. Downstream consumers use it to sort PRs within an awaiting bucket by actual stuck duration, or floor-divide by 24 for days.
|
||||||
|
|
||||||
|
### Rate-limit telemetry
|
||||||
|
|
||||||
|
`classify --all` records a `rate` object in the report and in the summary line so detector additions that quietly inflate API cost are visible immediately:
|
||||||
|
|
||||||
|
- `rate.before` / `rate.after`: GraphQL `rateLimit { remaining, limit, resetAt }` snapshots taken before and after the bulk fetch.
|
||||||
|
- `rate.cost`: `before.remaining − after.remaining` when both snapshots fall in the same reset window; `null` when the hourly window rolls over between snapshots.
|
||||||
|
|
||||||
|
Each snapshot itself costs 1 point; the two extra snapshot calls per `--all` run are negligible against the 5000-point hourly budget.
|
||||||
|
|
||||||
|
## Templates
|
||||||
|
|
||||||
|
`tools/pr/templates/*.md` holds **aesthetic references** for the recurring comment kinds surfaced by classify tags. Each file describes the beats the comment should hit and shows one exemplar phrasing in the maintainer's voice.
|
||||||
|
|
||||||
|
**Templates are not fill-in forms.** Do not `sed`-substitute placeholders and post the rendered text — repeated identical comments break the human-to-human tone we want to keep with contributors. Instead, for each post: read the template to absorb the tone, weave the PR-specific facts (author, awaiting duration, branch names, diff size) into a fresh comment that hits the same beats with locally adapted wording.
|
||||||
|
|
||||||
|
**Author-addressed comments adapt to the author's language.** For nudge / duplicate-ask / close-with-reason comments — i.e. anything @-mentioning a specific contributor in a private-feeling exchange — detect the author's preferred language before writing the comment:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
gh pr view <num> --json body,comments,author --jq '
|
||||||
|
.author.login as $a |
|
||||||
|
([.body, (.comments[] | select(.author.login == $a) | .body)] | join("\n"))
|
||||||
|
'
|
||||||
|
```
|
||||||
|
|
||||||
|
If the resulting text contains CJK characters (`grep -P "[\\p{Han}]"`), write the comment in Chinese; otherwise English. Broadcasting comments (PR descriptions, commit messages, review summaries visible to all reviewers) stay in English regardless. See maintainer memory `feedback_public_artifacts_english` for the full scope rule.
|
||||||
|
|
||||||
|
The frontmatter of each template lists the beats and the placeholder slots. Templates are intentionally text files maintained alongside the tool source, not generated by `tools-pr` — the tool itself stays side-effect-free.
|
||||||
|
|
||||||
|
Current templates:
|
||||||
|
|
||||||
|
| Template | Triggered by | Posted on |
|
||||||
|
|---|---|---|
|
||||||
|
| `duplicate-title-ask.md` | `duplicate-title` tag (same author, byte-for-byte identical title) | The older / more-iterated PR of the pair |
|
||||||
|
| `awaiting-author-nudge.md` | `awaiting-author-response-24h` tag, when `tag.awaitingHours ≥ 96` (≥ 4 days) | The PR; addressed to the author |
|
||||||
|
| `agent-review.md` | Bucket-3 high-value / high-risk technical PR review prep | Not posted; internal analysis artifact at `.tmp/tools-pr/reviews/<num>.md` |
|
||||||
|
|
||||||
|
## Operational playbook
|
||||||
|
|
||||||
|
Per classify tag bucket, the maintainer workflow. Each row is the minimum action — escalation (close, force-merge, etc.) is the maintainer's call.
|
||||||
|
|
||||||
|
### Direct merge (APPROVED + CLEAN, surgical)
|
||||||
|
|
||||||
|
1. Sanity-check the merge state:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
gh pr view <num> --json state,reviewDecision,mergeStateStatus,statusCheckRollup \
|
||||||
|
--jq '{state, reviewDecision, mergeStateStatus,
|
||||||
|
checks: [.statusCheckRollup[] | {conclusion, name: (.workflowName // .name)}]}'
|
||||||
|
```
|
||||||
|
|
||||||
|
Expect `state=OPEN`, `reviewDecision=APPROVED`, `mergeStateStatus=CLEAN`, every check `SUCCESS`.
|
||||||
|
|
||||||
|
2. If `tools-pr classify <num>` includes `bot-only-approval`, verify the change is surgical (size/XS, single file, < ~30 lines, no boundary or contract surface) before proceeding. Judgment lives in maintainer memory (`feedback_bot_only_approval`), not in the tool.
|
||||||
|
|
||||||
|
3. Squash-merge per repo convention: `gh pr merge <num> --squash`.
|
||||||
|
|
||||||
|
4. Confirm: `gh pr view <num> --json state,mergedAt,mergeCommit --jq '{state, mergedAt, sha: .mergeCommit.oid[0:10]}'`.
|
||||||
|
|
||||||
|
### `duplicate-title`
|
||||||
|
|
||||||
|
1. Inspect both PRs to pick the older / more-iterated one (the author may want to preserve its history). Useful comparison:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
gh pr view <NUM> --json number,headRefName,commits,additions,deletions,createdAt,updatedAt
|
||||||
|
```
|
||||||
|
|
||||||
|
2. Read `templates/duplicate-title-ask.md` for tone and beat structure, then write a fresh comment that hits the same beats with the actual PR facts woven in (author login, both branch names, both commit counts, both diff sizes). Post to the older PR:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
# Write the composed comment to a scratch file, then:
|
||||||
|
gh pr comment <older-num> -F /tmp/dup-ask-<older-num>.md
|
||||||
|
```
|
||||||
|
|
||||||
|
3. Wait for author response. If no response after 7d, close the older PR with `gh pr close <older-num> --comment "Superseded by #<newer-num>."`.
|
||||||
|
|
||||||
|
### `awaiting-author-response-24h` (long tail, ≥ 4 days)
|
||||||
|
|
||||||
|
1. Filter the classify report for the PRs that crossed the 96h threshold, **exclude `org-member` PRs**, and rank by `awaitingHours`:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
jq '[.byTag["awaiting-author-response-24h"][] as $n
|
||||||
|
| .byNumber[($n|tostring)] as $tags
|
||||||
|
| select($tags | map(.name) | contains(["org-member"]) | not)
|
||||||
|
| $tags[]
|
||||||
|
| select(.name=="awaiting-author-response-24h" and .awaitingHours >= 96)
|
||||||
|
| {n: $n, h: .awaitingHours}]
|
||||||
|
| sort_by(-.h)' .tmp/tools-pr/classify/<latest>.json
|
||||||
|
```
|
||||||
|
|
||||||
|
2. For each remaining PR, read `templates/awaiting-author-nudge.md` for tone, then compose a fresh comment that hits the same beats with the actual author login and human-formatted awaiting duration. Vary the wording slightly between PRs nudged in the same session (a contributor seeing identical pings across their notifications breaks the friendly-human feel).
|
||||||
|
|
||||||
|
3. Post: `gh pr comment <num> -F /tmp/nudge-<num>.md`.
|
||||||
|
|
||||||
|
4. Re-check the classify report in a follow-up run; the awaiting tag should clear once the author responds. If no response by 14d, escalate (a more direct stale-warning or close-after-warning).
|
||||||
|
|
||||||
|
### `org-member`
|
||||||
|
|
||||||
|
`org-member` is informational and pairs with the other operational tags rather than triggering its own GitHub action. When a PR carries `org-member` alongside `awaiting-*` / `duplicate-title` / `maintainer-edits-disabled` / etc., the run-of-the-mill GitHub-comment workflow does **not** apply — those communications are routed through the team's internal IM instead. See maintainer memory `feedback_org_members_im_channel` for the channel split (operational nudges → IM; substantive review feedback and decisions → GitHub).
|
||||||
|
|
||||||
|
Every operational playbook step that posts a public comment must filter `org-member` out first; the `awaiting-author-response-24h` flow above shows the canonical filter.
|
||||||
|
|
||||||
|
### `tools-pr assignment` — assigner-perspective ownership view
|
||||||
|
|
||||||
|
Read-only aggregation that pivots the open PR queue by assignee. For each currently-assigned PR per assignee, surfaces:
|
||||||
|
|
||||||
|
- **assigned-since**: `now − assignedAt`, where `assignedAt` is the latest `ASSIGNED_EVENT` for that assignee that has not been superseded by an `UNASSIGNED_EVENT` (fetched via `gh api graphql` timeline — only path that exposes it).
|
||||||
|
- **assigned-by**: the actor on that event. Marked `(self-assigned)` when actor == assignee.
|
||||||
|
- **idle-for**: `now − max(assignedAt, last assignee activity)`, where activity = commit / comment / review by that assignee.
|
||||||
|
- **state badges**: `reviewDecision`, `mergeStateStatus`, `draft` (only when non-trivial).
|
||||||
|
- **status / blockers**: composed from existing classify tags (`needs-rebase`, `unresolved-changes-requested`, `stale-approval`, `awaiting-*`, `bot-only-approval`). No new judgments.
|
||||||
|
|
||||||
|
Flow:
|
||||||
|
|
||||||
|
1. `gh issue/pr edit <num> --add-assignee <login>` is still the assignment action (tools-pr is read-only on this surface).
|
||||||
|
2. `pnpm tools-pr assignment` shows the resulting state grouped by assignee, sorted by idle-hours desc within each bucket.
|
||||||
|
3. Use `--user me` (or `--user <login>`) for one bucket, `--unassigned` to expand the un-owned tail.
|
||||||
|
4. `--json` for cron / digest consumption.
|
||||||
|
|
||||||
|
Timeline fetch uses the cursor-paginated graphql path (`fetchPaginatedPrList`) — same retry + page-size primitives as reviews/comments.
|
||||||
|
|
||||||
|
### Agent review (bucket-3 high-value / high-risk PRs)
|
||||||
|
|
||||||
|
For PRs that warrant a deep technical pre-review (contract lane PRs, large refactors, security-sensitive fixes, scope-mixed PRs flagged via classify, etc.), an agent produces an analysis brief under `.tmp/tools-pr/reviews/<num>.md`. The brief is an internal artifact for the maintainer's consumption — not a GitHub comment. If maintainer decides to post review feedback, a separate downstream step adapts findings to public-facing review text (typically rephrased to address the author directly, with the channel respecting `org-member` routing).
|
||||||
|
|
||||||
|
Flow:
|
||||||
|
|
||||||
|
1. Pull `tools-pr view <num>` for the structural brief and `gh pr diff <num>` for the patch.
|
||||||
|
2. Read `templates/agent-review.md` for tone + section pool, then compose for the specific PR (sections appear only when they carry signal — see `feedback_agent_review_shape`).
|
||||||
|
3. Write to `.tmp/tools-pr/reviews/<num>.md` (transient runtime artifact; this directory is not version-controlled and can be cleaned at any time).
|
||||||
|
4. Surface the brief to the maintainer; let them decide split / block / merge / IM / public-review.
|
||||||
|
|
||||||
|
Frozen-in-time exemplars covering three PR shapes live in `tools/pr/templates/examples/` (scope-expanded, clean contract feature, CHANGES_REQUESTED with prior human reviews) and are the canonical references for the style.
|
||||||
|
|
||||||
|
## Common commands
|
||||||
|
|
||||||
|
```bash
|
||||||
|
pnpm --filter @open-design/tools-pr typecheck
|
||||||
|
pnpm --filter @open-design/tools-pr build
|
||||||
|
pnpm tools-pr list
|
||||||
|
pnpm tools-pr list --bucket=merge-ready,approved-blocked
|
||||||
|
pnpm tools-pr list --lane=skill,contract
|
||||||
|
pnpm tools-pr list --author=xxiaoxiong --json
|
||||||
|
pnpm tools-pr view 1180
|
||||||
|
pnpm tools-pr view 1180 --json
|
||||||
|
pnpm tools-pr classify 1167 # single PR, stdout
|
||||||
|
pnpm tools-pr classify 1167 --json # single PR, JSON stdout
|
||||||
|
pnpm tools-pr classify --all # full queue, JSON file → .tmp/tools-pr/classify/<ts>.json
|
||||||
|
pnpm tools-pr classify --all --name daily # override filename stem
|
||||||
|
pnpm tools-pr classify --all --print # also dump JSON to stdout
|
||||||
|
pnpm tools-pr assignment # assigner-perspective queue view
|
||||||
|
pnpm tools-pr assignment --user me # only my bucket
|
||||||
|
pnpm tools-pr assignment --unassigned # expand the un-owned tail
|
||||||
|
pnpm tools-pr assignment --json # JSON for cron / digest
|
||||||
|
```
|
||||||
18
tools/pr/bin/tools-pr.mjs
Executable file
18
tools/pr/bin/tools-pr.mjs
Executable file
|
|
@ -0,0 +1,18 @@
|
||||||
|
#!/usr/bin/env node
|
||||||
|
|
||||||
|
import { existsSync } from 'node:fs';
|
||||||
|
import { dirname, resolve } from 'node:path';
|
||||||
|
import { fileURLToPath, pathToFileURL } from 'node:url';
|
||||||
|
|
||||||
|
const entryDir = dirname(fileURLToPath(import.meta.url));
|
||||||
|
const distEntry = resolve(entryDir, '../dist/index.mjs');
|
||||||
|
const requiredDistEntries = [distEntry];
|
||||||
|
const missingDistEntries = requiredDistEntries.filter((entry) => !existsSync(entry));
|
||||||
|
|
||||||
|
if (missingDistEntries.length > 0) {
|
||||||
|
throw new Error(
|
||||||
|
`tools-pr dist entries not found: ${missingDistEntries.join(', ')}. Run "pnpm --filter @open-design/tools-pr build" first.`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
await import(pathToFileURL(distEntry).href);
|
||||||
18
tools/pr/esbuild.config.mjs
Normal file
18
tools/pr/esbuild.config.mjs
Normal file
|
|
@ -0,0 +1,18 @@
|
||||||
|
import { build } from "esbuild";
|
||||||
|
|
||||||
|
await build({
|
||||||
|
banner: {
|
||||||
|
js: "#!/usr/bin/env node",
|
||||||
|
},
|
||||||
|
bundle: true,
|
||||||
|
entryNames: "[name]",
|
||||||
|
entryPoints: ["./src/index.ts"],
|
||||||
|
format: "esm",
|
||||||
|
outdir: "./dist",
|
||||||
|
outExtension: {
|
||||||
|
".js": ".mjs",
|
||||||
|
},
|
||||||
|
packages: "external",
|
||||||
|
platform: "node",
|
||||||
|
target: "node24",
|
||||||
|
});
|
||||||
27
tools/pr/package.json
Normal file
27
tools/pr/package.json
Normal file
|
|
@ -0,0 +1,27 @@
|
||||||
|
{
|
||||||
|
"name": "@open-design/tools-pr",
|
||||||
|
"version": "0.6.0",
|
||||||
|
"private": true,
|
||||||
|
"type": "module",
|
||||||
|
"bin": {
|
||||||
|
"tools-pr": "./bin/tools-pr.mjs"
|
||||||
|
},
|
||||||
|
"scripts": {
|
||||||
|
"build": "node ./esbuild.config.mjs",
|
||||||
|
"dev": "tsx ./src/index.ts",
|
||||||
|
"test": "node --import tsx --test tests/*.test.ts",
|
||||||
|
"typecheck": "tsc -p tsconfig.json --noEmit"
|
||||||
|
},
|
||||||
|
"dependencies": {
|
||||||
|
"cac": "6.7.14"
|
||||||
|
},
|
||||||
|
"devDependencies": {
|
||||||
|
"@types/node": "24.12.2",
|
||||||
|
"esbuild": "0.27.7",
|
||||||
|
"tsx": "4.21.0",
|
||||||
|
"typescript": "6.0.3"
|
||||||
|
},
|
||||||
|
"engines": {
|
||||||
|
"node": "~24"
|
||||||
|
}
|
||||||
|
}
|
||||||
461
tools/pr/src/assignment.ts
Normal file
461
tools/pr/src/assignment.ts
Normal file
|
|
@ -0,0 +1,461 @@
|
||||||
|
/**
|
||||||
|
* `tools-pr assignment` — assigner-perspective view of PR ownership.
|
||||||
|
*
|
||||||
|
* For each currently-assigned PR, surfaces who owns it, when they were
|
||||||
|
* assigned (and by whom), how long it has been idle, and the assignment's
|
||||||
|
* progress / blocker status as derived from the existing classify tag set.
|
||||||
|
*
|
||||||
|
* Read-only aggregation: no new tags, no new judgments. Status strings
|
||||||
|
* compose existing tag facts (`needs-rebase`, `awaiting-*`, `bot-only-
|
||||||
|
* approval`, etc.) — see `tools/pr/AGENTS.md` §Tag dictionary.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { reduceLatestReviewsByAuthor } from "./bot.js";
|
||||||
|
import {
|
||||||
|
detectRepoSlug,
|
||||||
|
fetchCurrentUser,
|
||||||
|
fetchOpenPrAssignmentTimelines,
|
||||||
|
fetchOpenPrs,
|
||||||
|
fetchOrgMembers,
|
||||||
|
} from "./gh.js";
|
||||||
|
import { buildContext, classifyPr } from "./tags.js";
|
||||||
|
import type {
|
||||||
|
ClassifyContext,
|
||||||
|
GhAssignmentEvent,
|
||||||
|
GhAssignmentTimeline,
|
||||||
|
PrFacts,
|
||||||
|
Tag,
|
||||||
|
} from "./types.js";
|
||||||
|
|
||||||
|
// ---- per-assignment derived shape ---------------------------------------
|
||||||
|
|
||||||
|
type AssignmentEntry = {
|
||||||
|
number: number;
|
||||||
|
title: string;
|
||||||
|
assignee: string;
|
||||||
|
assignedAt: string | null;
|
||||||
|
assignedBy: string | null;
|
||||||
|
selfAssigned: boolean;
|
||||||
|
/** Hours since the assignment record (null if no event in fetched window). */
|
||||||
|
assignedHoursAgo: number | null;
|
||||||
|
/** Most recent moment the assignee took action on the PR (commit/comment/review),
|
||||||
|
* bounded below by assignedAt. Null when assignedAt is also null and there's
|
||||||
|
* no recorded activity. */
|
||||||
|
idleSinceAt: string | null;
|
||||||
|
idleHours: number | null;
|
||||||
|
/** Compact state badges (REVIEW_REQUIRED, CHANGES_REQUESTED, DIRTY, etc.). */
|
||||||
|
stateBadges: string[];
|
||||||
|
/** Status line — one of "ready to merge", "blocked: ...", "in review". */
|
||||||
|
status: string;
|
||||||
|
/** Blocker bullet lines drawn from the tag set. Empty for non-blocked PRs. */
|
||||||
|
blockers: string[];
|
||||||
|
/** Raw tags present on the PR for downstream JSON consumption. */
|
||||||
|
tags: Tag[];
|
||||||
|
isDraft: boolean;
|
||||||
|
};
|
||||||
|
|
||||||
|
type AssignmentReport = {
|
||||||
|
generatedAt: string;
|
||||||
|
openPrTotal: number;
|
||||||
|
assignedCount: number;
|
||||||
|
unassignedCount: number;
|
||||||
|
byAssignee: Record<string, AssignmentEntry[]>;
|
||||||
|
unassigned: Array<{
|
||||||
|
number: number;
|
||||||
|
title: string;
|
||||||
|
stateBadges: string[];
|
||||||
|
status: string;
|
||||||
|
blockers: string[];
|
||||||
|
isDraft: boolean;
|
||||||
|
}>;
|
||||||
|
};
|
||||||
|
|
||||||
|
// ---- derivation helpers -------------------------------------------------
|
||||||
|
|
||||||
|
const HOUR_MS = 60 * 60 * 1000;
|
||||||
|
|
||||||
|
function hoursBetween(fromIso: string, now: number): number {
|
||||||
|
return Math.floor((now - Date.parse(fromIso)) / HOUR_MS);
|
||||||
|
}
|
||||||
|
|
||||||
|
function formatDuration(hours: number | null): string {
|
||||||
|
if (hours === null) return "(unknown)";
|
||||||
|
if (hours < 24) return `${hours}h`;
|
||||||
|
const days = Math.floor(hours / 24);
|
||||||
|
const rem = hours - days * 24;
|
||||||
|
return rem === 0 ? `${days}d` : `${days}d ${rem}h`;
|
||||||
|
}
|
||||||
|
|
||||||
|
function truncate(s: string, n: number): string {
|
||||||
|
return s.length <= n ? s : `${s.slice(0, n - 1)}…`;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Walks the timeline events chronologically and tracks the latest
|
||||||
|
* ASSIGNED_EVENT per login that has not been superseded by an
|
||||||
|
* UNASSIGNED_EVENT for that login. Returns a map keyed by assignee login.
|
||||||
|
*/
|
||||||
|
function indexAssignmentEvents(
|
||||||
|
events: ReadonlyArray<GhAssignmentEvent>,
|
||||||
|
): Map<string, GhAssignmentEvent> {
|
||||||
|
const sorted = [...events].sort((a, b) => a.createdAt.localeCompare(b.createdAt));
|
||||||
|
const map = new Map<string, GhAssignmentEvent>();
|
||||||
|
for (const event of sorted) {
|
||||||
|
const login = event.assignee?.login;
|
||||||
|
if (!login) continue;
|
||||||
|
if (event.kind === "ASSIGNED") {
|
||||||
|
map.set(login, event);
|
||||||
|
} else {
|
||||||
|
map.delete(login);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return map;
|
||||||
|
}
|
||||||
|
|
||||||
|
function lastAssigneeActivityAt(facts: PrFacts, assignee: string): string | null {
|
||||||
|
let latest: number | null = null;
|
||||||
|
const consider = (iso: string) => {
|
||||||
|
const t = Date.parse(iso);
|
||||||
|
if (Number.isFinite(t) && (latest === null || t > latest)) latest = t;
|
||||||
|
};
|
||||||
|
for (const c of facts.commits) {
|
||||||
|
if (c.authorLogin === assignee) consider(c.committedDate);
|
||||||
|
}
|
||||||
|
for (const cmt of facts.comments) {
|
||||||
|
if (cmt.author?.login === assignee) consider(cmt.createdAt);
|
||||||
|
}
|
||||||
|
for (const r of facts.reviews) {
|
||||||
|
if (r.author?.login === assignee) consider(r.submittedAt);
|
||||||
|
}
|
||||||
|
return latest === null ? null : new Date(latest).toISOString();
|
||||||
|
}
|
||||||
|
|
||||||
|
function deriveStateBadges(facts: PrFacts): string[] {
|
||||||
|
const badges: string[] = [];
|
||||||
|
badges.push(facts.reviewDecision || "REVIEW_REQUIRED");
|
||||||
|
if (facts.mergeStateStatus !== "CLEAN" && facts.mergeStateStatus !== "UNKNOWN") {
|
||||||
|
badges.push(facts.mergeStateStatus);
|
||||||
|
}
|
||||||
|
if (facts.isDraft) badges.push("draft");
|
||||||
|
return badges;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Translates the existing tag set + PR metadata into a status line + blocker
|
||||||
|
* bullets. Status precedence (when multiple apply): blockers → bot-only-
|
||||||
|
* approval → ready-to-merge → in review.
|
||||||
|
*
|
||||||
|
* `ready to merge` requires `reviewDecision === "APPROVED"` and
|
||||||
|
* `mergeStateStatus ∈ {CLEAN, UNSTABLE}` (UNSTABLE = mergeable but a non-
|
||||||
|
* required check is failing — still actionable for the maintainer). Without
|
||||||
|
* this branch, an APPROVED + CLEAN PR with a human reviewer renders the
|
||||||
|
* same as a REVIEW_REQUIRED one, which drops the main triage signal.
|
||||||
|
*
|
||||||
|
* Each blocker bullet is a one-liner that carries the tag's `reason`
|
||||||
|
* verbatim — no judgment language, no priority labels. The assigner can
|
||||||
|
* scan the bullets and decide.
|
||||||
|
*/
|
||||||
|
function deriveStatus(
|
||||||
|
tags: ReadonlyArray<Tag>,
|
||||||
|
facts: PrFacts,
|
||||||
|
): { status: string; blockers: string[] } {
|
||||||
|
const byName = new Map(tags.map((t) => [t.name, t] as const));
|
||||||
|
const get = (name: string) => byName.get(name);
|
||||||
|
|
||||||
|
const blockers: string[] = [];
|
||||||
|
if (get("needs-rebase")) blockers.push("needs-rebase (main has moved)");
|
||||||
|
const unresolved = get("unresolved-changes-requested");
|
||||||
|
if (unresolved) blockers.push(unresolved.reason);
|
||||||
|
const stale = get("stale-approval");
|
||||||
|
if (stale) blockers.push(stale.reason);
|
||||||
|
const awaitAuthor = get("awaiting-author-response-24h");
|
||||||
|
if (awaitAuthor) blockers.push(`awaiting author for ${formatDuration(awaitAuthor.awaitingHours ?? null)}`);
|
||||||
|
const awaitReviewer = get("awaiting-reviewer-response-24h");
|
||||||
|
if (awaitReviewer) blockers.push(`awaiting reviewer for ${formatDuration(awaitReviewer.awaitingHours ?? null)}`);
|
||||||
|
const awaitFirst = get("awaiting-first-review-24h");
|
||||||
|
if (awaitFirst) blockers.push(`no human review yet (${formatDuration(awaitFirst.awaitingHours ?? null)} since createdAt)`);
|
||||||
|
|
||||||
|
const mergeReadyState =
|
||||||
|
facts.reviewDecision === "APPROVED" &&
|
||||||
|
(facts.mergeStateStatus === "CLEAN" || facts.mergeStateStatus === "UNSTABLE");
|
||||||
|
|
||||||
|
let status: string;
|
||||||
|
if (blockers.length > 0) {
|
||||||
|
status = "blocked";
|
||||||
|
} else if (get("bot-only-approval")) {
|
||||||
|
status = "approved (bot-only — no human formal sign-off)";
|
||||||
|
} else if (mergeReadyState) {
|
||||||
|
status = "ready to merge";
|
||||||
|
} else {
|
||||||
|
status = "in review";
|
||||||
|
}
|
||||||
|
return { status, blockers };
|
||||||
|
}
|
||||||
|
|
||||||
|
function buildAssignmentEntries(
|
||||||
|
facts: PrFacts,
|
||||||
|
events: ReadonlyArray<GhAssignmentEvent>,
|
||||||
|
tags: ReadonlyArray<Tag>,
|
||||||
|
now: number,
|
||||||
|
): AssignmentEntry[] {
|
||||||
|
const eventIndex = indexAssignmentEvents(events);
|
||||||
|
const stateBadges = deriveStateBadges(facts);
|
||||||
|
const { status, blockers } = deriveStatus(tags, facts);
|
||||||
|
|
||||||
|
return facts.assignees.map((assignee) => {
|
||||||
|
const event = eventIndex.get(assignee) ?? null;
|
||||||
|
const assignedAt = event?.createdAt ?? null;
|
||||||
|
const assignedBy = event?.actor?.login ?? null;
|
||||||
|
const selfAssigned = assignedBy !== null && assignedBy === assignee;
|
||||||
|
const assignedHoursAgo = assignedAt === null ? null : hoursBetween(assignedAt, now);
|
||||||
|
|
||||||
|
const lastActivity = lastAssigneeActivityAt(facts, assignee);
|
||||||
|
let idleSinceAt: string | null = null;
|
||||||
|
if (assignedAt !== null && lastActivity !== null) {
|
||||||
|
idleSinceAt =
|
||||||
|
Date.parse(assignedAt) > Date.parse(lastActivity) ? assignedAt : lastActivity;
|
||||||
|
} else if (assignedAt !== null) {
|
||||||
|
idleSinceAt = assignedAt;
|
||||||
|
} else if (lastActivity !== null) {
|
||||||
|
idleSinceAt = lastActivity;
|
||||||
|
}
|
||||||
|
const idleHours = idleSinceAt === null ? null : hoursBetween(idleSinceAt, now);
|
||||||
|
|
||||||
|
return {
|
||||||
|
number: facts.number,
|
||||||
|
title: facts.title,
|
||||||
|
assignee,
|
||||||
|
assignedAt,
|
||||||
|
assignedBy,
|
||||||
|
selfAssigned,
|
||||||
|
assignedHoursAgo,
|
||||||
|
idleSinceAt,
|
||||||
|
idleHours,
|
||||||
|
stateBadges,
|
||||||
|
status,
|
||||||
|
blockers,
|
||||||
|
tags: [...tags],
|
||||||
|
isDraft: facts.isDraft,
|
||||||
|
};
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---- report assembly ----------------------------------------------------
|
||||||
|
|
||||||
|
function buildReport(
|
||||||
|
allFacts: PrFacts[],
|
||||||
|
timelines: Map<number, ReadonlyArray<GhAssignmentEvent>>,
|
||||||
|
ctx: ClassifyContext,
|
||||||
|
): AssignmentReport {
|
||||||
|
const now = Date.now();
|
||||||
|
const byAssignee: Record<string, AssignmentEntry[]> = {};
|
||||||
|
const unassigned: AssignmentReport["unassigned"] = [];
|
||||||
|
let assignedCount = 0;
|
||||||
|
|
||||||
|
for (const facts of allFacts) {
|
||||||
|
const events = timelines.get(facts.number) ?? [];
|
||||||
|
const tags = classifyPr(facts, ctx);
|
||||||
|
if (facts.assignees.length === 0) {
|
||||||
|
const { status, blockers } = deriveStatus(tags, facts);
|
||||||
|
unassigned.push({
|
||||||
|
number: facts.number,
|
||||||
|
title: facts.title,
|
||||||
|
stateBadges: deriveStateBadges(facts),
|
||||||
|
status,
|
||||||
|
blockers,
|
||||||
|
isDraft: facts.isDraft,
|
||||||
|
});
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
assignedCount += 1;
|
||||||
|
const entries = buildAssignmentEntries(facts, events, tags, now);
|
||||||
|
for (const entry of entries) {
|
||||||
|
const bucket = byAssignee[entry.assignee] ?? [];
|
||||||
|
bucket.push(entry);
|
||||||
|
byAssignee[entry.assignee] = bucket;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Sort each assignee bucket by idle desc (most stale at top)
|
||||||
|
for (const login of Object.keys(byAssignee)) {
|
||||||
|
byAssignee[login]?.sort((a, b) => (b.idleHours ?? -1) - (a.idleHours ?? -1));
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
generatedAt: new Date().toISOString(),
|
||||||
|
openPrTotal: allFacts.length,
|
||||||
|
assignedCount,
|
||||||
|
unassignedCount: unassigned.length,
|
||||||
|
byAssignee,
|
||||||
|
unassigned,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---- formatting ---------------------------------------------------------
|
||||||
|
|
||||||
|
function formatHumanReport(
|
||||||
|
report: AssignmentReport,
|
||||||
|
options: { meLogin: string | null; showUnassignedDetail: boolean; userFilter: string | null },
|
||||||
|
): string {
|
||||||
|
const lines: string[] = [];
|
||||||
|
const header = `PR assignment overview — ${report.openPrTotal} open PRs · ${report.assignedCount} assigned · ${report.unassignedCount} unassigned`;
|
||||||
|
lines.push(header);
|
||||||
|
lines.push("");
|
||||||
|
|
||||||
|
const buckets = Object.entries(report.byAssignee).sort((a, b) => {
|
||||||
|
// me first, then by PR count desc, then alphabetic
|
||||||
|
if (options.meLogin && a[0] === options.meLogin) return -1;
|
||||||
|
if (options.meLogin && b[0] === options.meLogin) return 1;
|
||||||
|
if (b[1].length !== a[1].length) return b[1].length - a[1].length;
|
||||||
|
return a[0].localeCompare(b[0]);
|
||||||
|
});
|
||||||
|
|
||||||
|
for (const [login, entries] of buckets) {
|
||||||
|
if (options.userFilter !== null && login !== options.userFilter) continue;
|
||||||
|
const youTag = options.meLogin && login === options.meLogin ? " (you)" : "";
|
||||||
|
lines.push(`▌ ${login}${youTag} · ${entries.length} PR${entries.length === 1 ? "" : "s"}`);
|
||||||
|
lines.push("");
|
||||||
|
for (const entry of entries) {
|
||||||
|
const num = String(entry.number).padStart(5, " ");
|
||||||
|
const title = truncate(entry.title, 64);
|
||||||
|
const draftSuffix = entry.isDraft ? " [draft]" : "";
|
||||||
|
lines.push(` #${num} ${title}${draftSuffix}`);
|
||||||
|
const assignedAgo = entry.assignedHoursAgo === null
|
||||||
|
? "assignment > 20 timeline events ago (unknown exact time)"
|
||||||
|
: `assigned ${formatDuration(entry.assignedHoursAgo)} ago`;
|
||||||
|
const by = entry.assignedBy
|
||||||
|
? entry.selfAssigned
|
||||||
|
? " (self-assigned)"
|
||||||
|
: ` by ${entry.assignedBy}`
|
||||||
|
: "";
|
||||||
|
const idle = entry.idleHours === null ? "" : ` · idle ${formatDuration(entry.idleHours)}`;
|
||||||
|
lines.push(` ${assignedAgo}${by}${idle}`);
|
||||||
|
lines.push(` state: ${entry.stateBadges.join(" · ")}`);
|
||||||
|
lines.push(` status: ${entry.status}`);
|
||||||
|
for (const blocker of entry.blockers) lines.push(` - ${blocker}`);
|
||||||
|
lines.push("");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (options.userFilter === null) {
|
||||||
|
if (options.showUnassignedDetail) {
|
||||||
|
lines.push(`▌ (unassigned) · ${report.unassigned.length} PRs`);
|
||||||
|
lines.push("");
|
||||||
|
for (const entry of report.unassigned) {
|
||||||
|
const num = String(entry.number).padStart(5, " ");
|
||||||
|
const title = truncate(entry.title, 64);
|
||||||
|
const draftSuffix = entry.isDraft ? " [draft]" : "";
|
||||||
|
lines.push(` #${num} ${title}${draftSuffix}`);
|
||||||
|
lines.push(` state: ${entry.stateBadges.join(" · ")} status: ${entry.status}`);
|
||||||
|
for (const blocker of entry.blockers) lines.push(` - ${blocker}`);
|
||||||
|
}
|
||||||
|
lines.push("");
|
||||||
|
} else {
|
||||||
|
lines.push(`▌ (unassigned) · ${report.unassignedCount} PRs`);
|
||||||
|
lines.push(" → see \`tools-pr assignment --unassigned\` for the full list");
|
||||||
|
lines.push("");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return lines.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---- command entry ------------------------------------------------------
|
||||||
|
|
||||||
|
export type AssignmentOptions = {
|
||||||
|
json?: boolean;
|
||||||
|
user?: string;
|
||||||
|
unassigned?: boolean;
|
||||||
|
includeDrafts?: boolean;
|
||||||
|
limit?: number | string;
|
||||||
|
};
|
||||||
|
|
||||||
|
export async function runAssignment(options: AssignmentOptions): Promise<void> {
|
||||||
|
// Default covers the whole open queue with growth headroom; `gh pr list`
|
||||||
|
// paginates internally so a high cap is cheap.
|
||||||
|
const limit = Math.max(1, Number(options.limit ?? 1000) || 1000);
|
||||||
|
const { owner } = await detectRepoSlug();
|
||||||
|
const userFilterRaw = options.user;
|
||||||
|
const meLoginPromise = userFilterRaw === "me" ? fetchCurrentUser() : Promise.resolve(null);
|
||||||
|
const [fetched, orgMembers, timelines, meLogin] = await Promise.all([
|
||||||
|
fetchOpenPrs(limit, { includeCommits: true, includeComments: true }),
|
||||||
|
fetchOrgMembers(owner),
|
||||||
|
fetchOpenPrAssignmentTimelines(),
|
||||||
|
meLoginPromise,
|
||||||
|
]);
|
||||||
|
|
||||||
|
// Reuse classify's fact-building. Inline here to avoid a circular import.
|
||||||
|
const statsBy = new Map(fetched.stats.map((row) => [row.number, row] as const));
|
||||||
|
const filesBy = new Map(fetched.files.map((row) => [row.number, row] as const));
|
||||||
|
const reviewsBy = new Map(fetched.reviews.map((row) => [row.number, row] as const));
|
||||||
|
const commitsBy = new Map((fetched.commits ?? []).map((row) => [row.number, row] as const));
|
||||||
|
const commentsBy = new Map((fetched.comments ?? []).map((row) => [row.number, row] as const));
|
||||||
|
|
||||||
|
const allFacts: PrFacts[] = fetched.meta
|
||||||
|
.filter((meta) => options.includeDrafts || !meta.isDraft)
|
||||||
|
.map((meta) => {
|
||||||
|
const stats = statsBy.get(meta.number);
|
||||||
|
const filesRow = filesBy.get(meta.number);
|
||||||
|
const reviewsRow = reviewsBy.get(meta.number);
|
||||||
|
const commitsRow = commitsBy.get(meta.number);
|
||||||
|
const commentsRow = commentsBy.get(meta.number);
|
||||||
|
return {
|
||||||
|
number: meta.number,
|
||||||
|
author: meta.author.login,
|
||||||
|
title: meta.title,
|
||||||
|
createdAt: meta.createdAt,
|
||||||
|
updatedAt: meta.updatedAt,
|
||||||
|
isDraft: meta.isDraft,
|
||||||
|
reviewDecision: meta.reviewDecision,
|
||||||
|
mergeStateStatus: stats?.mergeStateStatus ?? "UNKNOWN",
|
||||||
|
maintainerCanModify: meta.maintainerCanModify,
|
||||||
|
isOrgMember: orgMembers.has(meta.author.login),
|
||||||
|
headRefOid: stats?.headRefOid ?? "",
|
||||||
|
assignees: meta.assignees.map((a) => a.login),
|
||||||
|
labels: meta.labels,
|
||||||
|
filePaths: filesRow ? filesRow.files.map((f) => f.path) : [],
|
||||||
|
reviews: reviewsRow
|
||||||
|
? reduceLatestReviewsByAuthor(reviewsRow.reviews).map((r) => ({
|
||||||
|
author: r.author,
|
||||||
|
body: r.body,
|
||||||
|
state: r.state,
|
||||||
|
submittedAt: r.submittedAt,
|
||||||
|
commit: r.commit ?? null,
|
||||||
|
}))
|
||||||
|
: [],
|
||||||
|
comments: commentsRow ? commentsRow.comments : [],
|
||||||
|
commits: commitsRow
|
||||||
|
? commitsRow.commits.map((c) => ({
|
||||||
|
committedDate: c.committedDate,
|
||||||
|
authorLogin: c.authors[0]?.login ?? null,
|
||||||
|
}))
|
||||||
|
: [],
|
||||||
|
};
|
||||||
|
});
|
||||||
|
const ctx = buildContext(allFacts);
|
||||||
|
|
||||||
|
const timelineMap = new Map<number, ReadonlyArray<GhAssignmentEvent>>();
|
||||||
|
for (const t of timelines) timelineMap.set(t.number, t.events);
|
||||||
|
|
||||||
|
const report = buildReport(allFacts, timelineMap, ctx);
|
||||||
|
|
||||||
|
if (options.json) {
|
||||||
|
process.stdout.write(`${JSON.stringify(report, null, 2)}\n`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Resolve user filter for display: "me" expands to current login.
|
||||||
|
let userFilter: string | null = null;
|
||||||
|
if (userFilterRaw !== undefined) {
|
||||||
|
userFilter = userFilterRaw === "me" ? meLogin : userFilterRaw;
|
||||||
|
}
|
||||||
|
|
||||||
|
process.stdout.write(
|
||||||
|
`${formatHumanReport(report, {
|
||||||
|
meLogin: meLogin,
|
||||||
|
showUnassignedDetail: Boolean(options.unassigned),
|
||||||
|
userFilter,
|
||||||
|
})}\n`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
78
tools/pr/src/bot.ts
Normal file
78
tools/pr/src/bot.ts
Normal file
|
|
@ -0,0 +1,78 @@
|
||||||
|
/**
|
||||||
|
* Bot detection for review / comment bodies.
|
||||||
|
*
|
||||||
|
* Some maintainer accounts in this repo (e.g. mrcfps) operate partly as a
|
||||||
|
* Looper auto-review bot — the user account is a real MEMBER but the review
|
||||||
|
* body is generated by Looper and carries an HTML-comment stamp. We need to
|
||||||
|
* surface that distinction because GitHub's `reviewDecision === "APPROVED"`
|
||||||
|
* can otherwise land a PR in the `merge-ready` bucket with zero formal human
|
||||||
|
* sign-off.
|
||||||
|
*/
|
||||||
|
|
||||||
|
const BOT_MARKERS = [
|
||||||
|
/<!--\s*looper:/i,
|
||||||
|
/Powered by\s*<a[^>]*>Looper<\/a>/i,
|
||||||
|
/\[bot\]/i,
|
||||||
|
];
|
||||||
|
|
||||||
|
export function isBotAuthored(
|
||||||
|
actor: { login: string } | null,
|
||||||
|
body: string,
|
||||||
|
): boolean {
|
||||||
|
if (actor && actor.login.toLowerCase().endsWith("[bot]")) return true;
|
||||||
|
return BOT_MARKERS.some((rx) => rx.test(body));
|
||||||
|
}
|
||||||
|
|
||||||
|
export function stripBotMarkers(body: string): string {
|
||||||
|
let stripped = body;
|
||||||
|
stripped = stripped.replace(/<!--\s*looper:[\s\S]*?-->/gi, "");
|
||||||
|
stripped = stripped.replace(/<sub>[\s\S]*?Looper[\s\S]*?<\/sub>/gi, "");
|
||||||
|
return stripped.trim();
|
||||||
|
}
|
||||||
|
|
||||||
|
export function condense(body: string, max: number): string {
|
||||||
|
const cleaned = stripBotMarkers(body).replace(/\s+/g, " ").trim();
|
||||||
|
return cleaned.length <= max ? cleaned : `${cleaned.slice(0, max - 1)}…`;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* True when the PR's review-decision is APPROVED but every APPROVED-state
|
||||||
|
* review is bot-authored — i.e. no human reviewer formally signed off. We
|
||||||
|
* only flag when reviewDecision is APPROVED; for any other decision the
|
||||||
|
* notion doesn't apply.
|
||||||
|
*/
|
||||||
|
export function isBotOnlyApproval(
|
||||||
|
reviewDecision: string,
|
||||||
|
reviews: ReadonlyArray<{ author: { login: string } | null; body: string; state: string }>,
|
||||||
|
): boolean {
|
||||||
|
if (reviewDecision !== "APPROVED") return false;
|
||||||
|
const approvedReviews = reviews.filter((r) => r.state === "APPROVED");
|
||||||
|
if (approvedReviews.length === 0) return false;
|
||||||
|
return approvedReviews.every((r) => isBotAuthored(r.author, r.body));
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Reduces a PR's full review history to the most recent review per reviewer
|
||||||
|
* (by `submittedAt`). Anonymous / null-author entries are dropped. The
|
||||||
|
* result mirrors what `gh pr view --json latestReviews` returns, except it
|
||||||
|
* preserves the `commit.oid` field on each entry — which `latestReviews`
|
||||||
|
* does not expose. Detectors that ask "what is each reviewer's current
|
||||||
|
* stance?" should consume this output.
|
||||||
|
*/
|
||||||
|
export function reduceLatestReviewsByAuthor<
|
||||||
|
T extends {
|
||||||
|
author: { login: string } | null;
|
||||||
|
submittedAt: string;
|
||||||
|
},
|
||||||
|
>(history: ReadonlyArray<T>): T[] {
|
||||||
|
const byAuthor = new Map<string, T>();
|
||||||
|
for (const review of history) {
|
||||||
|
const login = review.author?.login;
|
||||||
|
if (!login) continue;
|
||||||
|
const existing = byAuthor.get(login);
|
||||||
|
if (!existing || existing.submittedAt < review.submittedAt) {
|
||||||
|
byAuthor.set(login, review);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return [...byAuthor.values()];
|
||||||
|
}
|
||||||
278
tools/pr/src/classify.ts
Normal file
278
tools/pr/src/classify.ts
Normal file
|
|
@ -0,0 +1,278 @@
|
||||||
|
/**
|
||||||
|
* `tools-pr classify` — emit script-level tags for one PR or the full open
|
||||||
|
* queue. Output is strictly factual: each tag carries a name + factual
|
||||||
|
* reason + source token; no severity, no judgment, no merge guidance.
|
||||||
|
*
|
||||||
|
* Tag dictionary lives in `tools/pr/src/tags.ts` and is documented in
|
||||||
|
* `tools/pr/AGENTS.md` §Tag dictionary.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { mkdir, writeFile } from "node:fs/promises";
|
||||||
|
import path from "node:path";
|
||||||
|
|
||||||
|
import { reduceLatestReviewsByAuthor } from "./bot.js";
|
||||||
|
import {
|
||||||
|
detectRepoSlug,
|
||||||
|
fetchOpenPrs,
|
||||||
|
fetchOrgMembers,
|
||||||
|
fetchRateLimit,
|
||||||
|
fetchView,
|
||||||
|
type FetchOpenPrsResult,
|
||||||
|
type RateLimitSnapshot,
|
||||||
|
} from "./gh.js";
|
||||||
|
import { buildContext, classifyPr, KNOWN_TAGS } from "./tags.js";
|
||||||
|
import type {
|
||||||
|
ClassifyContext,
|
||||||
|
ClassifyReport,
|
||||||
|
GhView,
|
||||||
|
PrFacts,
|
||||||
|
Tag,
|
||||||
|
} from "./types.js";
|
||||||
|
|
||||||
|
// ---- fact bridging -------------------------------------------------------
|
||||||
|
|
||||||
|
function factsFromList(input: FetchOpenPrsResult, orgMembers: ReadonlySet<string>): PrFacts[] {
|
||||||
|
const statsBy = new Map(input.stats.map((row) => [row.number, row] as const));
|
||||||
|
const filesBy = new Map(input.files.map((row) => [row.number, row] as const));
|
||||||
|
const reviewsBy = new Map(input.reviews.map((row) => [row.number, row] as const));
|
||||||
|
const commitsBy = new Map((input.commits ?? []).map((row) => [row.number, row] as const));
|
||||||
|
const commentsBy = new Map((input.comments ?? []).map((row) => [row.number, row] as const));
|
||||||
|
|
||||||
|
return input.meta.map((meta) => {
|
||||||
|
const stats = statsBy.get(meta.number);
|
||||||
|
const filesRow = filesBy.get(meta.number);
|
||||||
|
const reviewsRow = reviewsBy.get(meta.number);
|
||||||
|
const commitsRow = commitsBy.get(meta.number);
|
||||||
|
const commentsRow = commentsBy.get(meta.number);
|
||||||
|
return {
|
||||||
|
number: meta.number,
|
||||||
|
author: meta.author.login,
|
||||||
|
title: meta.title,
|
||||||
|
createdAt: meta.createdAt,
|
||||||
|
updatedAt: meta.updatedAt,
|
||||||
|
isDraft: meta.isDraft,
|
||||||
|
reviewDecision: meta.reviewDecision,
|
||||||
|
mergeStateStatus: stats?.mergeStateStatus ?? "UNKNOWN",
|
||||||
|
maintainerCanModify: meta.maintainerCanModify,
|
||||||
|
isOrgMember: orgMembers.has(meta.author.login),
|
||||||
|
headRefOid: stats?.headRefOid ?? "",
|
||||||
|
assignees: meta.assignees.map((a) => a.login),
|
||||||
|
labels: meta.labels,
|
||||||
|
filePaths: filesRow ? filesRow.files.map((f) => f.path) : [],
|
||||||
|
reviews: reviewsRow
|
||||||
|
? reduceLatestReviewsByAuthor(reviewsRow.reviews).map((r) => ({
|
||||||
|
author: r.author,
|
||||||
|
body: r.body,
|
||||||
|
state: r.state,
|
||||||
|
submittedAt: r.submittedAt,
|
||||||
|
commit: r.commit ?? null,
|
||||||
|
}))
|
||||||
|
: [],
|
||||||
|
comments: commentsRow ? commentsRow.comments : [],
|
||||||
|
commits: commitsRow
|
||||||
|
? commitsRow.commits.map((c) => ({
|
||||||
|
committedDate: c.committedDate,
|
||||||
|
authorLogin: c.authors[0]?.login ?? null,
|
||||||
|
}))
|
||||||
|
: [],
|
||||||
|
};
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function factsFromView(num: number, view: GhView, orgMembers: ReadonlySet<string>): PrFacts {
|
||||||
|
return {
|
||||||
|
number: num,
|
||||||
|
author: view.author.login,
|
||||||
|
title: view.title,
|
||||||
|
createdAt: view.createdAt,
|
||||||
|
updatedAt: view.updatedAt,
|
||||||
|
isDraft: view.isDraft,
|
||||||
|
reviewDecision: view.reviewDecision,
|
||||||
|
mergeStateStatus: view.mergeStateStatus,
|
||||||
|
maintainerCanModify: view.maintainerCanModify,
|
||||||
|
isOrgMember: orgMembers.has(view.author.login),
|
||||||
|
headRefOid: view.headRefOid,
|
||||||
|
assignees: view.assignees.map((a) => a.login),
|
||||||
|
labels: view.labels,
|
||||||
|
filePaths: view.files.map((f) => f.path),
|
||||||
|
reviews: reduceLatestReviewsByAuthor(view.reviews).map((r) => ({
|
||||||
|
author: r.author,
|
||||||
|
body: r.body,
|
||||||
|
state: r.state,
|
||||||
|
submittedAt: r.submittedAt,
|
||||||
|
commit: r.commit ?? null,
|
||||||
|
})),
|
||||||
|
comments: view.comments.map((c) => ({
|
||||||
|
author: c.author,
|
||||||
|
body: c.body,
|
||||||
|
createdAt: c.createdAt,
|
||||||
|
})),
|
||||||
|
commits: view.commits.map((c) => ({
|
||||||
|
committedDate: c.committedDate,
|
||||||
|
authorLogin: c.authors[0]?.login ?? null,
|
||||||
|
})),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---- output paths --------------------------------------------------------
|
||||||
|
|
||||||
|
function timestampStem(): string {
|
||||||
|
// YYYY-MM-DDTHHmmssZ — colon-free for filesystem portability
|
||||||
|
const d = new Date();
|
||||||
|
const iso = d.toISOString().replace(/[-:]/g, "").replace(/\..+/, "");
|
||||||
|
// iso is "20260511T034500Z"; reinsert dashes for readability
|
||||||
|
return `${iso.slice(0, 4)}-${iso.slice(4, 6)}-${iso.slice(6, 8)}T${iso.slice(9, 15)}Z`;
|
||||||
|
}
|
||||||
|
|
||||||
|
function classifyOutputDir(): string {
|
||||||
|
// Per AGENTS.md §Default runtime files convention:
|
||||||
|
// <project-root>/.tmp/<source>/...; we own `tools-pr` as our source slot.
|
||||||
|
// import.meta.dirname is the dist directory when bundled; walk up to repo
|
||||||
|
// root via dist/.. = tools/pr -> tools -> root.
|
||||||
|
const here = import.meta.dirname;
|
||||||
|
// when bundled: tools/pr/dist/index.mjs -> dirname = tools/pr/dist
|
||||||
|
// when tsx-run: tools/pr/src/classify.ts -> dirname = tools/pr/src
|
||||||
|
const repoRoot = path.resolve(here, "..", "..", "..");
|
||||||
|
return path.join(repoRoot, ".tmp", "tools-pr", "classify");
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---- report assembly -----------------------------------------------------
|
||||||
|
|
||||||
|
function buildReport(
|
||||||
|
allFacts: PrFacts[],
|
||||||
|
ctx: ClassifyContext,
|
||||||
|
rateBefore: RateLimitSnapshot,
|
||||||
|
rateAfter: RateLimitSnapshot,
|
||||||
|
): ClassifyReport {
|
||||||
|
const byTag: Record<string, number[]> = {};
|
||||||
|
const byNumber: Record<string, Tag[]> = {};
|
||||||
|
for (const tagName of KNOWN_TAGS) byTag[tagName] = [];
|
||||||
|
|
||||||
|
for (const facts of allFacts) {
|
||||||
|
const tags = classifyPr(facts, ctx);
|
||||||
|
byNumber[String(facts.number)] = tags;
|
||||||
|
for (const tag of tags) {
|
||||||
|
const bucket = byTag[tag.name] ?? [];
|
||||||
|
bucket.push(facts.number);
|
||||||
|
byTag[tag.name] = bucket;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Drop tag buckets that ended up empty so the JSON stays tight.
|
||||||
|
for (const tagName of Object.keys(byTag)) {
|
||||||
|
if ((byTag[tagName] ?? []).length === 0) delete byTag[tagName];
|
||||||
|
}
|
||||||
|
|
||||||
|
const cost =
|
||||||
|
rateBefore.resetAt === rateAfter.resetAt
|
||||||
|
? rateBefore.remaining - rateAfter.remaining
|
||||||
|
: null;
|
||||||
|
|
||||||
|
return {
|
||||||
|
generatedAt: new Date().toISOString(),
|
||||||
|
openPrTotal: allFacts.length,
|
||||||
|
classifiedCount: allFacts.length,
|
||||||
|
byTag,
|
||||||
|
byNumber,
|
||||||
|
rate: {
|
||||||
|
before: rateBefore,
|
||||||
|
after: rateAfter,
|
||||||
|
cost,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---- human single-PR output ---------------------------------------------
|
||||||
|
|
||||||
|
function formatAwaitingDuration(hours: number): string {
|
||||||
|
const days = Math.floor(hours / 24);
|
||||||
|
const rem = hours - days * 24;
|
||||||
|
if (days === 0) return `${rem}h`;
|
||||||
|
if (rem === 0) return `${days}d`;
|
||||||
|
return `${days}d ${rem}h`;
|
||||||
|
}
|
||||||
|
|
||||||
|
function formatSinglePr(num: number, tags: Tag[]): string {
|
||||||
|
const lines: string[] = [];
|
||||||
|
lines.push(`PR #${num} — ${tags.length} tag${tags.length === 1 ? "" : "s"}`);
|
||||||
|
if (tags.length === 0) {
|
||||||
|
lines.push(" (no tags matched)");
|
||||||
|
} else {
|
||||||
|
for (const tag of tags) {
|
||||||
|
const suffix =
|
||||||
|
tag.awaitingHours !== undefined
|
||||||
|
? ` (awaiting ${formatAwaitingDuration(tag.awaitingHours)})`
|
||||||
|
: "";
|
||||||
|
lines.push(` • ${tag.name}${suffix}`);
|
||||||
|
lines.push(` reason: ${tag.reason}`);
|
||||||
|
lines.push(` source: ${tag.source}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return lines.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---- command entries -----------------------------------------------------
|
||||||
|
|
||||||
|
export type ClassifyOptions = {
|
||||||
|
json?: boolean;
|
||||||
|
all?: boolean;
|
||||||
|
name?: string;
|
||||||
|
print?: boolean;
|
||||||
|
limit?: number | string;
|
||||||
|
};
|
||||||
|
|
||||||
|
export async function runClassifyOne(num: number, options: ClassifyOptions): Promise<void> {
|
||||||
|
if (!Number.isFinite(num) || num <= 0) {
|
||||||
|
throw new Error("classify <num> requires a positive PR number");
|
||||||
|
}
|
||||||
|
const { owner } = await detectRepoSlug();
|
||||||
|
const [view, orgMembers] = await Promise.all([fetchView(num), fetchOrgMembers(owner)]);
|
||||||
|
const facts = factsFromView(num, view, orgMembers);
|
||||||
|
// Single-PR mode has no cross-PR title corpus; duplicate-title cannot fire.
|
||||||
|
const ctx: ClassifyContext = { titleIndexByAuthor: new Map() };
|
||||||
|
const tags = classifyPr(facts, ctx);
|
||||||
|
|
||||||
|
if (options.json) {
|
||||||
|
process.stdout.write(`${JSON.stringify({ number: num, tags }, null, 2)}\n`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
process.stdout.write(`${formatSinglePr(num, tags)}\n`);
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function runClassifyAll(options: ClassifyOptions): Promise<void> {
|
||||||
|
// Default covers the whole open queue with growth headroom; `gh pr list`
|
||||||
|
// paginates internally so a high cap is cheap.
|
||||||
|
const limit = Math.max(1, Number(options.limit ?? 1000) || 1000);
|
||||||
|
const { owner } = await detectRepoSlug();
|
||||||
|
const rateBefore = await fetchRateLimit();
|
||||||
|
const [fetched, orgMembers] = await Promise.all([
|
||||||
|
fetchOpenPrs(limit, { includeCommits: true, includeComments: true }),
|
||||||
|
fetchOrgMembers(owner),
|
||||||
|
]);
|
||||||
|
const rateAfter = await fetchRateLimit();
|
||||||
|
const allFacts = factsFromList(fetched, orgMembers);
|
||||||
|
const ctx = buildContext(allFacts);
|
||||||
|
const report = buildReport(allFacts, ctx, rateBefore, rateAfter);
|
||||||
|
|
||||||
|
const dir = classifyOutputDir();
|
||||||
|
await mkdir(dir, { recursive: true });
|
||||||
|
const stem = options.name && options.name.length > 0 ? options.name : timestampStem();
|
||||||
|
const outPath = path.join(dir, `${stem}.json`);
|
||||||
|
await writeFile(outPath, `${JSON.stringify(report, null, 2)}\n`, "utf8");
|
||||||
|
|
||||||
|
const tagSummary = Object.entries(report.byTag)
|
||||||
|
.map(([name, nums]) => `${name}=${nums.length}`)
|
||||||
|
.join(" ");
|
||||||
|
const rateSummary =
|
||||||
|
report.rate.cost !== null
|
||||||
|
? `rate cost=${report.rate.cost} remaining=${report.rate.after.remaining}/${report.rate.after.limit} reset=${report.rate.after.resetAt}`
|
||||||
|
: `rate remaining=${report.rate.after.remaining}/${report.rate.after.limit} (window rolled over; cost N/A)`;
|
||||||
|
process.stdout.write(
|
||||||
|
`wrote ${report.openPrTotal} entries to ${outPath}` +
|
||||||
|
(tagSummary ? ` [${tagSummary}]` : " [no tags matched]") +
|
||||||
|
` ${rateSummary}\n`,
|
||||||
|
);
|
||||||
|
if (options.print) {
|
||||||
|
process.stdout.write(`${JSON.stringify(report, null, 2)}\n`);
|
||||||
|
}
|
||||||
|
}
|
||||||
535
tools/pr/src/gh.ts
Normal file
535
tools/pr/src/gh.ts
Normal file
|
|
@ -0,0 +1,535 @@
|
||||||
|
/**
|
||||||
|
* Thin gh CLI wrappers. Combining many fields in a single `gh pr list --json`
|
||||||
|
* call hits GitHub GraphQL with HTTP 502 across this repo's open queue, so
|
||||||
|
* fetchOpenPrs splits the query into three chunks and joins them by number.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { execFile as execFileCallback } from "node:child_process";
|
||||||
|
import { promisify } from "node:util";
|
||||||
|
|
||||||
|
import type {
|
||||||
|
GhAssignmentEvent,
|
||||||
|
GhAssignmentTimeline,
|
||||||
|
GhCommentsLite,
|
||||||
|
GhCommitsLite,
|
||||||
|
GhFiles,
|
||||||
|
GhMeta,
|
||||||
|
GhReviewsLite,
|
||||||
|
GhStats,
|
||||||
|
GhView,
|
||||||
|
} from "./types.js";
|
||||||
|
|
||||||
|
const execFile = promisify(execFileCallback);
|
||||||
|
|
||||||
|
async function ghOnce<T>(args: string[]): Promise<T> {
|
||||||
|
const { stdout } = await execFile("gh", args, { maxBuffer: 64 * 1024 * 1024 });
|
||||||
|
return JSON.parse(stdout) as T;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Treat upstream-side flakes as transient: 502/503/504 from the GraphQL gateway,
|
||||||
|
// localized GraphQL "Something went wrong" responses (which gh surfaces as 502),
|
||||||
|
// and connection-level errors. Everything else (auth failure, 4xx, json parse
|
||||||
|
// errors, schema rejection) must surface to the caller — retry would just hide
|
||||||
|
// real problems.
|
||||||
|
function looksTransient(err: unknown): boolean {
|
||||||
|
if (!(err instanceof Error)) return false;
|
||||||
|
const stderr = (err as { stderr?: string }).stderr ?? "";
|
||||||
|
const haystack = `${stderr} ${err.message}`;
|
||||||
|
return /HTTP 5\d\d|GraphQL.*(?:502|503|504|timeout)|ECONN(?:REFUSED|RESET)|ETIMEDOUT|EAI_AGAIN/i.test(
|
||||||
|
haystack,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Minimum-touch debounce around gh CLI calls. Two retries (1s, 2s) cover the
|
||||||
|
// short-window GitHub gateway hiccups we've observed during PR-duty runs. Long
|
||||||
|
// outages still bubble up — this isn't an exponential-backoff resilience layer.
|
||||||
|
const RETRY_BACKOFF_MS: readonly number[] = [1000, 2000] as const;
|
||||||
|
|
||||||
|
export async function gh<T>(args: string[]): Promise<T> {
|
||||||
|
let lastError: unknown;
|
||||||
|
for (let attempt = 0; attempt <= RETRY_BACKOFF_MS.length; attempt++) {
|
||||||
|
try {
|
||||||
|
return await ghOnce<T>(args);
|
||||||
|
} catch (err) {
|
||||||
|
lastError = err;
|
||||||
|
if (!looksTransient(err) || attempt === RETRY_BACKOFF_MS.length) throw err;
|
||||||
|
const delay = RETRY_BACKOFF_MS[attempt] ?? 1000;
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, delay));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
throw lastError;
|
||||||
|
}
|
||||||
|
|
||||||
|
export type RateLimitSnapshot = {
|
||||||
|
remaining: number;
|
||||||
|
limit: number;
|
||||||
|
resetAt: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Loads the login set of a GitHub org's members in one paginated REST call
|
||||||
|
* (`gh api orgs/<org>/members --paginate`). Cached for the process lifetime.
|
||||||
|
*
|
||||||
|
* Used by the `org-member` classify tag to route operational nudges away
|
||||||
|
* from GitHub comments (those go to internal IM instead). Outside repo
|
||||||
|
* collaborators who are not org members are NOT included — they don't have
|
||||||
|
* internal IM access either.
|
||||||
|
*/
|
||||||
|
let orgMembersCache: { org: string; members: Set<string> } | null = null;
|
||||||
|
export async function fetchOrgMembers(org: string): Promise<Set<string>> {
|
||||||
|
if (orgMembersCache && orgMembersCache.org === org) return orgMembersCache.members;
|
||||||
|
const { stdout } = await execFile(
|
||||||
|
"gh",
|
||||||
|
["api", "--paginate", `orgs/${org}/members`, "--jq", ".[].login"],
|
||||||
|
{ maxBuffer: 16 * 1024 * 1024 },
|
||||||
|
);
|
||||||
|
const members = new Set(stdout.split("\n").filter((line) => line.length > 0));
|
||||||
|
orgMembersCache = { org, members };
|
||||||
|
return members;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Snapshot the current GraphQL rate-limit state. The query itself costs 1
|
||||||
|
* point — cheap enough to call twice around an expensive fetch to compute
|
||||||
|
* an exact delta cost.
|
||||||
|
*/
|
||||||
|
export async function fetchRateLimit(): Promise<RateLimitSnapshot> {
|
||||||
|
const { stdout } = await execFile(
|
||||||
|
"gh",
|
||||||
|
["api", "graphql", "-f", "query={ rateLimit { remaining limit resetAt } }"],
|
||||||
|
{ maxBuffer: 64 * 1024 },
|
||||||
|
);
|
||||||
|
const parsed = JSON.parse(stdout) as { data: { rateLimit: RateLimitSnapshot } };
|
||||||
|
return parsed.data.rateLimit;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resolves the current repo's `owner/name`. Cached for the process lifetime.
|
||||||
|
* `gh pr list` auto-detects the repo from the cwd; for `gh api graphql` we
|
||||||
|
* need to pass it explicitly.
|
||||||
|
*/
|
||||||
|
let repoSlugCache: { owner: string; name: string } | null = null;
|
||||||
|
export async function detectRepoSlug(): Promise<{ owner: string; name: string }> {
|
||||||
|
if (repoSlugCache) return repoSlugCache;
|
||||||
|
const { stdout } = await execFile("gh", ["repo", "view", "--json", "owner,name"], {
|
||||||
|
maxBuffer: 64 * 1024,
|
||||||
|
});
|
||||||
|
const parsed = JSON.parse(stdout) as { owner: { login: string }; name: string };
|
||||||
|
repoSlugCache = { owner: parsed.owner.login, name: parsed.name };
|
||||||
|
return repoSlugCache;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Generic cursor-paginated `pullRequests(states: OPEN, ...)` fetch via
|
||||||
|
* `gh api graphql`. Used for the heavy-payload chunks (reviews, comments)
|
||||||
|
* where a single 100-PR query was both data-heavy (≈ 500 KB) and more
|
||||||
|
* likely to hit transient gateway flakes. Pagination spreads the load and
|
||||||
|
* lets per-page retries recover narrowly. Page size is 30 — the GitHub
|
||||||
|
* default — and keeps each node-traversal cost well under any limit.
|
||||||
|
*
|
||||||
|
* `nodeFields` is the per-PR graphql selection (must start with `number`
|
||||||
|
* so callers can join by PR number downstream). The query template wraps
|
||||||
|
* it with the `pullRequests(...)` connection + `pageInfo` so callers don't
|
||||||
|
* need to know about cursors.
|
||||||
|
*/
|
||||||
|
const PR_LIST_PAGE_SIZE = 30;
|
||||||
|
|
||||||
|
type PaginatedPrPage<N> = {
|
||||||
|
data: {
|
||||||
|
repository: {
|
||||||
|
pullRequests: {
|
||||||
|
nodes: N[];
|
||||||
|
pageInfo: { hasNextPage: boolean; endCursor: string | null };
|
||||||
|
};
|
||||||
|
};
|
||||||
|
};
|
||||||
|
};
|
||||||
|
|
||||||
|
async function fetchPaginatedPrList<N>(nodeFields: string): Promise<N[]> {
|
||||||
|
const { owner, name } = await detectRepoSlug();
|
||||||
|
const query = `query($owner: String!, $name: String!, $first: Int!, $cursor: String) {
|
||||||
|
repository(owner: $owner, name: $name) {
|
||||||
|
pullRequests(states: OPEN, first: $first, after: $cursor, orderBy: { field: UPDATED_AT, direction: DESC }) {
|
||||||
|
nodes { ${nodeFields} }
|
||||||
|
pageInfo { hasNextPage endCursor }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`;
|
||||||
|
const all: N[] = [];
|
||||||
|
let cursor: string | null = null;
|
||||||
|
for (;;) {
|
||||||
|
const args = [
|
||||||
|
"api",
|
||||||
|
"graphql",
|
||||||
|
"-F",
|
||||||
|
`owner=${owner}`,
|
||||||
|
"-F",
|
||||||
|
`name=${name}`,
|
||||||
|
"-F",
|
||||||
|
`first=${PR_LIST_PAGE_SIZE}`,
|
||||||
|
];
|
||||||
|
if (cursor !== null) args.push("-F", `cursor=${cursor}`);
|
||||||
|
args.push("-f", `query=${query}`);
|
||||||
|
const page = await gh<PaginatedPrPage<N>>(args);
|
||||||
|
all.push(...page.data.repository.pullRequests.nodes);
|
||||||
|
const info = page.data.repository.pullRequests.pageInfo;
|
||||||
|
if (!info.hasNextPage) break;
|
||||||
|
cursor = info.endCursor;
|
||||||
|
}
|
||||||
|
return all;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Fetches per-PR commit timestamps via cursor-paginated `gh api graphql`.
|
||||||
|
* Capped at `commits(last: 5)` per PR to keep node count well under
|
||||||
|
* GitHub's 500,000 traversal limit; `gh pr list --json commits` blows the
|
||||||
|
* budget even at limit=50 because it traverses the full commits + authors
|
||||||
|
* connection.
|
||||||
|
*
|
||||||
|
* Pagination removes the prior `first: <CLI limit>` one-shot — GraphQL's
|
||||||
|
* connection page size is capped at 100 by the server, so the previous
|
||||||
|
* implementation would error at runtime when callers passed `--limit
|
||||||
|
* 200`. Reusing `fetchPaginatedPrList` keeps this fetch consistent with
|
||||||
|
* the other heavy chunks (reviews, comments, assignment timelines).
|
||||||
|
*/
|
||||||
|
|
||||||
|
async function fetchOpenPrReviews(): Promise<GhReviewsLite[]> {
|
||||||
|
type Node = {
|
||||||
|
number: number;
|
||||||
|
reviews: {
|
||||||
|
nodes: {
|
||||||
|
author: { login: string } | null;
|
||||||
|
body: string;
|
||||||
|
state: string;
|
||||||
|
submittedAt: string;
|
||||||
|
commit: { oid: string } | null;
|
||||||
|
}[];
|
||||||
|
};
|
||||||
|
};
|
||||||
|
// last: 30 captures the latest review per reviewer for any realistically
|
||||||
|
// iterated PR (the heaviest in this repo's queue is ~15 reviews across
|
||||||
|
// 4 rounds). The reducer in bot.ts picks the latest per author from
|
||||||
|
// whatever subset we fetched.
|
||||||
|
const rows = await fetchPaginatedPrList<Node>(
|
||||||
|
`number
|
||||||
|
reviews(last: 30) {
|
||||||
|
nodes {
|
||||||
|
author { login }
|
||||||
|
body
|
||||||
|
state
|
||||||
|
submittedAt
|
||||||
|
commit { oid }
|
||||||
|
}
|
||||||
|
}`,
|
||||||
|
);
|
||||||
|
return rows.map((row) => ({ number: row.number, reviews: row.reviews.nodes }));
|
||||||
|
}
|
||||||
|
|
||||||
|
async function fetchOpenPrComments(): Promise<GhCommentsLite[]> {
|
||||||
|
type CommentNode = {
|
||||||
|
author: { login: string } | null;
|
||||||
|
body: string;
|
||||||
|
createdAt: string;
|
||||||
|
};
|
||||||
|
type Node = {
|
||||||
|
number: number;
|
||||||
|
comments: { nodes: CommentNode[] };
|
||||||
|
reviewThreads: { nodes: { comments: { nodes: CommentNode[] } }[] };
|
||||||
|
};
|
||||||
|
// last: 30 covers the issue-thread signal detection (we only need recent
|
||||||
|
// comments to decide whether human-reviewer signal predates author
|
||||||
|
// signal). reviewThreads sweeps inline review-comment replies; PR
|
||||||
|
// authors and reviewers regularly answer inline and never touch the
|
||||||
|
// conversation tab, so missing this stream would let `awaiting-*`
|
||||||
|
// tag the wrong side. last: 20 / first: 20 caps the per-PR fanout.
|
||||||
|
const rows = await fetchPaginatedPrList<Node>(
|
||||||
|
`number
|
||||||
|
comments(last: 30) {
|
||||||
|
nodes {
|
||||||
|
author { login }
|
||||||
|
body
|
||||||
|
createdAt
|
||||||
|
}
|
||||||
|
}
|
||||||
|
reviewThreads(last: 20) {
|
||||||
|
nodes {
|
||||||
|
comments(first: 20) {
|
||||||
|
nodes {
|
||||||
|
author { login }
|
||||||
|
body
|
||||||
|
createdAt
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`,
|
||||||
|
);
|
||||||
|
return rows.map((row) => {
|
||||||
|
const inline = row.reviewThreads.nodes.flatMap((t) => t.comments.nodes);
|
||||||
|
return { number: row.number, comments: [...row.comments.nodes, ...inline] };
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Per-PR assignment lifecycle (ASSIGNED + UNASSIGNED timeline events). Only
|
||||||
|
* available through `gh api graphql` — `gh pr list --json` doesn't expose
|
||||||
|
* timeline data. The shape narrows the GraphQL union (TimelineItem) to the
|
||||||
|
* two relevant event types via inline fragments.
|
||||||
|
*
|
||||||
|
* Used by the `assignment` subcommand to derive: per-current-assignee, when
|
||||||
|
* were they assigned (latest ASSIGNED_EVENT not followed by UNASSIGNED_EVENT)
|
||||||
|
* and by whom. Other assignment-related signals (idle time, status) compose
|
||||||
|
* this with existing commits/comments data inside `assignment.ts`.
|
||||||
|
*/
|
||||||
|
async function fetchOpenPrAssignmentTimelines(): Promise<GhAssignmentTimeline[]> {
|
||||||
|
type Node = {
|
||||||
|
number: number;
|
||||||
|
timelineItems: {
|
||||||
|
nodes: Array<
|
||||||
|
| {
|
||||||
|
__typename: "AssignedEvent";
|
||||||
|
createdAt: string;
|
||||||
|
actor: { login: string } | null;
|
||||||
|
assignee:
|
||||||
|
| { __typename: "User"; login: string }
|
||||||
|
| { __typename: string }
|
||||||
|
| null;
|
||||||
|
}
|
||||||
|
| {
|
||||||
|
__typename: "UnassignedEvent";
|
||||||
|
createdAt: string;
|
||||||
|
actor: { login: string } | null;
|
||||||
|
assignee:
|
||||||
|
| { __typename: "User"; login: string }
|
||||||
|
| { __typename: string }
|
||||||
|
| null;
|
||||||
|
}
|
||||||
|
>;
|
||||||
|
};
|
||||||
|
};
|
||||||
|
const rows = await fetchPaginatedPrList<Node>(
|
||||||
|
`number
|
||||||
|
timelineItems(itemTypes: [ASSIGNED_EVENT, UNASSIGNED_EVENT], last: 20) {
|
||||||
|
nodes {
|
||||||
|
__typename
|
||||||
|
... on AssignedEvent {
|
||||||
|
createdAt
|
||||||
|
actor { login }
|
||||||
|
assignee {
|
||||||
|
__typename
|
||||||
|
... on User { login }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
... on UnassignedEvent {
|
||||||
|
createdAt
|
||||||
|
actor { login }
|
||||||
|
assignee {
|
||||||
|
__typename
|
||||||
|
... on User { login }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`,
|
||||||
|
);
|
||||||
|
return rows.map((row) => ({
|
||||||
|
number: row.number,
|
||||||
|
events: row.timelineItems.nodes.map<GhAssignmentEvent>((event) => {
|
||||||
|
const kind: GhAssignmentEvent["kind"] =
|
||||||
|
event.__typename === "AssignedEvent" ? "ASSIGNED" : "UNASSIGNED";
|
||||||
|
const assignee =
|
||||||
|
event.assignee && "login" in event.assignee
|
||||||
|
? { login: event.assignee.login }
|
||||||
|
: null;
|
||||||
|
return {
|
||||||
|
kind,
|
||||||
|
createdAt: event.createdAt,
|
||||||
|
actor: event.actor,
|
||||||
|
assignee,
|
||||||
|
};
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
|
||||||
|
export { fetchOpenPrAssignmentTimelines };
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resolves the authenticated gh CLI user's login. Cached for the process
|
||||||
|
* lifetime — used by `tools-pr assignment --user me` to expand the alias.
|
||||||
|
*/
|
||||||
|
let currentUserCache: string | null = null;
|
||||||
|
export async function fetchCurrentUser(): Promise<string> {
|
||||||
|
if (currentUserCache !== null) return currentUserCache;
|
||||||
|
const { stdout } = await execFile("gh", ["api", "user", "--jq", ".login"], {
|
||||||
|
maxBuffer: 64 * 1024,
|
||||||
|
});
|
||||||
|
const login = stdout.trim();
|
||||||
|
if (login.length === 0) throw new Error("gh api user returned no login");
|
||||||
|
currentUserCache = login;
|
||||||
|
return login;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function fetchOpenPrCommits(): Promise<GhCommitsLite[]> {
|
||||||
|
type Node = {
|
||||||
|
number: number;
|
||||||
|
commits: {
|
||||||
|
nodes: {
|
||||||
|
commit: {
|
||||||
|
committedDate: string;
|
||||||
|
author: { user: { login: string } | null } | null;
|
||||||
|
};
|
||||||
|
}[];
|
||||||
|
};
|
||||||
|
};
|
||||||
|
const rows = await fetchPaginatedPrList<Node>(
|
||||||
|
`number
|
||||||
|
commits(last: 5) {
|
||||||
|
nodes {
|
||||||
|
commit {
|
||||||
|
committedDate
|
||||||
|
author { user { login } }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`,
|
||||||
|
);
|
||||||
|
return rows.map((row) => ({
|
||||||
|
number: row.number,
|
||||||
|
commits: row.commits.nodes.map((entry) => ({
|
||||||
|
oid: "",
|
||||||
|
committedDate: entry.commit.committedDate,
|
||||||
|
authors: [{ login: entry.commit.author?.user?.login ?? null }],
|
||||||
|
})),
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
|
||||||
|
export type FetchOpenPrsOptions = {
|
||||||
|
includeCommits?: boolean;
|
||||||
|
includeComments?: boolean;
|
||||||
|
};
|
||||||
|
|
||||||
|
export type FetchOpenPrsResult = {
|
||||||
|
meta: GhMeta[];
|
||||||
|
stats: GhStats[];
|
||||||
|
files: GhFiles[];
|
||||||
|
reviews: GhReviewsLite[];
|
||||||
|
commits?: GhCommitsLite[];
|
||||||
|
comments?: GhCommentsLite[];
|
||||||
|
};
|
||||||
|
|
||||||
|
export async function fetchOpenPrs(
|
||||||
|
limit: number,
|
||||||
|
options: FetchOpenPrsOptions = {},
|
||||||
|
): Promise<FetchOpenPrsResult> {
|
||||||
|
const baseArgs = ["pr", "list", "--state", "open", "--limit", String(limit)];
|
||||||
|
const metaPromise = gh<GhMeta[]>([
|
||||||
|
...baseArgs,
|
||||||
|
"--json",
|
||||||
|
"number,title,author,createdAt,updatedAt,isDraft,reviewDecision,labels,maintainerCanModify,assignees",
|
||||||
|
]);
|
||||||
|
const statsPromise = gh<GhStats[]>([
|
||||||
|
...baseArgs,
|
||||||
|
"--json",
|
||||||
|
"number,additions,deletions,changedFiles,headRefName,headRefOid,baseRefName,mergeable,mergeStateStatus",
|
||||||
|
]);
|
||||||
|
const filesPromise = gh<GhFiles[]>([...baseArgs, "--json", "number,files"]);
|
||||||
|
const reviewsPromise = fetchOpenPrReviews();
|
||||||
|
const commitsPromise = options.includeCommits
|
||||||
|
? fetchOpenPrCommits()
|
||||||
|
: Promise.resolve<GhCommitsLite[] | undefined>(undefined);
|
||||||
|
const commentsPromise = options.includeComments
|
||||||
|
? fetchOpenPrComments()
|
||||||
|
: Promise.resolve<GhCommentsLite[] | undefined>(undefined);
|
||||||
|
|
||||||
|
const [meta, stats, files, reviews, commits, comments] = await Promise.all([
|
||||||
|
metaPromise,
|
||||||
|
statsPromise,
|
||||||
|
filesPromise,
|
||||||
|
reviewsPromise,
|
||||||
|
commitsPromise,
|
||||||
|
commentsPromise,
|
||||||
|
]);
|
||||||
|
|
||||||
|
const result: FetchOpenPrsResult = { meta, stats, files, reviews };
|
||||||
|
if (commits !== undefined) result.commits = commits;
|
||||||
|
if (comments !== undefined) result.comments = comments;
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
const VIEW_FIELDS = [
|
||||||
|
"url",
|
||||||
|
"title",
|
||||||
|
"body",
|
||||||
|
"isDraft",
|
||||||
|
"reviewDecision",
|
||||||
|
"mergeStateStatus",
|
||||||
|
"state",
|
||||||
|
"author",
|
||||||
|
"createdAt",
|
||||||
|
"updatedAt",
|
||||||
|
"labels",
|
||||||
|
"additions",
|
||||||
|
"deletions",
|
||||||
|
"changedFiles",
|
||||||
|
"baseRefName",
|
||||||
|
"headRefName",
|
||||||
|
"headRefOid",
|
||||||
|
"maintainerCanModify",
|
||||||
|
"assignees",
|
||||||
|
"files",
|
||||||
|
"statusCheckRollup",
|
||||||
|
"reviews",
|
||||||
|
"comments",
|
||||||
|
"commits",
|
||||||
|
].join(",");
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Inline review-thread comments — `pullRequest.comments` GraphQL (and
|
||||||
|
* `gh pr view --json comments`) only return the issue-conversation thread.
|
||||||
|
* REST `pulls/{n}/comments` returns the inline review-comment thread,
|
||||||
|
* which is where authors and reviewers actually exchange most fix-up
|
||||||
|
* replies. Used by `fetchView` to merge into `GhView.comments` so
|
||||||
|
* downstream `awaiting-*` and brief output see both streams.
|
||||||
|
*/
|
||||||
|
async function fetchInlineReviewComments(num: number): Promise<
|
||||||
|
Array<{
|
||||||
|
author: { login: string } | null;
|
||||||
|
authorAssociation: string;
|
||||||
|
body: string;
|
||||||
|
createdAt: string;
|
||||||
|
}>
|
||||||
|
> {
|
||||||
|
type Raw = {
|
||||||
|
user: { login: string } | null;
|
||||||
|
author_association: string;
|
||||||
|
body: string;
|
||||||
|
created_at: string;
|
||||||
|
};
|
||||||
|
const { owner, name } = await detectRepoSlug();
|
||||||
|
const { stdout } = await execFile(
|
||||||
|
"gh",
|
||||||
|
["api", "--paginate", `repos/${owner}/${name}/pulls/${num}/comments`],
|
||||||
|
{ maxBuffer: 64 * 1024 * 1024 },
|
||||||
|
);
|
||||||
|
const raw = JSON.parse(stdout) as Raw[];
|
||||||
|
return raw.map((c) => ({
|
||||||
|
author: c.user ? { login: c.user.login } : null,
|
||||||
|
authorAssociation: c.author_association ?? "",
|
||||||
|
body: c.body ?? "",
|
||||||
|
createdAt: c.created_at,
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function fetchView(num: number): Promise<GhView> {
|
||||||
|
const [view, inline] = await Promise.all([
|
||||||
|
gh<GhView>(["pr", "view", String(num), "--json", VIEW_FIELDS]),
|
||||||
|
fetchInlineReviewComments(num),
|
||||||
|
]);
|
||||||
|
return { ...view, comments: [...view.comments, ...inline] };
|
||||||
|
}
|
||||||
|
|
||||||
|
export function labelByPrefix(labels: { name: string }[], prefix: string): string | null {
|
||||||
|
const hit = labels.find((label) => label.name.startsWith(prefix));
|
||||||
|
return hit ? hit.name.slice(prefix.length) : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function daysSince(iso: string, now: number): number {
|
||||||
|
return Math.max(0, Math.floor((now - new Date(iso).getTime()) / (24 * 60 * 60 * 1000)));
|
||||||
|
}
|
||||||
74
tools/pr/src/index.ts
Normal file
74
tools/pr/src/index.ts
Normal file
|
|
@ -0,0 +1,74 @@
|
||||||
|
/**
|
||||||
|
* tools-pr — maintainer PR-duty control plane for nexu-io/open-design.
|
||||||
|
*
|
||||||
|
* Thin gh wrapper that encodes review knowledge specific to this repo: lane
|
||||||
|
* derivation, forbidden-surface flags, per-lane checklists, and validation
|
||||||
|
* command derivation from touched paths.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { cac } from "cac";
|
||||||
|
|
||||||
|
import { runAssignment, type AssignmentOptions } from "./assignment.js";
|
||||||
|
import {
|
||||||
|
runClassifyAll,
|
||||||
|
runClassifyOne,
|
||||||
|
type ClassifyOptions,
|
||||||
|
} from "./classify.js";
|
||||||
|
import { runList, type ListOptions } from "./list.js";
|
||||||
|
import { runView, type ViewOptions } from "./view.js";
|
||||||
|
|
||||||
|
const cli = cac("tools-pr");
|
||||||
|
|
||||||
|
cli
|
||||||
|
.command("list", "Triage open PRs by lane and review state")
|
||||||
|
.option("--json", "print JSON")
|
||||||
|
.option("--include-drafts", "include draft PRs")
|
||||||
|
.option("--limit <n>", "limit (default 1000; pass a smaller value to truncate)")
|
||||||
|
.option("--lane <list>", "lane filter: skill,design-system,craft,contract,docs,default,multi")
|
||||||
|
.option("--bucket <list>", "bucket filter: merge-ready,approved-blocked,needs-rebase,changes-requested,new,stale,draft")
|
||||||
|
.option("--author <list>", "author login filter (comma-separated)")
|
||||||
|
.action(async (options: ListOptions) => {
|
||||||
|
await runList(options);
|
||||||
|
});
|
||||||
|
|
||||||
|
cli
|
||||||
|
.command("view <num>", "Agent-friendly review brief for a single PR")
|
||||||
|
.option("--json", "print JSON")
|
||||||
|
.action(async (numStr: string, options: ViewOptions) => {
|
||||||
|
await runView(Number(numStr), options);
|
||||||
|
});
|
||||||
|
|
||||||
|
cli
|
||||||
|
.command(
|
||||||
|
"assignment",
|
||||||
|
"Assigner-perspective view of PR ownership — who has what, how long, what blockers",
|
||||||
|
)
|
||||||
|
.option("--json", "structured output to stdout")
|
||||||
|
.option("--user <login>", "filter to one assignee (login or 'me')")
|
||||||
|
.option("--unassigned", "list every unassigned PR (default collapses to count)")
|
||||||
|
.option("--include-drafts", "include draft PRs (default skips)")
|
||||||
|
.option("--limit <n>", "max open PRs to consider (default 1000)")
|
||||||
|
.action(async (options: AssignmentOptions) => {
|
||||||
|
await runAssignment(options);
|
||||||
|
});
|
||||||
|
|
||||||
|
cli
|
||||||
|
.command("classify [num]", "Emit script-level tags for one PR or the full open queue")
|
||||||
|
.option("--all", "classify the full open queue and write JSON to .tmp/tools-pr/classify/")
|
||||||
|
.option("--json", "with <num>: print JSON to stdout (default is human)")
|
||||||
|
.option("--name <stem>", "with --all: filename stem (default: ISO timestamp)")
|
||||||
|
.option("--print", "with --all: also print the report JSON to stdout")
|
||||||
|
.option("--limit <n>", "with --all: limit (default 1000; pass a smaller value to truncate)")
|
||||||
|
.action(async (numArg: string | undefined, options: ClassifyOptions) => {
|
||||||
|
if (options.all) {
|
||||||
|
await runClassifyAll(options);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (numArg === undefined) {
|
||||||
|
throw new Error("classify needs a PR number or --all");
|
||||||
|
}
|
||||||
|
await runClassifyOne(Number(numArg), options);
|
||||||
|
});
|
||||||
|
|
||||||
|
cli.help();
|
||||||
|
cli.parse();
|
||||||
86
tools/pr/src/lane.ts
Normal file
86
tools/pr/src/lane.ts
Normal file
|
|
@ -0,0 +1,86 @@
|
||||||
|
/**
|
||||||
|
* Lane derivation from touched paths, plus forbidden-surface and public-seam
|
||||||
|
* detection. Rules track docs/code-review-guidelines.md §2 (Forbidden
|
||||||
|
* surfaces) and §4 (review lanes).
|
||||||
|
*/
|
||||||
|
|
||||||
|
import type { ForbiddenHit, Lane } from "./types.js";
|
||||||
|
|
||||||
|
export const SKILL_DIR = /^skills\/[^/]+\//;
|
||||||
|
export const DESIGN_DIR = /^design-systems\/[^/]+\//;
|
||||||
|
export const CRAFT_DIR = /^craft\/[^/]+\.md$/;
|
||||||
|
|
||||||
|
const CONTRACT_PATHS = [
|
||||||
|
/^packages\/contracts\//,
|
||||||
|
/^packages\/sidecar-proto\//,
|
||||||
|
/^apps\/daemon\/src\/.*\/(routes|api|sse)\b/,
|
||||||
|
];
|
||||||
|
|
||||||
|
const DOCS_ONLY = [
|
||||||
|
/^README(\..+)?\.md$/,
|
||||||
|
/^CONTRIBUTING(\..+)?\.md$/,
|
||||||
|
/^QUICKSTART(\..+)?\.md$/,
|
||||||
|
/^CHANGELOG\.md$/,
|
||||||
|
/^TRANSLATIONS\.md$/,
|
||||||
|
/^docs\//,
|
||||||
|
];
|
||||||
|
|
||||||
|
// Files we count toward changedFiles but suppress from the top-N preview.
|
||||||
|
const NOISY_FILE_PATTERNS = [
|
||||||
|
/^pnpm-lock\.yaml$/,
|
||||||
|
/^CHANGELOG\.md$/,
|
||||||
|
/^README\.[a-zA-Z-]+\.md$/,
|
||||||
|
/^CONTRIBUTING\.[a-zA-Z-]+\.md$/,
|
||||||
|
/^QUICKSTART\.[a-zA-Z-]+\.md$/,
|
||||||
|
/^generated\//,
|
||||||
|
/^.*\.lock$/,
|
||||||
|
];
|
||||||
|
|
||||||
|
export function isNoisyFile(filePath: string): boolean {
|
||||||
|
return NOISY_FILE_PATTERNS.some((rx) => rx.test(filePath));
|
||||||
|
}
|
||||||
|
|
||||||
|
export function deriveLane(paths: string[]): { lane: Lane; hits: Set<Lane> } {
|
||||||
|
const hits = new Set<Lane>();
|
||||||
|
let allDocs = paths.length > 0;
|
||||||
|
for (const filePath of paths) {
|
||||||
|
if (SKILL_DIR.test(filePath)) hits.add("skill");
|
||||||
|
else if (DESIGN_DIR.test(filePath)) hits.add("design-system");
|
||||||
|
else if (CRAFT_DIR.test(filePath)) hits.add("craft");
|
||||||
|
else if (CONTRACT_PATHS.some((rx) => rx.test(filePath))) hits.add("contract");
|
||||||
|
if (!DOCS_ONLY.some((rx) => rx.test(filePath))) allDocs = false;
|
||||||
|
}
|
||||||
|
if (hits.size === 0 && allDocs) return { lane: "docs", hits: new Set(["docs"]) };
|
||||||
|
if (hits.size === 0) return { lane: "default", hits: new Set(["default"]) };
|
||||||
|
if (hits.size === 1) {
|
||||||
|
const [only] = [...hits];
|
||||||
|
return { lane: (only ?? "default") as Lane, hits };
|
||||||
|
}
|
||||||
|
return { lane: "multi", hits };
|
||||||
|
}
|
||||||
|
|
||||||
|
// Path-only forbidden surfaces. We do NOT check root `package.json` here —
|
||||||
|
// AGENTS.md §Root command boundary forbids specific *lifecycle* aliases
|
||||||
|
// (pnpm dev / pnpm test / pnpm build / …), not the file itself, and
|
||||||
|
// tools-control-plane entrypoints like `pnpm tools-pr` are explicitly
|
||||||
|
// allowed. Distinguishing "forbidden alias added" from "allowed entry
|
||||||
|
// added" requires reading the diff content, which is the role of
|
||||||
|
// `pnpm guard` rather than a path-derived classify tag.
|
||||||
|
export function deriveForbidden(paths: string[]): ForbiddenHit[] {
|
||||||
|
const hits: ForbiddenHit[] = [];
|
||||||
|
if (paths.some((p) => p.startsWith("apps/nextjs/"))) hits.push("restores-apps/nextjs");
|
||||||
|
if (paths.some((p) => p.startsWith("packages/shared/"))) hits.push("restores-packages/shared");
|
||||||
|
return hits;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function deriveSeams(paths: string[]): string[] {
|
||||||
|
const seams: string[] = [];
|
||||||
|
if (paths.some((p) => p.startsWith("packages/contracts/"))) seams.push("packages/contracts");
|
||||||
|
if (paths.some((p) => p.startsWith("packages/sidecar-proto/"))) seams.push("packages/sidecar-proto");
|
||||||
|
if (paths.some((p) => p.startsWith("apps/daemon/src/") && /(routes|api|sse|http)/i.test(p)))
|
||||||
|
seams.push("daemon HTTP/SSE routes");
|
||||||
|
if (paths.some((p) => /migration|schema|sql/i.test(p))) seams.push("persisted schema");
|
||||||
|
if (paths.some((p) => p === "pnpm-workspace.yaml")) seams.push("workspace layout");
|
||||||
|
if (paths.some((p) => p === "package.json")) seams.push("root package.json");
|
||||||
|
return seams;
|
||||||
|
}
|
||||||
259
tools/pr/src/list.ts
Normal file
259
tools/pr/src/list.ts
Normal file
|
|
@ -0,0 +1,259 @@
|
||||||
|
/**
|
||||||
|
* `tools-pr list` — triage queue scan.
|
||||||
|
*
|
||||||
|
* Classifies open PRs into review-state buckets (merge-ready, needs-rebase,
|
||||||
|
* changes-requested, new, stale, draft, approved-blocked) and lanes derived
|
||||||
|
* from touched paths. Outputs a grouped human report or JSON.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { isBotOnlyApproval, reduceLatestReviewsByAuthor } from "./bot.js";
|
||||||
|
import { daysSince, fetchOpenPrs, labelByPrefix } from "./gh.js";
|
||||||
|
import { deriveForbidden, deriveLane } from "./lane.js";
|
||||||
|
import type {
|
||||||
|
Bucket,
|
||||||
|
GhFiles,
|
||||||
|
GhMeta,
|
||||||
|
GhReviewsLite,
|
||||||
|
GhStats,
|
||||||
|
Lane,
|
||||||
|
Pr,
|
||||||
|
} from "./types.js";
|
||||||
|
|
||||||
|
const BUCKET_ORDER: Bucket[] = [
|
||||||
|
"merge-ready",
|
||||||
|
"approved-blocked",
|
||||||
|
"needs-rebase",
|
||||||
|
"changes-requested",
|
||||||
|
"new",
|
||||||
|
"stale",
|
||||||
|
"draft",
|
||||||
|
];
|
||||||
|
|
||||||
|
const LANE_ORDER: Lane[] = [
|
||||||
|
"contract",
|
||||||
|
"default",
|
||||||
|
"skill",
|
||||||
|
"design-system",
|
||||||
|
"craft",
|
||||||
|
"docs",
|
||||||
|
"multi",
|
||||||
|
"unknown",
|
||||||
|
];
|
||||||
|
|
||||||
|
function deriveBucket(args: {
|
||||||
|
isDraft: boolean;
|
||||||
|
reviewDecision: GhMeta["reviewDecision"];
|
||||||
|
mergeStateStatus: GhStats["mergeStateStatus"];
|
||||||
|
staleDays: number;
|
||||||
|
}): Bucket {
|
||||||
|
if (args.isDraft) return "draft";
|
||||||
|
if (args.reviewDecision === "APPROVED") {
|
||||||
|
if (args.mergeStateStatus === "CLEAN" || args.mergeStateStatus === "UNSTABLE") return "merge-ready";
|
||||||
|
return "approved-blocked";
|
||||||
|
}
|
||||||
|
if (args.mergeStateStatus === "DIRTY" || args.mergeStateStatus === "BEHIND") return "needs-rebase";
|
||||||
|
if (args.reviewDecision === "CHANGES_REQUESTED") return "changes-requested";
|
||||||
|
if (args.staleDays > 14) return "stale";
|
||||||
|
return "new";
|
||||||
|
}
|
||||||
|
|
||||||
|
function classify(input: {
|
||||||
|
meta: GhMeta[];
|
||||||
|
stats: GhStats[];
|
||||||
|
files: GhFiles[];
|
||||||
|
reviews: GhReviewsLite[];
|
||||||
|
}): Pr[] {
|
||||||
|
const now = Date.now();
|
||||||
|
const statsByNum = new Map(input.stats.map((row) => [row.number, row] as const));
|
||||||
|
const filesByNum = new Map(input.files.map((row) => [row.number, row] as const));
|
||||||
|
const reviewsByNum = new Map(input.reviews.map((row) => [row.number, row] as const));
|
||||||
|
|
||||||
|
return input.meta.map((meta) => {
|
||||||
|
const stats = statsByNum.get(meta.number);
|
||||||
|
const filesRow = filesByNum.get(meta.number);
|
||||||
|
const reviewsRow = reviewsByNum.get(meta.number);
|
||||||
|
const paths = filesRow ? filesRow.files.map((file) => file.path) : [];
|
||||||
|
const { lane, hits } = deriveLane(paths);
|
||||||
|
const ageDays = daysSince(meta.createdAt, now);
|
||||||
|
const staleDays = daysSince(meta.updatedAt, now);
|
||||||
|
const bucket = deriveBucket({
|
||||||
|
isDraft: meta.isDraft,
|
||||||
|
reviewDecision: meta.reviewDecision,
|
||||||
|
mergeStateStatus: stats?.mergeStateStatus ?? "UNKNOWN",
|
||||||
|
staleDays,
|
||||||
|
});
|
||||||
|
const latestPerReviewer = reviewsRow
|
||||||
|
? reduceLatestReviewsByAuthor(reviewsRow.reviews)
|
||||||
|
: [];
|
||||||
|
const botOnlyApproval = isBotOnlyApproval(meta.reviewDecision, latestPerReviewer);
|
||||||
|
return {
|
||||||
|
number: meta.number,
|
||||||
|
title: meta.title,
|
||||||
|
author: meta.author.login,
|
||||||
|
ageDays,
|
||||||
|
staleDays,
|
||||||
|
isDraft: meta.isDraft,
|
||||||
|
reviewDecision: meta.reviewDecision,
|
||||||
|
mergeStateStatus: stats?.mergeStateStatus ?? "UNKNOWN",
|
||||||
|
size: labelByPrefix(meta.labels, "size/"),
|
||||||
|
risk: labelByPrefix(meta.labels, "risk/"),
|
||||||
|
type: labelByPrefix(meta.labels, "type/"),
|
||||||
|
changedFiles: stats?.changedFiles ?? 0,
|
||||||
|
additions: stats?.additions ?? 0,
|
||||||
|
deletions: stats?.deletions ?? 0,
|
||||||
|
headRefName: stats?.headRefName ?? "",
|
||||||
|
baseRefName: stats?.baseRefName ?? "main",
|
||||||
|
lane,
|
||||||
|
laneHits: hits,
|
||||||
|
forbidden: deriveForbidden(paths),
|
||||||
|
bucket,
|
||||||
|
botOnlyApproval,
|
||||||
|
};
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function laneTag(lane: Lane): string {
|
||||||
|
switch (lane) {
|
||||||
|
case "contract": return "CONTRACT";
|
||||||
|
case "skill": return "SKILL";
|
||||||
|
case "design-system": return "DSGN-SYS";
|
||||||
|
case "craft": return "CRAFT";
|
||||||
|
case "docs": return "DOCS";
|
||||||
|
case "multi": return "MULTI";
|
||||||
|
case "default": return "DEFAULT";
|
||||||
|
case "unknown": return "UNKNOWN";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function truncate(s: string, n: number): string {
|
||||||
|
return s.length <= n ? s : `${s.slice(0, n - 1)}…`;
|
||||||
|
}
|
||||||
|
|
||||||
|
function formatHumanRow(pr: Pr): string {
|
||||||
|
const flags = [
|
||||||
|
pr.risk ? `risk:${pr.risk[0]}` : null,
|
||||||
|
pr.size ? `sz:${pr.size}` : null,
|
||||||
|
pr.type ? `t:${pr.type}` : null,
|
||||||
|
pr.forbidden.length > 0 ? `forbid:${pr.forbidden.length}` : null,
|
||||||
|
pr.botOnlyApproval ? "bot-only" : null,
|
||||||
|
]
|
||||||
|
.filter((v): v is string => v !== null)
|
||||||
|
.join(" ");
|
||||||
|
const num = String(pr.number).padStart(4, " ");
|
||||||
|
const lane = laneTag(pr.lane).padEnd(8, " ");
|
||||||
|
const age = `${String(pr.ageDays).padStart(2, " ")}d/${String(pr.staleDays).padStart(2, " ")}d`;
|
||||||
|
const author = truncate(pr.author, 16).padEnd(16, " ");
|
||||||
|
const title = truncate(pr.title, 64);
|
||||||
|
return ` #${num} ${lane} ${age} ${author} ${flags.padEnd(34, " ")} ${title}`;
|
||||||
|
}
|
||||||
|
|
||||||
|
function formatHumanReport(prs: Pr[], total: number): string {
|
||||||
|
const byBucket = new Map<Bucket, Pr[]>();
|
||||||
|
for (const pr of prs) {
|
||||||
|
const list = byBucket.get(pr.bucket) ?? [];
|
||||||
|
list.push(pr);
|
||||||
|
byBucket.set(pr.bucket, list);
|
||||||
|
}
|
||||||
|
|
||||||
|
const lines: string[] = [];
|
||||||
|
const countLine =
|
||||||
|
prs.length === total
|
||||||
|
? `open-design PR triage — ${total} open PRs`
|
||||||
|
: `open-design PR triage — showing ${prs.length} of ${total} open PRs`;
|
||||||
|
lines.push(countLine);
|
||||||
|
lines.push("");
|
||||||
|
|
||||||
|
for (const bucket of BUCKET_ORDER) {
|
||||||
|
const inBucket = byBucket.get(bucket);
|
||||||
|
if (!inBucket || inBucket.length === 0) continue;
|
||||||
|
lines.push(`▌ ${bucket} (${inBucket.length})`);
|
||||||
|
|
||||||
|
inBucket.sort((a, b) => {
|
||||||
|
const laneDelta = LANE_ORDER.indexOf(a.lane) - LANE_ORDER.indexOf(b.lane);
|
||||||
|
if (laneDelta !== 0) return laneDelta;
|
||||||
|
return a.staleDays - b.staleDays;
|
||||||
|
});
|
||||||
|
|
||||||
|
for (const pr of inBucket) lines.push(formatHumanRow(pr));
|
||||||
|
lines.push("");
|
||||||
|
}
|
||||||
|
|
||||||
|
const forbiddenPrs = prs.filter((pr) => pr.forbidden.length > 0);
|
||||||
|
if (forbiddenPrs.length > 0) {
|
||||||
|
lines.push(`▌ forbidden-surface hits (${forbiddenPrs.length})`);
|
||||||
|
for (const pr of forbiddenPrs) {
|
||||||
|
lines.push(` #${pr.number} ${pr.forbidden.join(", ")} ${truncate(pr.title, 60)}`);
|
||||||
|
}
|
||||||
|
lines.push("");
|
||||||
|
}
|
||||||
|
|
||||||
|
const botOnly = prs.filter((pr) => pr.botOnlyApproval);
|
||||||
|
if (botOnly.length > 0) {
|
||||||
|
lines.push(`▌ bot-only approval (${botOnly.length})`);
|
||||||
|
lines.push(" reviewDecision=APPROVED, but every APPROVED review is bot-authored");
|
||||||
|
for (const pr of botOnly) {
|
||||||
|
lines.push(` #${pr.number} ${truncate(pr.title, 70)}`);
|
||||||
|
}
|
||||||
|
lines.push("");
|
||||||
|
}
|
||||||
|
|
||||||
|
lines.push("legend: age = created/updated days ago lane = derived from touched paths");
|
||||||
|
lines.push(" risk / sz / t = gh label values (size/, risk/, type/ prefixes)");
|
||||||
|
lines.push(" forbid:N = N path matches against AGENTS.md §Forbidden surfaces");
|
||||||
|
lines.push(" bot-only = reviewDecision=APPROVED and every APPROVED review is bot-authored");
|
||||||
|
return lines.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
|
export type ListOptions = {
|
||||||
|
json?: boolean;
|
||||||
|
includeDrafts?: boolean;
|
||||||
|
limit?: number | string;
|
||||||
|
lane?: string;
|
||||||
|
bucket?: string;
|
||||||
|
author?: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
type Filters = {
|
||||||
|
lanes?: Set<Lane>;
|
||||||
|
buckets?: Set<Bucket>;
|
||||||
|
authors?: Set<string>;
|
||||||
|
includeDrafts: boolean;
|
||||||
|
};
|
||||||
|
|
||||||
|
function normalizeFilters(options: ListOptions): Filters {
|
||||||
|
const filters: Filters = { includeDrafts: Boolean(options.includeDrafts) };
|
||||||
|
if (options.lane) filters.lanes = new Set(options.lane.split(",") as Lane[]);
|
||||||
|
if (options.bucket) filters.buckets = new Set(options.bucket.split(",") as Bucket[]);
|
||||||
|
if (options.author) filters.authors = new Set(options.author.split(","));
|
||||||
|
return filters;
|
||||||
|
}
|
||||||
|
|
||||||
|
function applyFilters(prs: Pr[], filters: Filters): Pr[] {
|
||||||
|
return prs.filter((pr) => {
|
||||||
|
if (!filters.includeDrafts && pr.isDraft) return false;
|
||||||
|
if (filters.lanes && !filters.lanes.has(pr.lane)) return false;
|
||||||
|
if (filters.buckets && !filters.buckets.has(pr.bucket)) return false;
|
||||||
|
if (filters.authors && !filters.authors.has(pr.author)) return false;
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function runList(options: ListOptions): Promise<void> {
|
||||||
|
const limitRaw = options.limit;
|
||||||
|
// Default is large enough to cover this repo's open queue plus growth
|
||||||
|
// headroom; `gh pr list --limit N` paginates internally so high values
|
||||||
|
// are cheap. Users can pass `--limit <small>` for a truncated preview.
|
||||||
|
const limit = Math.max(1, Number(limitRaw ?? 1000) || 1000);
|
||||||
|
const filters = normalizeFilters(options);
|
||||||
|
|
||||||
|
const raw = await fetchOpenPrs(limit);
|
||||||
|
const classified = classify(raw);
|
||||||
|
const filtered = applyFilters(classified, filters);
|
||||||
|
|
||||||
|
if (options.json) {
|
||||||
|
const payload = filtered.map((pr) => ({ ...pr, laneHits: [...pr.laneHits] }));
|
||||||
|
process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
process.stdout.write(`${formatHumanReport(filtered, classified.length)}\n`);
|
||||||
|
}
|
||||||
287
tools/pr/src/tags.ts
Normal file
287
tools/pr/src/tags.ts
Normal file
|
|
@ -0,0 +1,287 @@
|
||||||
|
/**
|
||||||
|
* `tools-pr classify` tag detectors.
|
||||||
|
*
|
||||||
|
* Every detector returns either a Tag (fact + reason + source token) or
|
||||||
|
* null. Rules track this repo's AGENTS.md and code-review-guidelines.md;
|
||||||
|
* no judgment language enters the output. See
|
||||||
|
* `tools/pr/AGENTS.md` for the v1 tag dictionary.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { isBotAuthored, isBotOnlyApproval } from "./bot.js";
|
||||||
|
import { deriveForbidden, deriveLane, DESIGN_DIR } from "./lane.js";
|
||||||
|
import type { ClassifyContext, PrFacts, Tag } from "./types.js";
|
||||||
|
|
||||||
|
const AWAITING_THRESHOLD_HOURS = 24;
|
||||||
|
const HOUR_MS = 60 * 60 * 1000;
|
||||||
|
|
||||||
|
// ---- detectors ----------------------------------------------------------
|
||||||
|
|
||||||
|
function tagBotOnlyApproval(facts: PrFacts): Tag | null {
|
||||||
|
if (!isBotOnlyApproval(facts.reviewDecision, facts.reviews)) return null;
|
||||||
|
return {
|
||||||
|
name: "bot-only-approval",
|
||||||
|
reason: "reviewDecision=APPROVED; every APPROVED review is bot-authored",
|
||||||
|
source: "gh.reviewDecision+latestReviews",
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function tagNeedsRebase(facts: PrFacts): Tag | null {
|
||||||
|
if (facts.mergeStateStatus === "DIRTY" || facts.mergeStateStatus === "BEHIND") {
|
||||||
|
return {
|
||||||
|
name: "needs-rebase",
|
||||||
|
reason: `mergeStateStatus=${facts.mergeStateStatus}`,
|
||||||
|
source: "gh.mergeStateStatus",
|
||||||
|
};
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
function tagForbiddenSurface(facts: PrFacts): Tag | null {
|
||||||
|
const hits = deriveForbidden(facts.filePaths);
|
||||||
|
if (hits.length === 0) return null;
|
||||||
|
return {
|
||||||
|
name: "forbidden-surface",
|
||||||
|
reason: `path matches AGENTS.md §Forbidden surfaces: ${hits.join(", ")}`,
|
||||||
|
source: "files+lane.deriveForbidden",
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function tagUnlabeled(facts: PrFacts): Tag | null {
|
||||||
|
const names = facts.labels.map((label) => label.name);
|
||||||
|
const missing: string[] = [];
|
||||||
|
if (!names.some((n) => n.startsWith("size/"))) missing.push("size/");
|
||||||
|
if (!names.some((n) => n.startsWith("risk/"))) missing.push("risk/");
|
||||||
|
if (!names.some((n) => n.startsWith("type/"))) missing.push("type/");
|
||||||
|
if (missing.length === 0) return null;
|
||||||
|
return {
|
||||||
|
name: "unlabeled",
|
||||||
|
reason: `missing label prefixes: ${missing.join(", ")}`,
|
||||||
|
source: "gh.labels",
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function tagDuplicateTitle(facts: PrFacts, ctx: ClassifyContext): Tag | null {
|
||||||
|
const key = `${facts.author}\u0000${facts.title}`;
|
||||||
|
const siblings = ctx.titleIndexByAuthor.get(key);
|
||||||
|
if (!siblings || siblings.length < 2) return null;
|
||||||
|
const others = siblings.filter((n) => n !== facts.number);
|
||||||
|
if (others.length === 0) return null;
|
||||||
|
return {
|
||||||
|
name: "duplicate-title",
|
||||||
|
reason: `same author has another open PR(s) with byte-for-byte title: #${others.join(", #")}`,
|
||||||
|
source: "cross-pr.titleIndexByAuthor",
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
const ASCII_SLUG = /^[a-z0-9-]+$/;
|
||||||
|
|
||||||
|
function tagNonAsciiSlug(facts: PrFacts): Tag | null {
|
||||||
|
const { lane, hits } = deriveLane(facts.filePaths);
|
||||||
|
if (lane !== "design-system" && !hits.has("design-system")) return null;
|
||||||
|
const slugs = new Set(
|
||||||
|
facts.filePaths
|
||||||
|
.filter((p) => DESIGN_DIR.test(p))
|
||||||
|
.map((p) => p.split("/")[1])
|
||||||
|
.filter((s): s is string => typeof s === "string"),
|
||||||
|
);
|
||||||
|
const offenders = [...slugs].filter((s) => !ASCII_SLUG.test(s));
|
||||||
|
if (offenders.length === 0) return null;
|
||||||
|
return {
|
||||||
|
name: "non-ascii-slug",
|
||||||
|
reason: `design-system slug fails /^[a-z0-9-]+$/: ${offenders.join(", ")}`,
|
||||||
|
source: "files+lane.DESIGN_DIR",
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function tagMaintainerEditsDisabled(facts: PrFacts): Tag | null {
|
||||||
|
if (facts.maintainerCanModify) return null;
|
||||||
|
return {
|
||||||
|
name: "maintainer-edits-disabled",
|
||||||
|
reason: "maintainerCanModify=false on the fork — maintainers cannot push to the PR branch",
|
||||||
|
source: "gh.maintainerCanModify",
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function tagOrgMember(facts: PrFacts): Tag | null {
|
||||||
|
if (!facts.isOrgMember) return null;
|
||||||
|
return {
|
||||||
|
name: "org-member",
|
||||||
|
reason: `author ${facts.author} is a member of the repo's org`,
|
||||||
|
source: "gh api orgs/<org>/members",
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function tagUnresolvedChangesRequested(facts: PrFacts): Tag | null {
|
||||||
|
const reviewers = facts.reviews
|
||||||
|
.filter((r) => r.state === "CHANGES_REQUESTED" && r.author?.login)
|
||||||
|
.map((r) => r.author?.login)
|
||||||
|
.filter((login): login is string => typeof login === "string");
|
||||||
|
if (reviewers.length === 0) return null;
|
||||||
|
return {
|
||||||
|
name: "unresolved-changes-requested",
|
||||||
|
reason: `latestReviews carries CHANGES_REQUESTED from: ${[...new Set(reviewers)].join(", ")}`,
|
||||||
|
source: "gh.latestReviews[].state",
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function tagStaleApproval(facts: PrFacts): Tag | null {
|
||||||
|
if (!facts.headRefOid) return null;
|
||||||
|
const stale = facts.reviews
|
||||||
|
.filter((r) => r.state === "APPROVED")
|
||||||
|
.map((r) => {
|
||||||
|
const oid = r.commit?.oid;
|
||||||
|
if (!oid || oid.length === 0) return null;
|
||||||
|
if (oid === facts.headRefOid) return null;
|
||||||
|
return { login: r.author?.login ?? "(unknown)", oid };
|
||||||
|
})
|
||||||
|
.filter((entry): entry is { login: string; oid: string } => entry !== null);
|
||||||
|
if (stale.length === 0) return null;
|
||||||
|
const summary = stale
|
||||||
|
.map((entry) => `${entry.login}@${entry.oid.slice(0, 7)}`)
|
||||||
|
.join(", ");
|
||||||
|
return {
|
||||||
|
name: "stale-approval",
|
||||||
|
reason: `APPROVED review(s) at ${summary} predate current head ${facts.headRefOid.slice(0, 7)}`,
|
||||||
|
source: "gh.latestReviews[].commit.oid+gh.headRefOid",
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---- timing detectors ---------------------------------------------------
|
||||||
|
|
||||||
|
function authorSignalAt(facts: PrFacts): number | null {
|
||||||
|
const author = facts.author;
|
||||||
|
let max: number | null = null;
|
||||||
|
for (const c of facts.commits) {
|
||||||
|
// On maintainerCanModify=true PRs a maintainer can push a follow-up
|
||||||
|
// commit. Counting that as an author signal would flip
|
||||||
|
// `awaiting-author-response-*` off even though the author never replied.
|
||||||
|
if (c.authorLogin !== author) continue;
|
||||||
|
const t = Date.parse(c.committedDate);
|
||||||
|
if (Number.isFinite(t) && (max === null || t > max)) max = t;
|
||||||
|
}
|
||||||
|
for (const cmt of facts.comments) {
|
||||||
|
if (cmt.author?.login !== author) continue;
|
||||||
|
const t = Date.parse(cmt.createdAt);
|
||||||
|
if (Number.isFinite(t) && (max === null || t > max)) max = t;
|
||||||
|
}
|
||||||
|
return max;
|
||||||
|
}
|
||||||
|
|
||||||
|
function humanReviewerSignalAt(facts: PrFacts): number | null {
|
||||||
|
const author = facts.author;
|
||||||
|
let max: number | null = null;
|
||||||
|
for (const r of facts.reviews) {
|
||||||
|
const login = r.author?.login;
|
||||||
|
if (!login || login === author) continue;
|
||||||
|
if (isBotAuthored(r.author, r.body)) continue;
|
||||||
|
const t = Date.parse(r.submittedAt);
|
||||||
|
if (Number.isFinite(t) && (max === null || t > max)) max = t;
|
||||||
|
}
|
||||||
|
for (const c of facts.comments) {
|
||||||
|
const login = c.author?.login;
|
||||||
|
if (!login || login === author) continue;
|
||||||
|
if (isBotAuthored(c.author, c.body)) continue;
|
||||||
|
const t = Date.parse(c.createdAt);
|
||||||
|
if (Number.isFinite(t) && (max === null || t > max)) max = t;
|
||||||
|
}
|
||||||
|
return max;
|
||||||
|
}
|
||||||
|
|
||||||
|
function tagAwaitingAuthorResponse(facts: PrFacts): Tag | null {
|
||||||
|
const reviewer = humanReviewerSignalAt(facts);
|
||||||
|
const author = authorSignalAt(facts);
|
||||||
|
if (reviewer === null || author === null) return null;
|
||||||
|
if (reviewer <= author) return null;
|
||||||
|
const gapHours = Math.floor((Date.now() - reviewer) / HOUR_MS);
|
||||||
|
if (gapHours < AWAITING_THRESHOLD_HOURS) return null;
|
||||||
|
return {
|
||||||
|
name: `awaiting-author-response-${AWAITING_THRESHOLD_HOURS}h`,
|
||||||
|
reason: `latest human-reviewer signal (${new Date(reviewer).toISOString()}) is ${gapHours}h ago and newer than latest author signal (${new Date(author).toISOString()})`,
|
||||||
|
source: "latestReviews+comments+commits",
|
||||||
|
awaitingHours: gapHours,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function tagAwaitingReviewerResponse(facts: PrFacts): Tag | null {
|
||||||
|
const reviewer = humanReviewerSignalAt(facts);
|
||||||
|
const author = authorSignalAt(facts);
|
||||||
|
if (reviewer === null || author === null) return null;
|
||||||
|
if (author <= reviewer) return null;
|
||||||
|
const gapHours = Math.floor((Date.now() - author) / HOUR_MS);
|
||||||
|
if (gapHours < AWAITING_THRESHOLD_HOURS) return null;
|
||||||
|
return {
|
||||||
|
name: `awaiting-reviewer-response-${AWAITING_THRESHOLD_HOURS}h`,
|
||||||
|
reason: `latest author signal (${new Date(author).toISOString()}) is ${gapHours}h ago and newer than latest human-reviewer signal (${new Date(reviewer).toISOString()})`,
|
||||||
|
source: "latestReviews+comments+commits",
|
||||||
|
awaitingHours: gapHours,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function tagAwaitingFirstReview(facts: PrFacts): Tag | null {
|
||||||
|
const reviewer = humanReviewerSignalAt(facts);
|
||||||
|
if (reviewer !== null) return null;
|
||||||
|
const createdAt = Date.parse(facts.createdAt);
|
||||||
|
if (!Number.isFinite(createdAt)) return null;
|
||||||
|
const ageHours = Math.floor((Date.now() - createdAt) / HOUR_MS);
|
||||||
|
if (ageHours < AWAITING_THRESHOLD_HOURS) return null;
|
||||||
|
return {
|
||||||
|
name: `awaiting-first-review-${AWAITING_THRESHOLD_HOURS}h`,
|
||||||
|
reason: `no human review or non-author non-bot comment exists; createdAt is ${ageHours}h ago`,
|
||||||
|
source: "latestReviews+comments+createdAt",
|
||||||
|
awaitingHours: ageHours,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---- orchestrator -------------------------------------------------------
|
||||||
|
|
||||||
|
const DETECTORS: ReadonlyArray<(facts: PrFacts, ctx: ClassifyContext) => Tag | null> = [
|
||||||
|
tagBotOnlyApproval,
|
||||||
|
tagNeedsRebase,
|
||||||
|
tagForbiddenSurface,
|
||||||
|
tagUnlabeled,
|
||||||
|
tagDuplicateTitle,
|
||||||
|
tagNonAsciiSlug,
|
||||||
|
tagMaintainerEditsDisabled,
|
||||||
|
tagOrgMember,
|
||||||
|
tagUnresolvedChangesRequested,
|
||||||
|
tagStaleApproval,
|
||||||
|
tagAwaitingAuthorResponse,
|
||||||
|
tagAwaitingReviewerResponse,
|
||||||
|
tagAwaitingFirstReview,
|
||||||
|
];
|
||||||
|
|
||||||
|
export function classifyPr(facts: PrFacts, ctx: ClassifyContext): Tag[] {
|
||||||
|
const out: Tag[] = [];
|
||||||
|
for (const detector of DETECTORS) {
|
||||||
|
const tag = detector(facts, ctx);
|
||||||
|
if (tag !== null) out.push(tag);
|
||||||
|
}
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function buildContext(allFacts: PrFacts[]): ClassifyContext {
|
||||||
|
const titleIndexByAuthor = new Map<string, number[]>();
|
||||||
|
for (const facts of allFacts) {
|
||||||
|
const key = `${facts.author}\u0000${facts.title}`;
|
||||||
|
const existing = titleIndexByAuthor.get(key);
|
||||||
|
if (existing) existing.push(facts.number);
|
||||||
|
else titleIndexByAuthor.set(key, [facts.number]);
|
||||||
|
}
|
||||||
|
return { titleIndexByAuthor };
|
||||||
|
}
|
||||||
|
|
||||||
|
export const KNOWN_TAGS: readonly string[] = [
|
||||||
|
"bot-only-approval",
|
||||||
|
"needs-rebase",
|
||||||
|
"forbidden-surface",
|
||||||
|
"unlabeled",
|
||||||
|
"duplicate-title",
|
||||||
|
"non-ascii-slug",
|
||||||
|
"maintainer-edits-disabled",
|
||||||
|
"org-member",
|
||||||
|
"unresolved-changes-requested",
|
||||||
|
"stale-approval",
|
||||||
|
`awaiting-author-response-${AWAITING_THRESHOLD_HOURS}h`,
|
||||||
|
`awaiting-reviewer-response-${AWAITING_THRESHOLD_HOURS}h`,
|
||||||
|
`awaiting-first-review-${AWAITING_THRESHOLD_HOURS}h`,
|
||||||
|
] as const;
|
||||||
333
tools/pr/src/types.ts
Normal file
333
tools/pr/src/types.ts
Normal file
|
|
@ -0,0 +1,333 @@
|
||||||
|
/**
|
||||||
|
* Shared types for tools-pr.
|
||||||
|
*
|
||||||
|
* Lanes mirror docs/code-review-guidelines.md §4 (default / contract /
|
||||||
|
* design-system / skill / craft). Buckets are triage-state derived from
|
||||||
|
* review decision + merge state + staleness.
|
||||||
|
*/
|
||||||
|
|
||||||
|
export type Lane =
|
||||||
|
| "skill"
|
||||||
|
| "design-system"
|
||||||
|
| "craft"
|
||||||
|
| "contract"
|
||||||
|
| "docs"
|
||||||
|
| "default"
|
||||||
|
| "multi"
|
||||||
|
| "unknown";
|
||||||
|
|
||||||
|
export type Bucket =
|
||||||
|
| "merge-ready"
|
||||||
|
| "approved-blocked"
|
||||||
|
| "changes-requested"
|
||||||
|
| "needs-rebase"
|
||||||
|
| "new"
|
||||||
|
| "draft"
|
||||||
|
| "stale";
|
||||||
|
|
||||||
|
export type ForbiddenHit =
|
||||||
|
| "restores-apps/nextjs"
|
||||||
|
| "restores-packages/shared";
|
||||||
|
|
||||||
|
// --- gh `pr list --json` chunked shapes ----------------------------------
|
||||||
|
|
||||||
|
export type GhMeta = {
|
||||||
|
number: number;
|
||||||
|
title: string;
|
||||||
|
author: { login: string };
|
||||||
|
createdAt: string;
|
||||||
|
updatedAt: string;
|
||||||
|
isDraft: boolean;
|
||||||
|
reviewDecision: "" | "APPROVED" | "CHANGES_REQUESTED" | "REVIEW_REQUIRED" | "COMMENTED";
|
||||||
|
labels: { name: string }[];
|
||||||
|
maintainerCanModify: boolean;
|
||||||
|
assignees: { login: string }[];
|
||||||
|
};
|
||||||
|
|
||||||
|
export type GhStats = {
|
||||||
|
number: number;
|
||||||
|
additions: number;
|
||||||
|
deletions: number;
|
||||||
|
changedFiles: number;
|
||||||
|
headRefName: string;
|
||||||
|
headRefOid: string;
|
||||||
|
baseRefName: string;
|
||||||
|
mergeable: "MERGEABLE" | "CONFLICTING" | "UNKNOWN";
|
||||||
|
mergeStateStatus:
|
||||||
|
| "CLEAN"
|
||||||
|
| "BLOCKED"
|
||||||
|
| "BEHIND"
|
||||||
|
| "DIRTY"
|
||||||
|
| "UNKNOWN"
|
||||||
|
| "UNSTABLE"
|
||||||
|
| "HAS_HOOKS"
|
||||||
|
| "DRAFT";
|
||||||
|
};
|
||||||
|
|
||||||
|
export type GhFiles = {
|
||||||
|
number: number;
|
||||||
|
files: { path: string; additions: number; deletions: number }[];
|
||||||
|
};
|
||||||
|
|
||||||
|
export type GhReviewsLite = {
|
||||||
|
number: number;
|
||||||
|
/**
|
||||||
|
* Full review history (every state transition by every reviewer). Use
|
||||||
|
* `reduceLatestReviewsByAuthor` to collapse to the per-reviewer current
|
||||||
|
* state. The full history is needed because `gh pr list --json
|
||||||
|
* latestReviews` strips `commit.oid`, while `--json reviews` preserves
|
||||||
|
* it — that field is load-bearing for the `stale-approval` tag.
|
||||||
|
*/
|
||||||
|
reviews: {
|
||||||
|
author: { login: string } | null;
|
||||||
|
body: string;
|
||||||
|
state: string;
|
||||||
|
submittedAt: string;
|
||||||
|
commit?: { oid: string } | null;
|
||||||
|
}[];
|
||||||
|
};
|
||||||
|
|
||||||
|
export type GhCommitsLite = {
|
||||||
|
number: number;
|
||||||
|
commits: {
|
||||||
|
oid: string;
|
||||||
|
committedDate: string;
|
||||||
|
authors: { login: string | null }[];
|
||||||
|
}[];
|
||||||
|
};
|
||||||
|
|
||||||
|
export type GhCommentsLite = {
|
||||||
|
number: number;
|
||||||
|
comments: {
|
||||||
|
author: { login: string } | null;
|
||||||
|
body: string;
|
||||||
|
createdAt: string;
|
||||||
|
}[];
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Per-PR ASSIGNED_EVENT / UNASSIGNED_EVENT timeline entries fetched via
|
||||||
|
* `gh api graphql` (the only path that exposes `assignedAt` / `actor` for
|
||||||
|
* assignment lifecycle on a PR). `kind` distinguishes the two event types
|
||||||
|
* after the GraphQL union has been narrowed; `assignee` is the user the
|
||||||
|
* event targets (other actor types like Bot / Mannequin are filtered to
|
||||||
|
* null at fetch time).
|
||||||
|
*/
|
||||||
|
export type GhAssignmentEvent = {
|
||||||
|
kind: "ASSIGNED" | "UNASSIGNED";
|
||||||
|
createdAt: string;
|
||||||
|
actor: { login: string } | null;
|
||||||
|
assignee: { login: string } | null;
|
||||||
|
};
|
||||||
|
|
||||||
|
export type GhAssignmentTimeline = {
|
||||||
|
number: number;
|
||||||
|
events: GhAssignmentEvent[];
|
||||||
|
};
|
||||||
|
|
||||||
|
// --- gh `pr view --json` shape -------------------------------------------
|
||||||
|
|
||||||
|
export type GhFile = { path: string; additions: number; deletions: number; changeType: string };
|
||||||
|
|
||||||
|
export type GhReview = {
|
||||||
|
author: { login: string } | null;
|
||||||
|
authorAssociation: string;
|
||||||
|
body: string;
|
||||||
|
state: string;
|
||||||
|
submittedAt: string;
|
||||||
|
commit?: { oid: string } | null;
|
||||||
|
};
|
||||||
|
|
||||||
|
export type GhComment = {
|
||||||
|
author: { login: string } | null;
|
||||||
|
authorAssociation: string;
|
||||||
|
body: string;
|
||||||
|
createdAt: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
export type GhCheck = {
|
||||||
|
__typename: string;
|
||||||
|
name?: string;
|
||||||
|
workflowName?: string;
|
||||||
|
conclusion?: string | null;
|
||||||
|
status?: string;
|
||||||
|
state?: string;
|
||||||
|
context?: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
export type GhView = {
|
||||||
|
url: string;
|
||||||
|
title: string;
|
||||||
|
body: string;
|
||||||
|
isDraft: boolean;
|
||||||
|
reviewDecision: GhMeta["reviewDecision"];
|
||||||
|
mergeStateStatus: GhStats["mergeStateStatus"];
|
||||||
|
state: string;
|
||||||
|
author: { login: string };
|
||||||
|
createdAt: string;
|
||||||
|
updatedAt: string;
|
||||||
|
labels: { name: string }[];
|
||||||
|
additions: number;
|
||||||
|
deletions: number;
|
||||||
|
changedFiles: number;
|
||||||
|
baseRefName: string;
|
||||||
|
headRefName: string;
|
||||||
|
headRefOid: string;
|
||||||
|
maintainerCanModify: boolean;
|
||||||
|
assignees: { login: string }[];
|
||||||
|
files: GhFile[];
|
||||||
|
statusCheckRollup: GhCheck[];
|
||||||
|
reviews: GhReview[];
|
||||||
|
comments: GhComment[];
|
||||||
|
commits: {
|
||||||
|
oid: string;
|
||||||
|
committedDate: string;
|
||||||
|
authors: { login: string | null }[];
|
||||||
|
}[];
|
||||||
|
};
|
||||||
|
|
||||||
|
// --- triage / brief composites -------------------------------------------
|
||||||
|
|
||||||
|
export type Pr = {
|
||||||
|
number: number;
|
||||||
|
title: string;
|
||||||
|
author: string;
|
||||||
|
ageDays: number;
|
||||||
|
staleDays: number;
|
||||||
|
isDraft: boolean;
|
||||||
|
reviewDecision: GhMeta["reviewDecision"];
|
||||||
|
mergeStateStatus: GhStats["mergeStateStatus"];
|
||||||
|
size: string | null;
|
||||||
|
risk: string | null;
|
||||||
|
type: string | null;
|
||||||
|
changedFiles: number;
|
||||||
|
additions: number;
|
||||||
|
deletions: number;
|
||||||
|
headRefName: string;
|
||||||
|
baseRefName: string;
|
||||||
|
lane: Lane;
|
||||||
|
laneHits: Set<Lane>;
|
||||||
|
forbidden: ForbiddenHit[];
|
||||||
|
bucket: Bucket;
|
||||||
|
botOnlyApproval: boolean;
|
||||||
|
};
|
||||||
|
|
||||||
|
export type ValidationCommand = { command: string; reason: string };
|
||||||
|
|
||||||
|
// --- classify ------------------------------------------------------------
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A single tag emitted by classify. Each tag carries a script-stable name,
|
||||||
|
* a factual reason string (no judgment language), and a source token
|
||||||
|
* identifying which data field or rule produced it.
|
||||||
|
*
|
||||||
|
* `awaitingHours` is set only by the three `awaiting-*` tags; it reports
|
||||||
|
* the integer hour count between the awaiting-window start and the
|
||||||
|
* classify-run moment. Downstream consumers can floor-divide by 24 to get
|
||||||
|
* days, or use it as a sort key to prioritise the longest-waiting PRs
|
||||||
|
* inside an awaiting bucket.
|
||||||
|
*/
|
||||||
|
export type Tag = {
|
||||||
|
name: string;
|
||||||
|
reason: string;
|
||||||
|
source: string;
|
||||||
|
awaitingHours?: number;
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Distilled per-PR facts consumed by tag detectors. Built from either the
|
||||||
|
* list-mode chunked fetch or the single-PR `gh pr view` fetch — both
|
||||||
|
* representations converge here so detectors stay agnostic.
|
||||||
|
*/
|
||||||
|
export type PrFacts = {
|
||||||
|
number: number;
|
||||||
|
author: string;
|
||||||
|
title: string;
|
||||||
|
createdAt: string;
|
||||||
|
updatedAt: string;
|
||||||
|
isDraft: boolean;
|
||||||
|
reviewDecision: GhMeta["reviewDecision"];
|
||||||
|
mergeStateStatus: GhStats["mergeStateStatus"];
|
||||||
|
maintainerCanModify: boolean;
|
||||||
|
isOrgMember: boolean;
|
||||||
|
headRefOid: string;
|
||||||
|
assignees: string[];
|
||||||
|
labels: { name: string }[];
|
||||||
|
filePaths: string[];
|
||||||
|
reviews: {
|
||||||
|
author: { login: string } | null;
|
||||||
|
body: string;
|
||||||
|
state: string;
|
||||||
|
submittedAt: string;
|
||||||
|
commit?: { oid: string } | null;
|
||||||
|
}[];
|
||||||
|
comments: {
|
||||||
|
author: { login: string } | null;
|
||||||
|
body: string;
|
||||||
|
createdAt: string;
|
||||||
|
}[];
|
||||||
|
commits: {
|
||||||
|
committedDate: string;
|
||||||
|
authorLogin: string | null;
|
||||||
|
}[];
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Cross-PR context passed to detectors that need to look beyond a single
|
||||||
|
* PR (e.g. duplicate-title needs the full open-queue title index).
|
||||||
|
*/
|
||||||
|
export type ClassifyContext = {
|
||||||
|
/** Map of `${author}\0${title}` to PR numbers that share it. */
|
||||||
|
titleIndexByAuthor: Map<string, number[]>;
|
||||||
|
};
|
||||||
|
|
||||||
|
export type ClassifyReport = {
|
||||||
|
generatedAt: string;
|
||||||
|
openPrTotal: number;
|
||||||
|
classifiedCount: number;
|
||||||
|
byTag: Record<string, number[]>;
|
||||||
|
byNumber: Record<string, Tag[]>;
|
||||||
|
rate: {
|
||||||
|
/** GraphQL points remaining before the fetch started. */
|
||||||
|
before: { remaining: number; limit: number; resetAt: string };
|
||||||
|
/** GraphQL points remaining after the fetch finished. */
|
||||||
|
after: { remaining: number; limit: number; resetAt: string };
|
||||||
|
/**
|
||||||
|
* Computed delta = before.remaining - after.remaining, *only* when both
|
||||||
|
* snapshots fell in the same reset window. `null` when the window rolled
|
||||||
|
* over between snapshots and the delta is no longer meaningful.
|
||||||
|
*/
|
||||||
|
cost: number | null;
|
||||||
|
};
|
||||||
|
};
|
||||||
|
|
||||||
|
export type Brief = {
|
||||||
|
number: number;
|
||||||
|
url: string;
|
||||||
|
title: string;
|
||||||
|
state: string;
|
||||||
|
reviewDecision: GhMeta["reviewDecision"];
|
||||||
|
mergeStateStatus: GhStats["mergeStateStatus"];
|
||||||
|
isDraft: boolean;
|
||||||
|
author: string;
|
||||||
|
branch: { head: string; base: string };
|
||||||
|
age: { createdAt: string; updatedAt: string; ageDays: number; staleDays: number };
|
||||||
|
labels: { size: string | null; risk: string | null; type: string | null; all: string[] };
|
||||||
|
diff: { additions: number; deletions: number; changedFiles: number };
|
||||||
|
lane: Lane;
|
||||||
|
laneHits: Lane[];
|
||||||
|
forbidden: ForbiddenHit[];
|
||||||
|
seamsTouched: string[];
|
||||||
|
topFiles: { path: string; additions: number; deletions: number; changeType: string }[];
|
||||||
|
filterSuppressedFileCount: number;
|
||||||
|
laneRules: string[];
|
||||||
|
validation: ValidationCommand[];
|
||||||
|
reviews: { author: string; state: string; submittedAt: string; body: string }[];
|
||||||
|
reviewCountTotal: number;
|
||||||
|
botOnlyApproval: boolean;
|
||||||
|
comments: { author: string; createdAt: string; body: string }[];
|
||||||
|
commentCountTotal: number;
|
||||||
|
checks: { workflow: string; passing: number; failing: number; pending: number; total: number }[];
|
||||||
|
bodyPreview: string;
|
||||||
|
bodyChars: number;
|
||||||
|
};
|
||||||
408
tools/pr/src/view.ts
Normal file
408
tools/pr/src/view.ts
Normal file
|
|
@ -0,0 +1,408 @@
|
||||||
|
/**
|
||||||
|
* `tools-pr view <num>` — factual brief for a single PR.
|
||||||
|
*
|
||||||
|
* Output is strictly facts + repo-doc citations; no judgments, no directives.
|
||||||
|
* Sections:
|
||||||
|
* - lane + forbidden-surface + public-seam observations
|
||||||
|
* - top files (lockfile, translations, generated dirs filtered as noise)
|
||||||
|
* - per-lane rules cited from code-review-guidelines.md / AGENTS.md /
|
||||||
|
* CONTRIBUTING.zh-CN.md with sources noted inline
|
||||||
|
* - validation commands derived from touched packages, per AGENTS.md
|
||||||
|
* §Validation strategy
|
||||||
|
* - bot-stripped review + comment summary (Looper-stamped reviews moved to
|
||||||
|
* the bot-only-approval fact line)
|
||||||
|
* - CI rollup grouped by workflow
|
||||||
|
*/
|
||||||
|
|
||||||
|
import {
|
||||||
|
condense,
|
||||||
|
isBotAuthored,
|
||||||
|
isBotOnlyApproval,
|
||||||
|
reduceLatestReviewsByAuthor,
|
||||||
|
} from "./bot.js";
|
||||||
|
import { daysSince, fetchView, labelByPrefix } from "./gh.js";
|
||||||
|
import { deriveForbidden, deriveLane, deriveSeams, isNoisyFile, SKILL_DIR, DESIGN_DIR } from "./lane.js";
|
||||||
|
import type {
|
||||||
|
Brief,
|
||||||
|
GhCheck,
|
||||||
|
GhView,
|
||||||
|
Lane,
|
||||||
|
ValidationCommand,
|
||||||
|
} from "./types.js";
|
||||||
|
|
||||||
|
// --- validation derivation -----------------------------------------------
|
||||||
|
|
||||||
|
function deriveValidation(paths: string[]): ValidationCommand[] {
|
||||||
|
const cmds: ValidationCommand[] = [];
|
||||||
|
const seen = new Set<string>();
|
||||||
|
const add = (command: string, reason: string): void => {
|
||||||
|
if (seen.has(command)) return;
|
||||||
|
seen.add(command);
|
||||||
|
cmds.push({ command, reason });
|
||||||
|
};
|
||||||
|
|
||||||
|
add("pnpm guard", "TS-first + .js allowlist gate");
|
||||||
|
add("pnpm typecheck", "workspace-wide typecheck (root)");
|
||||||
|
|
||||||
|
const touched = (prefix: string): boolean => paths.some((p) => p.startsWith(prefix));
|
||||||
|
const touchedAny = (prefixes: string[]): boolean => prefixes.some(touched);
|
||||||
|
|
||||||
|
if (touched("apps/web/")) {
|
||||||
|
add("pnpm --filter @open-design/web typecheck", "apps/web changed");
|
||||||
|
add("pnpm --filter @open-design/web test", "apps/web changed");
|
||||||
|
add("pnpm --filter @open-design/web build", "apps/web changed");
|
||||||
|
}
|
||||||
|
if (touched("apps/daemon/")) {
|
||||||
|
add("pnpm --filter @open-design/daemon typecheck", "apps/daemon changed");
|
||||||
|
add("pnpm --filter @open-design/daemon test", "apps/daemon changed");
|
||||||
|
add("pnpm --filter @open-design/daemon build", "apps/daemon changed");
|
||||||
|
}
|
||||||
|
if (touched("apps/desktop/")) {
|
||||||
|
add("pnpm --filter @open-design/desktop typecheck", "apps/desktop changed");
|
||||||
|
add("pnpm --filter @open-design/desktop build", "apps/desktop changed");
|
||||||
|
}
|
||||||
|
if (touched("apps/packaged/")) {
|
||||||
|
add("pnpm --filter @open-design/packaged typecheck", "apps/packaged changed");
|
||||||
|
add("pnpm --filter @open-design/packaged build", "apps/packaged changed");
|
||||||
|
}
|
||||||
|
if (touched("packages/contracts/")) {
|
||||||
|
add("pnpm --filter @open-design/contracts typecheck", "packages/contracts changed");
|
||||||
|
}
|
||||||
|
if (touched("packages/sidecar-proto/")) {
|
||||||
|
add("pnpm --filter @open-design/sidecar-proto typecheck", "sidecar-proto changed");
|
||||||
|
add("pnpm --filter @open-design/sidecar-proto test", "sidecar-proto changed");
|
||||||
|
}
|
||||||
|
if (touched("packages/sidecar/")) {
|
||||||
|
add("pnpm --filter @open-design/sidecar typecheck", "packages/sidecar changed");
|
||||||
|
add("pnpm --filter @open-design/sidecar test", "packages/sidecar changed");
|
||||||
|
}
|
||||||
|
if (touched("packages/platform/")) {
|
||||||
|
add("pnpm --filter @open-design/platform typecheck", "packages/platform changed");
|
||||||
|
add("pnpm --filter @open-design/platform test", "packages/platform changed");
|
||||||
|
}
|
||||||
|
if (touched("tools/dev/")) {
|
||||||
|
add("pnpm --filter @open-design/tools-dev typecheck", "tools/dev changed");
|
||||||
|
add("pnpm --filter @open-design/tools-dev build", "tools/dev changed");
|
||||||
|
}
|
||||||
|
if (touched("tools/pack/")) {
|
||||||
|
add("pnpm --filter @open-design/tools-pack typecheck", "tools/pack changed");
|
||||||
|
add("pnpm --filter @open-design/tools-pack build", "tools/pack changed");
|
||||||
|
}
|
||||||
|
if (touched("tools/pr/")) {
|
||||||
|
add("pnpm --filter @open-design/tools-pr typecheck", "tools/pr changed");
|
||||||
|
add("pnpm --filter @open-design/tools-pr build", "tools/pr changed");
|
||||||
|
}
|
||||||
|
if (touchedAny(["e2e/specs/", "e2e/tests/", "e2e/lib/"])) {
|
||||||
|
add("pnpm --filter @open-design/e2e typecheck", "e2e/ changed");
|
||||||
|
add("(cd e2e && pnpm test specs)", "e2e specs are the PR smoke gate");
|
||||||
|
}
|
||||||
|
if (touched("e2e/ui/")) {
|
||||||
|
add("(cd e2e && pnpm exec playwright test -c playwright.config.ts)", "Playwright UI changed");
|
||||||
|
}
|
||||||
|
|
||||||
|
const stampRelated = paths.some((p) => /(sidecar|stamp|namespace|packaged|tools-pack)/i.test(p));
|
||||||
|
if (stampRelated) {
|
||||||
|
add(
|
||||||
|
"# run inspect eval + screenshot for two concurrent namespaces (AGENTS.md)",
|
||||||
|
"stamp/namespace surface touched",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
const pathLogRelated = paths.some((p) => /(tools-dev|tools-pack|log|logger|\.tmp)/i.test(p));
|
||||||
|
if (pathLogRelated) {
|
||||||
|
add(
|
||||||
|
"pnpm tools-dev logs --namespace <name> --json",
|
||||||
|
"path/log surface touched — confirm paths under .tmp/tools-dev/<namespace>/",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return cmds;
|
||||||
|
}
|
||||||
|
|
||||||
|
// --- lane checklist ------------------------------------------------------
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Per-lane rule citations. Each line is either an observed fact about the
|
||||||
|
* touched paths or a quotation/summary of an existing repo rule with its
|
||||||
|
* source noted. No judgments, no directives.
|
||||||
|
*/
|
||||||
|
function laneRules(lane: Lane, paths: string[]): string[] {
|
||||||
|
const items: string[] = [];
|
||||||
|
const hasFile = (suffix: string): boolean => paths.some((p) => p.endsWith(suffix));
|
||||||
|
const skillRoots = new Set(
|
||||||
|
paths.filter((p) => SKILL_DIR.test(p)).map((p) => p.split("/").slice(0, 2).join("/")),
|
||||||
|
);
|
||||||
|
const designRoots = new Set(
|
||||||
|
paths.filter((p) => DESIGN_DIR.test(p)).map((p) => p.split("/").slice(0, 2).join("/")),
|
||||||
|
);
|
||||||
|
|
||||||
|
switch (lane) {
|
||||||
|
case "skill":
|
||||||
|
items.push(`fact: skill roots touched — ${[...skillRoots].join(", ") || "(none)"}`);
|
||||||
|
items.push(
|
||||||
|
`fact: SKILL.md present at every touched root — ${[...skillRoots].every((root) => paths.includes(`${root}/SKILL.md`)) ? "yes" : "no"}`,
|
||||||
|
);
|
||||||
|
items.push(`fact: example.html present at a touched root — ${hasFile("/example.html") ? "yes" : "no"}`);
|
||||||
|
items.push(`fact: references/checklist.md present at a touched root — ${hasFile("/references/checklist.md") ? "yes" : "no"}`);
|
||||||
|
items.push("rule [CONTRIBUTING.zh-CN.md skill 硬线 1]: real hand-crafted example.html present");
|
||||||
|
items.push("rule [CONTRIBUTING.zh-CN.md skill 硬线 2]: anti-AI-slop list — purple gradients, generic emoji icons, Inter-as-display, invented numbers");
|
||||||
|
items.push("rule [CONTRIBUTING.zh-CN.md skill 硬线 4]: references/checklist.md with at least the P0 gate");
|
||||||
|
items.push("rule [CONTRIBUTING.zh-CN.md skill 硬线 5]: featured skill ⇒ docs/screenshots/skills/<slug>.png");
|
||||||
|
items.push("rule [CONTRIBUTING.zh-CN.md skill 硬线 6]: self-contained — CDN scope ≤ existing skills, no unlicensed fonts, assets ≲ 250 KB");
|
||||||
|
break;
|
||||||
|
case "design-system":
|
||||||
|
items.push(`fact: design-system roots touched — ${[...designRoots].join(", ") || "(none)"}`);
|
||||||
|
items.push(`fact: DESIGN.md present at a touched root — ${hasFile("/DESIGN.md") ? "yes" : "no"}`);
|
||||||
|
items.push("rule [code-review-guidelines.md §4.3]: first H1 = picker title; '> Category:' line uses an existing dropdown group");
|
||||||
|
items.push("rule [CONTRIBUTING.zh-CN.md design-system 硬线 1]: 9 sections present (visual / color / typography / spacing / layout / components / motion / voice / anti-patterns)");
|
||||||
|
items.push("rule [CONTRIBUTING.zh-CN.md design-system 硬线 5]: ASCII slug only (linear.app → linear-app)");
|
||||||
|
items.push("rule [CONTRIBUTING.zh-CN.md design-system 硬线 2-3]: hex sampled from real source; OKLch acceptable for accent ramps");
|
||||||
|
break;
|
||||||
|
case "craft":
|
||||||
|
items.push("rule [code-review-guidelines.md §4.5]: universal brand-agnostic craft (not brand-specific, not artifact-shape)");
|
||||||
|
items.push("rule [code-review-guidelines.md §4.5]: at least one shipping skill opts in via od.craft.requires, or follow-up named in PR description");
|
||||||
|
items.push("reference: existing craft entry shapes — craft/typography.md, craft/color.md, craft/animation-discipline.md");
|
||||||
|
break;
|
||||||
|
case "contract":
|
||||||
|
items.push("rule [AGENTS.md §Boundary constraints]: packages/contracts free of Next/Express/Node fs|process/browser APIs/SQLite/daemon internals/sidecar control-plane deps");
|
||||||
|
items.push("rule [AGENTS.md §Boundary constraints]: stamp fields exactly five — app, mode, namespace, ipc, source");
|
||||||
|
items.push("rule [code-review-guidelines.md §4.2]: contract change lands before consumers, or in same PR with both sides wired");
|
||||||
|
items.push("rule [code-review-guidelines.md §4.2]: breaking persisted-format change requires explicit migration + one-release compat window");
|
||||||
|
items.push("rule [code-review-guidelines.md §4.2]: producer and consumer both have type/test coverage of the new shape");
|
||||||
|
break;
|
||||||
|
case "docs":
|
||||||
|
items.push("rule [code-review-guidelines.md §7 Documentation-only review]: internal link integrity (relative paths, anchors)");
|
||||||
|
items.push("rule [code-review-guidelines.md §7]: no conflict with AGENTS.md chain (root > directory-level)");
|
||||||
|
items.push("rule [AGENTS.md §Validation strategy]: pnpm guard + pnpm typecheck required");
|
||||||
|
break;
|
||||||
|
case "multi":
|
||||||
|
items.push("rule [code-review-guidelines.md §3 In scope, Multi-area]: public seam motivates the cross-cut (HTTP API / contract / sidecar / command / persisted format)");
|
||||||
|
items.push("rule [code-review-guidelines.md §3]: owning contract/protocol/primitive change lands first or in same PR");
|
||||||
|
items.push("rule [code-review-guidelines.md §3]: one clear primary owner");
|
||||||
|
break;
|
||||||
|
case "default":
|
||||||
|
case "unknown":
|
||||||
|
items.push("rule [AGENTS.md §Boundary constraints]: tests live in sibling tests/ directories; no *.test.* under src/");
|
||||||
|
items.push("rule [AGENTS.md §Boundary constraints]: shared logic in owning package; no cross-app private imports");
|
||||||
|
items.push("rule [AGENTS.md §Boundary constraints]: shared web/daemon API DTOs in packages/contracts, not in app internals");
|
||||||
|
items.push("reference: forbidden-surface scan in the Boundaries section above is authoritative");
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
return items;
|
||||||
|
}
|
||||||
|
|
||||||
|
// --- check rollup --------------------------------------------------------
|
||||||
|
|
||||||
|
function summarizeChecks(rollup: GhCheck[]): Brief["checks"] {
|
||||||
|
const groups = new Map<
|
||||||
|
string,
|
||||||
|
{ passing: number; failing: number; pending: number; total: number }
|
||||||
|
>();
|
||||||
|
for (const check of rollup) {
|
||||||
|
const key = check.workflowName ?? check.name ?? check.context ?? "(unknown)";
|
||||||
|
const bucket = groups.get(key) ?? { passing: 0, failing: 0, pending: 0, total: 0 };
|
||||||
|
bucket.total += 1;
|
||||||
|
const conclusion = (check.conclusion ?? check.state ?? "").toUpperCase();
|
||||||
|
if (conclusion === "SUCCESS" || conclusion === "NEUTRAL" || conclusion === "SKIPPED") bucket.passing += 1;
|
||||||
|
else if (
|
||||||
|
conclusion === "FAILURE" ||
|
||||||
|
conclusion === "CANCELLED" ||
|
||||||
|
conclusion === "TIMED_OUT" ||
|
||||||
|
conclusion === "ACTION_REQUIRED"
|
||||||
|
)
|
||||||
|
bucket.failing += 1;
|
||||||
|
else bucket.pending += 1;
|
||||||
|
groups.set(key, bucket);
|
||||||
|
}
|
||||||
|
return [...groups.entries()]
|
||||||
|
.map(([workflow, stats]) => ({ workflow, ...stats }))
|
||||||
|
.sort((a, b) => b.failing - a.failing || a.workflow.localeCompare(b.workflow));
|
||||||
|
}
|
||||||
|
|
||||||
|
// --- brief assembly + formatting -----------------------------------------
|
||||||
|
|
||||||
|
function buildBrief(num: number, view: GhView): Brief {
|
||||||
|
const now = Date.now();
|
||||||
|
const paths = view.files.map((file) => file.path);
|
||||||
|
const { lane, hits } = deriveLane(paths);
|
||||||
|
|
||||||
|
const topFiles = [...view.files]
|
||||||
|
.filter((file) => !isNoisyFile(file.path))
|
||||||
|
.sort((a, b) => b.additions + b.deletions - (a.additions + a.deletions))
|
||||||
|
.slice(0, 8)
|
||||||
|
.map((file) => ({
|
||||||
|
path: file.path,
|
||||||
|
additions: file.additions,
|
||||||
|
deletions: file.deletions,
|
||||||
|
changeType: file.changeType,
|
||||||
|
}));
|
||||||
|
const filterSuppressedFileCount = view.files.length - topFiles.length;
|
||||||
|
|
||||||
|
const latestPerReviewer = reduceLatestReviewsByAuthor(view.reviews);
|
||||||
|
const reviews = latestPerReviewer
|
||||||
|
.filter((review) => !isBotAuthored(review.author, review.body))
|
||||||
|
.map((review) => ({
|
||||||
|
author: review.author?.login ?? "(unknown)",
|
||||||
|
state: review.state,
|
||||||
|
submittedAt: review.submittedAt,
|
||||||
|
body: condense(review.body, 200),
|
||||||
|
}))
|
||||||
|
.sort((a, b) => b.submittedAt.localeCompare(a.submittedAt));
|
||||||
|
|
||||||
|
const comments = view.comments
|
||||||
|
.filter((comment) => !isBotAuthored(comment.author, comment.body))
|
||||||
|
.map((comment) => ({
|
||||||
|
author: comment.author?.login ?? "(unknown)",
|
||||||
|
createdAt: comment.createdAt,
|
||||||
|
body: condense(comment.body, 200),
|
||||||
|
}))
|
||||||
|
.sort((a, b) => b.createdAt.localeCompare(a.createdAt))
|
||||||
|
.slice(0, 3);
|
||||||
|
|
||||||
|
return {
|
||||||
|
number: num,
|
||||||
|
url: view.url,
|
||||||
|
title: view.title,
|
||||||
|
state: view.state,
|
||||||
|
reviewDecision: view.reviewDecision,
|
||||||
|
mergeStateStatus: view.mergeStateStatus,
|
||||||
|
isDraft: view.isDraft,
|
||||||
|
author: view.author.login,
|
||||||
|
branch: { head: view.headRefName, base: view.baseRefName },
|
||||||
|
age: {
|
||||||
|
createdAt: view.createdAt,
|
||||||
|
updatedAt: view.updatedAt,
|
||||||
|
ageDays: daysSince(view.createdAt, now),
|
||||||
|
staleDays: daysSince(view.updatedAt, now),
|
||||||
|
},
|
||||||
|
labels: {
|
||||||
|
size: labelByPrefix(view.labels, "size/"),
|
||||||
|
risk: labelByPrefix(view.labels, "risk/"),
|
||||||
|
type: labelByPrefix(view.labels, "type/"),
|
||||||
|
all: view.labels.map((label) => label.name),
|
||||||
|
},
|
||||||
|
diff: {
|
||||||
|
additions: view.additions,
|
||||||
|
deletions: view.deletions,
|
||||||
|
changedFiles: view.changedFiles,
|
||||||
|
},
|
||||||
|
lane,
|
||||||
|
laneHits: [...hits],
|
||||||
|
forbidden: deriveForbidden(paths),
|
||||||
|
seamsTouched: deriveSeams(paths),
|
||||||
|
topFiles,
|
||||||
|
filterSuppressedFileCount,
|
||||||
|
laneRules: laneRules(lane, paths),
|
||||||
|
validation: deriveValidation(paths),
|
||||||
|
reviews,
|
||||||
|
reviewCountTotal: latestPerReviewer.length,
|
||||||
|
botOnlyApproval: isBotOnlyApproval(view.reviewDecision, latestPerReviewer),
|
||||||
|
comments,
|
||||||
|
commentCountTotal: view.comments.length,
|
||||||
|
checks: summarizeChecks(view.statusCheckRollup),
|
||||||
|
bodyPreview: condense(view.body, 400),
|
||||||
|
bodyChars: view.body.length,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function formatBrief(brief: Brief): string {
|
||||||
|
const lines: string[] = [];
|
||||||
|
const labelTags = [
|
||||||
|
brief.labels.size ? `size/${brief.labels.size}` : null,
|
||||||
|
brief.labels.risk ? `risk/${brief.labels.risk}` : null,
|
||||||
|
brief.labels.type ? `type/${brief.labels.type}` : null,
|
||||||
|
].filter((v): v is string => v !== null);
|
||||||
|
|
||||||
|
lines.push(`PR #${brief.number} · ${brief.title}`);
|
||||||
|
lines.push(` url ${brief.url}`);
|
||||||
|
lines.push(` author ${brief.author}`);
|
||||||
|
lines.push(` branch ${brief.branch.head} → ${brief.branch.base}`);
|
||||||
|
lines.push(
|
||||||
|
` state ${brief.state} · ${brief.reviewDecision || "REVIEW_REQUIRED"} · ${brief.mergeStateStatus}${brief.isDraft ? " · draft" : ""}`,
|
||||||
|
);
|
||||||
|
lines.push(` age created ${brief.age.ageDays}d ago · updated ${brief.age.staleDays}d ago`);
|
||||||
|
lines.push(` labels ${labelTags.join(", ") || "(none)"}`);
|
||||||
|
lines.push(
|
||||||
|
` diff +${brief.diff.additions} −${brief.diff.deletions} across ${brief.diff.changedFiles} files`,
|
||||||
|
);
|
||||||
|
if (brief.botOnlyApproval) {
|
||||||
|
lines.push("");
|
||||||
|
lines.push(" fact: bot-only approval — reviewDecision=APPROVED, every APPROVED review is bot-authored.");
|
||||||
|
lines.push(" fact: zero APPROVED reviews authored by a non-bot account.");
|
||||||
|
}
|
||||||
|
lines.push("");
|
||||||
|
|
||||||
|
lines.push("── Boundaries (lane / forbidden / seams) ──");
|
||||||
|
lines.push(
|
||||||
|
` lane ${brief.lane}${brief.laneHits.length > 1 ? ` (hits: ${brief.laneHits.join(", ")})` : ""}`,
|
||||||
|
);
|
||||||
|
lines.push(` forbidden ${brief.forbidden.length === 0 ? "[none]" : brief.forbidden.join(", ")}`);
|
||||||
|
lines.push(` seams ${brief.seamsTouched.length === 0 ? "[none]" : brief.seamsTouched.join(", ")}`);
|
||||||
|
lines.push("");
|
||||||
|
|
||||||
|
lines.push(`── Top files (${brief.topFiles.length} shown, ${brief.filterSuppressedFileCount} filtered by NOISY_FILE_PATTERNS in lane.ts) ──`);
|
||||||
|
for (const file of brief.topFiles) {
|
||||||
|
const delta = `+${file.additions} −${file.deletions}`.padEnd(10, " ");
|
||||||
|
lines.push(` ${delta} ${file.path} (${file.changeType})`);
|
||||||
|
}
|
||||||
|
lines.push("");
|
||||||
|
|
||||||
|
lines.push(`── Lane rules (${brief.lane}) ──`);
|
||||||
|
for (const item of brief.laneRules) lines.push(` • ${item}`);
|
||||||
|
lines.push("");
|
||||||
|
|
||||||
|
lines.push("── Validation (AGENTS.md §Validation strategy, derived from touched packages) ──");
|
||||||
|
for (const cmd of brief.validation) {
|
||||||
|
lines.push(` $ ${cmd.command}`);
|
||||||
|
lines.push(` ↳ ${cmd.reason}`);
|
||||||
|
}
|
||||||
|
lines.push("");
|
||||||
|
|
||||||
|
lines.push(`── Recent reviews (${brief.reviews.length} human-shown of ${brief.reviewCountTotal} total) ──`);
|
||||||
|
if (brief.reviews.length === 0) lines.push(" (no human reviews yet)");
|
||||||
|
for (const review of brief.reviews) {
|
||||||
|
lines.push(` @${review.author} ${review.state} ${review.submittedAt}`);
|
||||||
|
lines.push(` "${review.body}"`);
|
||||||
|
}
|
||||||
|
lines.push("");
|
||||||
|
|
||||||
|
lines.push(`── Recent comments (${brief.comments.length} of ${brief.commentCountTotal}) ──`);
|
||||||
|
if (brief.comments.length === 0) lines.push(" (no human comments)");
|
||||||
|
for (const comment of brief.comments) {
|
||||||
|
lines.push(` @${comment.author} ${comment.createdAt}`);
|
||||||
|
lines.push(` "${comment.body}"`);
|
||||||
|
}
|
||||||
|
lines.push("");
|
||||||
|
|
||||||
|
lines.push("── CI ──");
|
||||||
|
if (brief.checks.length === 0) lines.push(" (no checks reported)");
|
||||||
|
for (const group of brief.checks) {
|
||||||
|
const symbol = group.failing > 0 ? "✗" : group.pending > 0 ? "·" : "✓";
|
||||||
|
lines.push(
|
||||||
|
` ${symbol} ${group.workflow.padEnd(28, " ")} ${group.passing}/${group.total} pass` +
|
||||||
|
(group.failing ? `, ${group.failing} fail` : "") +
|
||||||
|
(group.pending ? `, ${group.pending} pending` : ""),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
lines.push("");
|
||||||
|
|
||||||
|
lines.push(`── PR body (preview, ${brief.bodyChars} chars total) ──`);
|
||||||
|
lines.push(brief.bodyPreview ? ` ${brief.bodyPreview}` : " (empty body)");
|
||||||
|
|
||||||
|
return lines.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
|
export type ViewOptions = {
|
||||||
|
json?: boolean;
|
||||||
|
};
|
||||||
|
|
||||||
|
export async function runView(num: number, options: ViewOptions): Promise<void> {
|
||||||
|
if (!Number.isFinite(num) || num <= 0) {
|
||||||
|
throw new Error("view requires a positive PR number, e.g. tools-pr view 1180");
|
||||||
|
}
|
||||||
|
const view = await fetchView(num);
|
||||||
|
const brief = buildBrief(num, view);
|
||||||
|
if (options.json) {
|
||||||
|
process.stdout.write(`${JSON.stringify(brief, null, 2)}\n`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
process.stdout.write(`${formatBrief(brief)}\n`);
|
||||||
|
}
|
||||||
108
tools/pr/templates/agent-review.md
Normal file
108
tools/pr/templates/agent-review.md
Normal file
|
|
@ -0,0 +1,108 @@
|
||||||
|
<!--
|
||||||
|
Style reference: agent-produced PR review brief
|
||||||
|
|
||||||
|
This file is an aesthetic reference, not a fill-in form. When an agent
|
||||||
|
writes a PR review brief (typically to `.tmp/tools-pr/reviews/<num>.md` as
|
||||||
|
internal analysis for a human maintainer), read this guide for tone +
|
||||||
|
section pool, then compose for the specific PR. See maintainer memory
|
||||||
|
`feedback_templates_are_style_refs` and `feedback_agent_review_shape`.
|
||||||
|
|
||||||
|
The agent review is NOT a GitHub-posted comment — it's an internal
|
||||||
|
analysis artifact. A separate, downstream step would adapt findings into
|
||||||
|
public-facing review feedback if posting is decided.
|
||||||
|
|
||||||
|
## Tone
|
||||||
|
|
||||||
|
- Factual. Every claim cites a file:line, an AGENTS.md / code-review-
|
||||||
|
guidelines.md / CONTRIBUTING.zh-CN.md rule, or PR metadata.
|
||||||
|
- No judgment words: skip "nice work", "looks clean", "should merge",
|
||||||
|
"high risk". Risk severity comes from gh labels, not from the review.
|
||||||
|
- Suggestions section is maintainer-to-maintainer punchy advice, not
|
||||||
|
author teaching. Bullets like "Ask the author to split: land the
|
||||||
|
symlink fix standalone" beat "(a) consider splitting (b) leave it
|
||||||
|
bundled (c) discuss".
|
||||||
|
- Concise. If a section has no signal beyond "everything ✓", omit it
|
||||||
|
rather than fill the page with boilerplate compliance ticks.
|
||||||
|
|
||||||
|
## Section pool (pick what carries signal)
|
||||||
|
|
||||||
|
Section order, when included, is stable: Identification → Prior reviewer
|
||||||
|
findings → Findings → Suggestions → Validation. Most reviews only have
|
||||||
|
3-5 sections; never all of them.
|
||||||
|
|
||||||
|
### Identification (always)
|
||||||
|
|
||||||
|
PR metadata block. Title, author (note `(external)` vs org-member),
|
||||||
|
status (compose state flags: OPEN, REVIEW_REQUIRED / CHANGES_REQUESTED /
|
||||||
|
APPROVED, DIRTY / BLOCKED, draft), labels, lane + parenthetical note on
|
||||||
|
forbidden surfaces / public seams if any, diff totals.
|
||||||
|
|
||||||
|
### Prior reviewer findings (still applicable)
|
||||||
|
|
||||||
|
Only when `state=CHANGES_REQUESTED` AND prior human reviews carry concrete
|
||||||
|
flags. Summarize what each prior reviewer raised, distinguishing human
|
||||||
|
vs bot. Cite the relevant file:line. Don't re-derive their analysis —
|
||||||
|
just verify against the current head and reference.
|
||||||
|
|
||||||
|
### Findings / Observations
|
||||||
|
|
||||||
|
Substantive technical / scope / contract / security / test-coverage
|
||||||
|
observations beyond boundary-pass-through. Numbered subsections, each
|
||||||
|
worth its space:
|
||||||
|
- Skip a "boundary check" subsection when all rules pass quietly — fold
|
||||||
|
the one-liner into the Identification's lane line instead.
|
||||||
|
- Skip a "current PR state" subsection when the only thing to say is
|
||||||
|
"DIRTY, needs rebase" — that's already in Identification.
|
||||||
|
- Only spend space on the symlink-fix-is-clean kind of observation when
|
||||||
|
it's actually load-bearing (e.g. it tells the maintainer which slice
|
||||||
|
is ready to land in a split).
|
||||||
|
|
||||||
|
### Suggestions
|
||||||
|
|
||||||
|
Only when valuable maintainer-perspective advice exists. Punchy bullets
|
||||||
|
that state what to do and why in one breath. Self-check before writing:
|
||||||
|
1. Is this actionable for a maintainer right now?
|
||||||
|
2. Is it a fact-grounded direction, not "consider option (a) vs (b)"?
|
||||||
|
3. Can it be one sentence tighter?
|
||||||
|
|
||||||
|
### Validation expected
|
||||||
|
|
||||||
|
Pnpm command list derived from touched packages (matches `tools-pr view`
|
||||||
|
output), plus any manual smoke / regression test recommendation specific
|
||||||
|
to the change. Always useful unless the PR is trivial.
|
||||||
|
|
||||||
|
## Variables that drive shape
|
||||||
|
|
||||||
|
- **PR state**: drives whether "Prior reviewer findings" exists and
|
||||||
|
whether to comment on state explicitly.
|
||||||
|
- **Lane**: drives which rule citations are relevant. Contract-lane
|
||||||
|
reviews care about §4.2; skill/design-system reviews about §4.3-4.4.
|
||||||
|
- **Scope health**: a clean PR rarely needs a long Findings section. A
|
||||||
|
scope-mixed PR justifies Findings as the largest section.
|
||||||
|
- **Author org membership**: noted in Identification line. Influences
|
||||||
|
whether eventual human-facing communication should route through IM
|
||||||
|
vs GitHub — that decision lives outside this review.
|
||||||
|
|
||||||
|
## Committed exemplars
|
||||||
|
|
||||||
|
These are not literal templates. They're frozen-in-time examples of the
|
||||||
|
style applied to three different PR shapes. See `tools/pr/templates/examples/`:
|
||||||
|
|
||||||
|
- `agent-review-1037-scope-expansion.md` — REVIEW_REQUIRED + draft + scope
|
||||||
|
expansion: Findings is the largest section because there are three
|
||||||
|
distinct changes to disentangle.
|
||||||
|
- `agent-review-1149-clean-contract.md` — REVIEW_REQUIRED + clean contract
|
||||||
|
feature: Findings is short, Suggestions concentrate on edge cases
|
||||||
|
(caps, docs, traversal regression test).
|
||||||
|
- `agent-review-1009-changes-requested.md` — CHANGES_REQUESTED + prior
|
||||||
|
human reviews + security: Prior reviewer findings section carries most
|
||||||
|
of the analysis weight; Findings adds two notes the prior reviewers
|
||||||
|
didn't cover; Suggestions prescribes a concrete direction.
|
||||||
|
|
||||||
|
The differences in shape are the point. A homogenized 7-section template
|
||||||
|
applied to all three would dilute the signal density.
|
||||||
|
|
||||||
|
The exemplars are frozen historical snapshots (see the HTML-comment
|
||||||
|
header in each file) — they teach the style applied to a real moment,
|
||||||
|
not the live state of those PRs. Fresh runtime reviews land in
|
||||||
|
`.tmp/tools-pr/reviews/<num>.md` and are transient by design.
|
||||||
38
tools/pr/templates/awaiting-author-nudge.md
Normal file
38
tools/pr/templates/awaiting-author-nudge.md
Normal file
|
|
@ -0,0 +1,38 @@
|
||||||
|
<!--
|
||||||
|
Template: awaiting-author nudge
|
||||||
|
|
||||||
|
This file is an aesthetic reference, not a fill-in form. When you need to
|
||||||
|
post this kind of comment, read the beats and the exemplar below to absorb
|
||||||
|
the tone, then compose a fresh comment for the specific PR. Do not
|
||||||
|
sed-substitute the placeholders and post the result verbatim — see
|
||||||
|
maintainer memory `feedback_templates_are_style_refs`.
|
||||||
|
|
||||||
|
When this applies:
|
||||||
|
classify emits the `awaiting-author-response-24h` tag and the author has
|
||||||
|
been silent for ≥ 4 days (96h+). For PRs in the 24h-96h window, hold off
|
||||||
|
— the author may still be working on it.
|
||||||
|
|
||||||
|
Beats the comment should hit (in order, very short):
|
||||||
|
1. Friendly check-in opener (one short clause).
|
||||||
|
2. State the waiting duration as a fact, no judgment.
|
||||||
|
3. Direct status question: still planning to land it?
|
||||||
|
4. Optional unblock offer if the next step is non-obvious from the diff.
|
||||||
|
5. Graceful exit option: "just say so and we'll close it cleanly" — this
|
||||||
|
respects the author's time and keeps the queue honest.
|
||||||
|
6. Tooling-transparency line ("Surfaced via `awaiting-author-response-24h`...").
|
||||||
|
|
||||||
|
Placeholders mark the PR-specific facts to weave in (not literal blanks):
|
||||||
|
{AUTHOR} — the PR author's GitHub login
|
||||||
|
{AWAITING_HUMAN} — human-readable awaiting duration, e.g. "8 days" or
|
||||||
|
"5 days 4 hours" (derived from tag.awaitingHours)
|
||||||
|
|
||||||
|
Tone target: friendly + dignified, concise, no padding, no robot phrasing.
|
||||||
|
Vary the wording between PRs you nudge in the same session — repeated
|
||||||
|
identical comments visible across a contributor's notifications break the
|
||||||
|
"this is a person talking to me" feel.
|
||||||
|
|
||||||
|
Exemplar (one valid way to write it):
|
||||||
|
-->
|
||||||
|
@{AUTHOR} — quick check-in: this has been waiting on your follow-up for about {AWAITING_HUMAN}. Still planning to land it? Happy to help if you're stuck on anything specific; if priorities have shifted, just say so and we'll close it cleanly.
|
||||||
|
|
||||||
|
(Surfaced via the `awaiting-author-response-24h` maintainer tooling tag.)
|
||||||
36
tools/pr/templates/duplicate-title-ask.md
Normal file
36
tools/pr/templates/duplicate-title-ask.md
Normal file
|
|
@ -0,0 +1,36 @@
|
||||||
|
<!--
|
||||||
|
Template: duplicate-title ask
|
||||||
|
|
||||||
|
This file is an aesthetic reference, not a fill-in form. When you need to
|
||||||
|
post this kind of comment, read the beats and the exemplar below to absorb
|
||||||
|
the tone, then compose a fresh comment for the specific PR pair. Do not
|
||||||
|
sed-substitute the placeholders and post the result verbatim — see
|
||||||
|
maintainer memory `feedback_templates_are_style_refs`.
|
||||||
|
|
||||||
|
When this applies:
|
||||||
|
classify emits the `duplicate-title` tag (same author + byte-for-byte
|
||||||
|
identical title with another open PR). Post on the older / more-iterated
|
||||||
|
PR of the pair; the author may want to preserve its history.
|
||||||
|
|
||||||
|
Beats the comment should hit (in order):
|
||||||
|
1. Brief, neutral observation that two same-titled PRs exist.
|
||||||
|
2. List both PRs with one-line distinguishing facts (branch + commits + diff size).
|
||||||
|
3. Direct ask: which one to land?
|
||||||
|
4. Offer to close on the author's behalf if #other is the intended one.
|
||||||
|
5. Invite a clarifying note if both are intentional.
|
||||||
|
6. Closing line attributing the surface to maintainer tooling.
|
||||||
|
|
||||||
|
Placeholders mark the PR-specific facts to weave in (not literal blanks):
|
||||||
|
{AUTHOR}, {TITLE}, {THIS_NUM} / {THIS_BRANCH} / {THIS_COMMITS} / {THIS_PLUS} /
|
||||||
|
{THIS_MINUS}, and the same set for {OTHER_*}.
|
||||||
|
|
||||||
|
Exemplar (one valid way to write it):
|
||||||
|
-->
|
||||||
|
@{AUTHOR} — flagging this one because the open queue currently has two PRs from you with byte-for-byte identical titles (`{TITLE}`):
|
||||||
|
|
||||||
|
- this PR (#{THIS_NUM}, branch `{THIS_BRANCH}`, {THIS_COMMITS} commits, +{THIS_PLUS} −{THIS_MINUS})
|
||||||
|
- #{OTHER_NUM} (branch `{OTHER_BRANCH}`, {OTHER_COMMITS} commits, +{OTHER_PLUS} −{OTHER_MINUS})
|
||||||
|
|
||||||
|
Could you confirm which one you'd like reviewers to land? If #{OTHER_NUM} is meant to supersede this one, feel free to close this in favor of it (or let me know and I'll close it). If they're intentionally covering different cases, a one-line note here clarifying the split would help reviewers track them.
|
||||||
|
|
||||||
|
(Surfaced by maintainer tooling: `duplicate-title` tag on same-author byte-for-byte titles.)
|
||||||
|
|
@ -0,0 +1,53 @@
|
||||||
|
<!--
|
||||||
|
Frozen-in-time snapshot — captured 2026-05-11 against PR #1009 at the
|
||||||
|
state it had on that day. Prior reviewer findings, file paths, line
|
||||||
|
numbers, and the CHANGES_REQUESTED state reflect that moment and will
|
||||||
|
not be updated when the PR or the underlying code evolves. Kept here as
|
||||||
|
a style-shape exemplar for the `agent-review.md` template
|
||||||
|
(CHANGES_REQUESTED with prior human review case).
|
||||||
|
-->
|
||||||
|
|
||||||
|
# Agent review · PR #1009
|
||||||
|
|
||||||
|
## Identification
|
||||||
|
|
||||||
|
- Title: `feat(daemon): load user design systems from ~/.open-design/design-systems`
|
||||||
|
- Author: mathd (external)
|
||||||
|
- Status: OPEN · **CHANGES_REQUESTED** · DIRTY (no push since reviews)
|
||||||
|
- Labels: `size/M`, `risk/medium`, `type/feature`
|
||||||
|
- Lane: `default` (no forbidden surfaces, no public seams touched)
|
||||||
|
- Diff: +213 / −12 across 5 files
|
||||||
|
|
||||||
|
## Prior reviewer findings (still applicable)
|
||||||
|
|
||||||
|
Two human MEMBER reviews on file, both still unaddressed in the current head:
|
||||||
|
|
||||||
|
- **@lefarcen** (`CHANGES_REQUESTED`): "one critical path traversal vulnerability plus several edge-case gaps". `readDesignSystem(root, id)` and the new `readDesignSystemFromAny(bundledRoot, userRoot, id)` (`apps/daemon/src/design-systems.ts:54-71`) both do `path.join(root, id, 'DESIGN.md')` with no validation that `id` stays inside `root`. An `id` like `../../../etc/passwd` escapes the design-systems directory if reachable from user input.
|
||||||
|
- **@mrcfps** (`COMMENTED`): `OD_USER_STATE_DIR` override is non-deterministic for tests/programmatic callers, and `userDesignSystemsDir()` falling back to `~/.open-design/design-systems` causes isolated finalize runs to silently read host-local design systems.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
|
||||||
|
### Test isolation risk
|
||||||
|
|
||||||
|
`apps/daemon/tests/design-systems.test.ts` adds 104 lines. If those cases don't explicitly set `OD_USER_STATE_DIR` to a tmpdir, they hit the developer's real `~/.open-design/design-systems` and become non-reproducible across machines — same shape as @mrcfps's host-leak concern but in test scope.
|
||||||
|
|
||||||
|
### Severity vs label
|
||||||
|
|
||||||
|
`risk/medium` likely undersells the path-traversal severity from finding above. Worth re-labeling `risk/high` once author addresses the traversal, since the fix path itself is security-sensitive.
|
||||||
|
|
||||||
|
## Suggestions
|
||||||
|
|
||||||
|
- Path-traversal fix should validate `id` against a strict slug regex (e.g. `/^[a-z0-9-]+$/`, matching the existing `non-ascii-slug` lane convention) **at the call site closest to user input**, not deep inside `readDesignSystem` — preserves the low-level "trust the caller" contract while moving validation to the boundary.
|
||||||
|
- Pick one isolation strategy and apply it consistently: either every test sets `OD_USER_STATE_DIR=<tmpdir>`, or the daemon takes a `userStateDir` config knob whose default is the home-dir fallback. Mixing the two leaves footguns.
|
||||||
|
- Consider splitting the path-traversal fix into a security-only PR ahead of the feature — the traversal is exploitable as soon as `id` reaches the function from a user-facing route, regardless of whether the user-overlay feature ships.
|
||||||
|
|
||||||
|
## Validation expected (after author addresses CHANGES_REQUESTED items)
|
||||||
|
|
||||||
|
```bash
|
||||||
|
pnpm guard
|
||||||
|
pnpm typecheck
|
||||||
|
pnpm --filter @open-design/daemon typecheck
|
||||||
|
pnpm --filter @open-design/daemon test
|
||||||
|
```
|
||||||
|
|
||||||
|
Test re-runs should include a path-traversal regression (`readDesignSystem(root, '../etc/passwd')` returns `null` or throws) and an `OD_USER_STATE_DIR=<tmpdir>` isolation case.
|
||||||
|
|
@ -0,0 +1,56 @@
|
||||||
|
<!--
|
||||||
|
Frozen-in-time snapshot — captured 2026-05-11 against PR #1037 at the
|
||||||
|
state it had on that day. File paths, line numbers, status, prior review
|
||||||
|
references, and SHAs reflect that moment and will not be updated when the
|
||||||
|
PR or the underlying code evolves. Kept here as a style-shape exemplar
|
||||||
|
for the `agent-review.md` template (scope-expansion case).
|
||||||
|
-->
|
||||||
|
|
||||||
|
# Agent review · PR #1037
|
||||||
|
|
||||||
|
## Identification
|
||||||
|
|
||||||
|
- Title: `fix(daemon): follow symlinked skill and design-system dirs`
|
||||||
|
- Author: desmond-rai (external)
|
||||||
|
- Status: OPEN · REVIEW_REQUIRED · DIRTY · **draft**
|
||||||
|
- Labels: `size/S`, `risk/high`, `type/bugfix`
|
||||||
|
- Lane: `default` (no forbidden surfaces, no public seams touched)
|
||||||
|
- Diff: +899 / −3 across 4 files
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
|
||||||
|
### Scope expansion vs declared intent
|
||||||
|
|
||||||
|
The PR title declares a symlink-follow fix. The diff actually contains three distinct changes:
|
||||||
|
|
||||||
|
| Change | Files | Lines | Matches PR title? |
|
||||||
|
|---|---|---|---|
|
||||||
|
| Symlink-follow helper + call sites | `apps/daemon/src/design-systems.ts`, `apps/daemon/src/skills.ts` | ~50 | ✓ |
|
||||||
|
| Chat-run inactivity timeout `2m → 20m` | `apps/daemon/src/server.ts:1819-1824` | 3 | ✗ unrelated |
|
||||||
|
| Hardcoded `renderReturningAiShowcase` route + 837-line implementation | `apps/daemon/src/design-system-showcase.ts:107-128, 549-1402` | 853 | ✗ unrelated, vendor-specific |
|
||||||
|
|
||||||
|
`code-review-guidelines.md §3 Out of scope` lists "Piggybacks unrelated cleanup, formatting, dependency churn, migrations, or feature work onto a focused fix" as a block-or-split condition.
|
||||||
|
|
||||||
|
### Product-relevance test
|
||||||
|
|
||||||
|
`renderReturningAiShowcase` is triggered by `id === 'returning-ai' || /ReturningAI|CFD-brokerage/i.test(raw)` and renders a vendor-specific design-system showcase. The PR body doesn't name an Open Design capability this branch validates. `code-review-guidelines.md §1` lists "customer vertical / marketing experiment / unrelated rendering demo / arbitrary product page" as out-of-scope unless first-party and motivated, and this matches.
|
||||||
|
|
||||||
|
### Symlink fix itself is clean
|
||||||
|
|
||||||
|
`entryIsDirectoryFollowingLinks` (duplicated 14 lines in `design-systems.ts:33-46` and `skills.ts:94-107`, well-commented) is the correct fix for `Dirent.isDirectory()` returning false on symlinks. Broken links fall through `try/catch` and are skipped — identical end-state to the previous filter. No regression test for the symlink-as-directory path. lefarcen's prior `COMMENTED` review at SHA `b86885c0` predates the scope-expanding commits.
|
||||||
|
|
||||||
|
## Suggestions
|
||||||
|
|
||||||
|
- Ask the author to split: land the symlink fix as a standalone PR (`design-systems.ts` + `skills.ts` + one regression test), and bring the timeout and vendor-specific showcase back as separately motivated PRs or drop them.
|
||||||
|
- The 2m → 20m chat timeout has real resource implications (10× idle window); if the author wants to keep it, it needs its own motivation. The vendor-specific showcase fails `§1` as written and is the more disposable of the two.
|
||||||
|
|
||||||
|
## Validation expected (for the trimmed symlink-fix-only scope)
|
||||||
|
|
||||||
|
```bash
|
||||||
|
pnpm guard
|
||||||
|
pnpm typecheck
|
||||||
|
pnpm --filter @open-design/daemon typecheck
|
||||||
|
pnpm --filter @open-design/daemon test
|
||||||
|
```
|
||||||
|
|
||||||
|
Plus a regression test under `apps/daemon/tests/` that creates a temp dir with a symlinked subdir and confirms `listSkills` / `listDesignSystems` discover it.
|
||||||
|
|
@ -0,0 +1,55 @@
|
||||||
|
<!--
|
||||||
|
Frozen-in-time snapshot — captured 2026-05-11 against PR #1149 at the
|
||||||
|
state it had on that day. File paths, line numbers, status, and other
|
||||||
|
PR-specific facts reflect that moment and will not be updated when the
|
||||||
|
PR or the underlying code evolves. Kept here as a style-shape exemplar
|
||||||
|
for the `agent-review.md` template (clean contract-lane feature case).
|
||||||
|
-->
|
||||||
|
|
||||||
|
# Agent review · PR #1149
|
||||||
|
|
||||||
|
## Identification
|
||||||
|
|
||||||
|
- Title: `feat(daemon): deploy linked HTML pages together as a multi-page site on Vercel with clean URLs (#1077)`
|
||||||
|
- Author: ButterHost69 (external)
|
||||||
|
- Status: OPEN · REVIEW_REQUIRED · BLOCKED (branch-protection, not a code blocker)
|
||||||
|
- Labels: `size/M`, `risk/high`, `type/feature`
|
||||||
|
- Lane: `contract` (public seam: `packages/contracts`)
|
||||||
|
- Diff: +326 / −11 across 5 files
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
|
||||||
|
### Contract surface is clean
|
||||||
|
|
||||||
|
Two optional fields added to existing request DTOs (`DeployProjectFileRequest.linkedPages?: string[]` and `DeployPreflightRequest.linkedPages?: string[]`). `packages/contracts/src/api/projects.ts` stays pure TS (no new imports). Daemon consumer + 130-line `deploy.test.ts` land in the same PR. Meets `code-review-guidelines.md §4.2` (contract change lands with consumer + tests, additive only, no migration window needed).
|
||||||
|
|
||||||
|
### BFS implementation
|
||||||
|
|
||||||
|
`collectLinkedHtmlPages` discovers `<a href>` links to local HTML; the iterative `while (linkedPageIdx < linkedPages.length)` loop in `apps/daemon/src/deploy.ts:44-102` handles transitive chains correctly and is cycle-safe via `visited.has(page.path)`. The `page.deployedName === 'index.html'` guard at lines 81-85 explicitly throws `DeployError(400)` to prevent overwriting the entry page — defensive and well-placed. `vercel.json` injection at lines 127-135 is scoped to Vercel deploys only and respects user-supplied `vercel.json`.
|
||||||
|
|
||||||
|
### Loose ends
|
||||||
|
|
||||||
|
- `apps/web/src/providers/registry.ts` 1-line change is unmotivated by the PR body — worth confirming it's required by the feature, not a stale touch from a rebase.
|
||||||
|
- `collectLinkedHtmlPages` (lines 1466-1487) resolves paths via `resolveReferencedPath` (out of diff). Whether `../` escapes are rejected depends on that helper — given the `risk/high` label and the new caller paths exercising it, a path-traversal regression test would be appropriate.
|
||||||
|
|
||||||
|
## Suggestions
|
||||||
|
|
||||||
|
- Add a hard cap on transitively discovered HTML pages (e.g. > 50 → `DeployError`). The BFS is currently unbounded; a malformed project with a wide link graph could blow up a deploy.
|
||||||
|
- Document the `vercel.json` `cleanUrls: true` injection in release notes — it changes routing semantics on the deployed site, and users with existing `.html`-URL expectations need to know.
|
||||||
|
- Add a path-traversal test alongside the new 130 lines (an `<a href="../../../etc/passwd.html">` case asserting `collectLinkedHtmlPages` rejects or normalizes).
|
||||||
|
|
||||||
|
## Validation expected
|
||||||
|
|
||||||
|
```bash
|
||||||
|
pnpm guard
|
||||||
|
pnpm typecheck
|
||||||
|
pnpm --filter @open-design/contracts typecheck
|
||||||
|
pnpm --filter @open-design/daemon typecheck
|
||||||
|
pnpm --filter @open-design/daemon test
|
||||||
|
pnpm --filter @open-design/daemon build
|
||||||
|
pnpm --filter @open-design/web typecheck
|
||||||
|
pnpm --filter @open-design/web test
|
||||||
|
pnpm --filter @open-design/web build
|
||||||
|
```
|
||||||
|
|
||||||
|
Plus a manual sandbox smoke: deploy a 3-page linked-HTML project to a test Vercel target, confirm clean-URL routing and that the entry-page index resolution still works.
|
||||||
20
tools/pr/tsconfig.json
Normal file
20
tools/pr/tsconfig.json
Normal file
|
|
@ -0,0 +1,20 @@
|
||||||
|
{
|
||||||
|
"compilerOptions": {
|
||||||
|
"allowSyntheticDefaultImports": true,
|
||||||
|
"esModuleInterop": true,
|
||||||
|
"exactOptionalPropertyTypes": true,
|
||||||
|
"forceConsistentCasingInFileNames": true,
|
||||||
|
"isolatedModules": true,
|
||||||
|
"lib": ["ES2024"],
|
||||||
|
"module": "NodeNext",
|
||||||
|
"moduleResolution": "NodeNext",
|
||||||
|
"noEmit": true,
|
||||||
|
"noUncheckedIndexedAccess": true,
|
||||||
|
"resolveJsonModule": true,
|
||||||
|
"skipLibCheck": true,
|
||||||
|
"strict": true,
|
||||||
|
"target": "ES2024",
|
||||||
|
"types": ["node"]
|
||||||
|
},
|
||||||
|
"include": ["src/**/*.ts", "tests/**/*.ts", "esbuild.config.mjs"]
|
||||||
|
}
|
||||||
Loading…
Reference in a new issue