mirror of
https://github.com/nexu-io/open-design.git
synced 2026-10-27 09:51:12 +01:00
codex/add-codebuddy-pet
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 PerishFireandGitHub
|
819c34fd8f |
fix(tools-pr): fall back on reviewDecision for unresolved-changes-requested (#1287)
* fix(tools-pr): fall back on reviewDecision for unresolved-changes-requested Patrol classify on the live 102-PR queue missed three PRs (#1101, #1127, #1163) where GitHub's reviewDecision is CHANGES_REQUESTED but the classify tag did not fire. Root cause is a divergence between two notions of "latest review state per reviewer": - GitHub's reviewDecision keeps a reviewer's CHANGES_REQUESTED in effect until that same reviewer submits APPROVED or DISMISSED. A subsequent COMMENTED review by the same reviewer does NOT supersede it. - Our `reduceLatestReviewsByAuthor` collapses every reviewer to their latest review with no special-casing of state, so a CHANGES_REQUESTED followed by COMMENTED disappears from the reduced view. `tagUnresolvedChangesRequested` filtered the reduced view for `state === "CHANGES_REQUESTED"`, so the three PRs above (each had a reviewer write CHANGES_REQUESTED → COMMENTED) escaped the rule even though the PR-level reviewDecision was still CHANGES_REQUESTED. Add a narrow fallback: when the first path returns no per-reviewer reviewers, trust `facts.reviewDecision === "CHANGES_REQUESTED"` as the source of truth. The fallback reason and source token differ from the first path so report consumers can tell which signal fired. Reducer semantics left alone on purpose — flipping COMMENTED handling there would cascade to `bot-only-approval`, `stale-approval`, and `humanReviewerSignalAt`, each of which has its own correctness story. * fix(tools-pr): keep fallback reason strictly factual Codex flagged that the fallback path's reason text asserted a specific review sequence ("CHANGES_REQUESTED then COMMENTED") that the condition alone does not prove. The condition only observes: - `facts.reviewDecision === "CHANGES_REQUESTED"`, and - after `reduceLatestReviewsByAuthor`, no review carries `state === "CHANGES_REQUESTED"`. Multiple GitHub configurations satisfy that pair — a reviewer's CR followed by COMMENTED, a CR that sits outside the `reviews(last: 30)` fetch window, etc. Per `tools/pr/AGENTS.md`'s strictly-factual rule, the reason must report only what is directly observed, not the most likely upstream cause. Drop the inferred-cause clause from `reason`; move the explanation of possible upstream causes into the code comment above the branch where it does not show up in classify output. * docs(tools-pr): document fallback data source for unresolved-changes-requested Siri-Ray and lefarcen both flagged that the tag dictionary row for `unresolved-changes-requested` only describes the primary per-reviewer path. The fallback added earlier in this PR emits the same tag with a different `source` token (`gh.reviewDecision` vs the original `gh.latestReviews[].state`), so report consumers need the dictionary to list both paths to interpret which one fired. Update the row to call out both: the primary per-reviewer rule, and the PR-level reviewDecision fallback that fires when no per-reviewer CR survives the latest-per-author reduction. The two-token source column mirrors the actual `Tag.source` strings emitted at runtime. * test(tools-pr): pin both emission paths of unresolved-changes-requested lefarcen flagged the fallback was validated only by live-PR examples in the PR body, so a refactor could silently regress the coverage. Add a deterministic test file `tests/tags-unresolved-cr.test.ts` that exercises `classifyPr` against crafted `PrFacts` fixtures: - primary path (per-reviewer CR after reduction) fires with source=gh.latestReviews[].state and surfaces the reviewer login - fallback path fires with source=gh.reviewDecision when no per-reviewer CR survives reduction (covers both the COMMENTED-follow-up shape and the empty-reviews shape — the latter pins the `reviews(last: 30)` out-of-window concern from the factual-reason fix) - primary wins over fallback when both signals are present (single tag emitted, source=gh.latestReviews[].state) - two negative cases: empty reviewDecision and APPROVED — neither emits Also extend the fallback's code comment with the observed scale (3 of 102 open PRs hit this gap: #1101, #1127, #1163) so future maintainers can tell this is a recurring queue pattern, not a theoretical edge case. This is the first test under `tools/pr/tests/`; the package test script already ran `node --import tsx --test tests/*.test.ts` against an empty glob, so no scaffolding changes are needed. |
||
|
PerishFireandGitHub
|
c3d41c7d45 |
fix(tools-pr): chunk stats fetch through cursor-paginated GraphQL (#1285)
`fetchOpenPrs` was reading the stats chunk via `gh pr list --limit 1000 --json mergeStateStatus,...`. With the default limit raised to 1000 in #1259, this 502s reliably on the live open queue (107 PRs): GitHub's GraphQL gateway has to recompute mergeStateStatus for every PR up front, and the resulting query exceeds the gateway budget once the requested page passes ~60 PRs. Switch the stats chunk to `fetchPaginatedPrList`, the same cursor- paginated GraphQL helper that already drives reviews / comments / commits / assignment-timelines. Page size stays at PR_LIST_PAGE_SIZE (30), well within the gateway budget, and the heavy stats fetch is now consistent with the other heavy chunks. Verified locally: `pnpm tools-pr list` now completes against the live 107-PR queue without a 502. |
||
|
PerishFireandGitHub
|
8c0fb8dc01 |
feat(tools-pr): add maintainer PR-duty workspace (#1259)
* feat(tools-pr): add maintainer PR-duty workspace
Adds `tools/pr` as the maintainer-only control plane for PR-duty work on
this repo. Thin `gh` wrapper that encodes repo-specific knowledge:
review lanes, forbidden surfaces, lane-specific checklists, validation
command derivation from touched packages.
Subcommands:
- `list` — triage open queue by lane and review-state bucket.
- `view <num>` — agent-friendly review brief for a single PR.
- `classify [num]` — emit script-level tags for one PR or the whole
open queue; full-queue JSON output lands under `.tmp/tools-pr/classify/`
with rate-limit telemetry per run.
- `assignment` — assigner-perspective view of PR ownership, idle time,
and blockers (derived from existing tags; no new judgments).
Tag dictionary (13 tags) covers: bot-only-approval, needs-rebase,
forbidden-surface, unlabeled, duplicate-title, non-ascii-slug,
maintainer-edits-disabled, org-member, unresolved-changes-requested,
stale-approval, and three awaiting-* timing tags. Each rule is
expressible as one factual sentence over `gh` data + repo paths — see
`tools/pr/AGENTS.md` for the full dictionary plus precision rules.
Templates in `tools/pr/templates/*.md` are aesthetic references for
recurring maintainer comments (duplicate-title ask, awaiting-author
nudge, agent-review brief shape). `templates/examples/` holds
frozen-in-time agent-review snapshots for three PR shapes.
Infrastructure:
- `gh()` wraps `execFile` with minimum-touch retry (2 attempts at 1s + 2s
backoff) on transient 5xx / network errors. Persistent failures still
surface — retry is anti-jitter, not an exponential-backoff resilience
layer.
- Heavy chunks (`reviews`, `comments`, `commits`, assignment timelines)
use cursor-paginated `gh api graphql` via `fetchPaginatedPrList` to
stay under GitHub's GraphQL server-side timeout. Light chunks stay on
`gh pr list --json`.
- `fetchOrgMembers` cached per process via `gh api orgs/<owner>/members
--paginate`.
Wiring:
- Root `package.json` adds `pnpm tools-pr` to the allowed root entry
points.
- `scripts/postinstall.mjs` builds `tools/pr` alongside other workspace
packages.
- `scripts/guard.ts` allowlists `tools/pr/bin/tools-pr.mjs` and
`tools/pr/esbuild.config.mjs`, and adds `pr/` to the `tools/` top-level
layout allowlist.
- Root `AGENTS.md` and `tools/AGENTS.md` document the new command
surface, root-command-boundary update, and per-tool ownership.
* docs(agents): brief tools-pr in root AGENTS.md, link to tools/pr/AGENTS.md
Adds a `PR-duty tooling` section to the root AGENTS.md summarising what
`pnpm tools-pr` is, listing the four common subcommands (list / view /
classify / assignment), and pointing readers to `tools/pr/AGENTS.md` for
the full tag dictionary, operational playbook, templates, and design
rules. The section keeps root-level guidance to high-level orientation
while details stay local to the tool's own AGENTS.md.
* fix(tools-pr): drop overly broad touches-root-package.json forbidden hit
`deriveForbidden` was flagging any change to root `package.json` as a
forbidden-surface hit, but AGENTS.md §Root command boundary only forbids
specific *lifecycle* aliases (pnpm dev / test / build / daemon / preview
/ start) — tools-control-plane entrypoints like `pnpm tools-pr` are
explicitly allowed. Distinguishing "forbidden alias" from "allowed
entry" requires reading the diff content, which is `pnpm guard`'s job
rather than a path-derived classify tag.
Dogfooded on this branch's own PR (#1259), which added the `pnpm
tools-pr` script and was incorrectly flagged. Removing the hit aligns
the `forbidden-surface` tag with what tools-pr can mechanically detect
from file paths alone (apps/nextjs/, packages/shared/).
* fix(tools-pr): paginate commits fetch, recognise ready-to-merge, escape title-index separator
Three review follow-ups on #1259, all factual fixes:
- `fetchOpenPrCommits` now uses `fetchPaginatedPrList` instead of a
one-shot `pullRequests(first: $first)` query. GitHub GraphQL caps
connection page size at 100, so the previous implementation would
fail at runtime when callers passed `--limit > 100`. The paginated
path makes the commits fetch consistent with the other heavy chunks
(reviews, comments, assignment timelines) and removes the artificial
ceiling entirely. The `limit` parameter is dropped from
`fetchOpenPrCommits`; the CLI `--limit` continues to bound the
`gh pr list --json` chunks.
- `deriveStatus` in `assignment.ts` now reads `facts.reviewDecision`
and `facts.mergeStateStatus`. When the PR is `APPROVED` with merge
state `CLEAN` or `UNSTABLE` and carries no blockers, status renders
as `ready to merge` instead of falling through to `in review`. The
assignment view loses its main triage signal without this — a clean
human-approved PR rendered identical to a REVIEW_REQUIRED one.
- `tags.ts:tagDuplicateTitle` and `tags.ts:buildContext` both
constructed the title-index key with a literal NUL byte between
author and title, which made the file appear as binary in `git diff`
/ review tooling. Replaced the literal byte with a Unicode escape
sequence in source; the runtime string value is identical, the
source stays plain text and round-trips through review tooling
cleanly.
* fix(tools-pr): raise default --limit to 1000 to cover the live open queue
mrcfps flagged that `tools-pr list` (and `classify --all`, `assignment`)
defaults to `--limit 100`, which silently drops every PR past the first
100 in the open queue. The repo currently sits at 104 open PRs, so the
out-of-the-box run was already omitting four PRs.
Raise the default to 1000 in `list.ts`, `classify.ts`, and `assignment.ts`,
and remove the now-pointless 200 ceiling — `gh pr list --limit N` paginates
internally, so a high cap is cheap. Users can still pass `--limit <small>`
for a truncated preview. CLI help text on the three subcommands updated to
match.
* fix(web): pass designTemplates to ProjectView render helper
#955 made `designTemplates` a required Prop on ProjectView, but the test
helper added in #1244 (`renderProjectView` in
`ProjectView.api-empty-response.test.tsx`) was never updated. The two
PRs landed on main without conflicting, leaving `apps/web` typecheck red
for every PR that rebases past
|