Skip to content

tools(github): poll-pr-gate.ts v0 — promote prose-jq to executable (5-AI convergent)#921

Merged
AceHack merged 8 commits intomainfrom
ops/poll-pr-gate-executable-script-v0-2026-04-30
Apr 30, 2026
Merged

tools(github): poll-pr-gate.ts v0 — promote prose-jq to executable (5-AI convergent)#921
AceHack merged 8 commits intomainfrom
ops/poll-pr-gate-executable-script-v0-2026-04-30

Conversation

@AceHack
Copy link
Copy Markdown
Member

@AceHack AceHack commented Apr 30, 2026

Summary

Closes part 1 of task #355 — the highest-leverage hardening item this session. 5-AI peer-reviewer convergence on 2026-04-30 named the recommendation: promote the inline jq snippets in memory/feedback_amara_poll_gate_not_ending_holding_is_not_status_2026_04_30.md into a tested executable. Full attribution at docs/research/2026-04-30-multi-ai-feedback-packets-this-session.md.

Carved blade from that packet:

"If the loop uses it every tick, it deserves tests."

What's in v0

  • tools/github/poll-pr-gate.ts — Bun+TS skeleton with happy-path query
  • tools/github/fixtures/clean-armed-auto-merge.json — clean gate + auto-merge-armed shape
  • tools/github/fixtures/blocked-by-threads.json — clean-CI + 3 unresolved threads shape
  • tools/github/fixtures/status-context-error.json — StatusContext ERROR + EXPECTED states
  • tools/github/fixtures/behind-needs-rebase.json — BEHIND mergeStateStatus → rebase action

Output shape

{
  "number": 917,
  "state": "OPEN" | "MERGED" | "CLOSED",
  "gate": "CLEAN" | "BLOCKED" | "DIRTY" | "UNSTABLE" | "UNKNOWN",
  "checks": { "ok": N, "inProgress": N, "pending": N, "failed": N },
  "unresolvedThreads": N,
  "autoMerge": "armed" | "none",
  "mergeCommit": "<sha>" | null,
  "nextAction": "wait-ci" | "fix-failed-checks" | "resolve-threads" | "rebase" | "verify-merge" | "none"
}

Required-check semantics

  • Merge-satisfying: SUCCESS, NEUTRAL, SKIPPED
  • Blocking: FAILURE, CANCELLED, TIMED_OUT, STARTUP_FAILURE, ACTION_REQUIRED, STALE, ERROR
  • Pending: QUEUED, PENDING, IN_PROGRESS, EXPECTED, REQUESTED, WAITING

Exit codes

  • 0 — query succeeded, JSON emitted
  • 1 — invocation / argument / dependency error (bad args, gh missing, fixture missing, PR ≤ 0)
  • 2 — gh CLI returned non-zero (auth, rate-limit, PR not found)
  • 3 — gh CLI output couldn't be parsed (truncated, non-JSON)

Verification

Tested live against multiple PRs and offline against 4 fixtures. All cases produce the expected report shape.

Why this matters

Aaron's substrate-IS-product framing (2026-04-30) — executable substrate IS substrate-quality work. The factory's tooling-product deserves the same honest-substrate discipline as the substrate-product. The memory file should stop being the implementation and start pointing to it. Slice 22 of B-0086 TS+Bun migration trajectory.

Follow-ups (queued under task #355)

  • Matrix fixture coverage (mixed-rollup, missing-conclusion, pagination edges)
  • Test runner integration with existing TS test harness
  • Top-of-memory-file pointer addition (after this PR merges)
  • Loop integration: replace inline jq calls in the autonomous-loop tick close

🤖 Generated with Claude Code

…-AI convergent)

Closes part 1 of task #355. 5-AI convergence (Amara 2nd, Deepseek 4th,
Alexia 5th, Ani 3rd, Gemini 4th — all 2026-04-30) on promoting the
inline jq snippets in
`memory/feedback_amara_poll_gate_not_ending_holding_is_not_status_2026_04_30.md`
into a tested executable. Amara's blade: *"if the loop uses it every
tick, it deserves tests."*

This is **v0** — skeleton + minimal happy-path query. Works live
against `gh pr view --json` + a paired `gh api graphql` call for
review threads. Fixture mode for offline testing.

Output shape per Amara's spec:

```json
{
  "number": 917,
  "state": "OPEN",
  "gate": "CLEAN" | "BLOCKED" | "DIRTY" | "UNSTABLE" | "UNKNOWN",
  "checks": { "ok": N, "inProgress": N, "pending": N, "failed": N },
  "unresolvedThreads": N,
  "autoMerge": "armed" | "none",
  "mergeCommit": "<sha>" | null,
  "nextAction": "wait-ci" | "resolve-threads" | "rebase" | "verify-merge" | "none"
}
```

Required-check semantics (per Amara 2nd's GitHub-docs verification):

- Merge-satisfying: `SUCCESS`, `NEUTRAL`, `SKIPPED`
- Blocking: `FAILURE`, `CANCELLED`, `TIMED_OUT`, `STARTUP_FAILURE`,
  `ACTION_REQUIRED`, `STALE`
- Pending: `QUEUED`, `PENDING`, `IN_PROGRESS`

Verified against:
- Live PR #915 (just merged) → state=MERGED, gate=CLEAN,
  nextAction=verify-merge
- Live PR #919 (just merged) → state=MERGED, gate=CLEAN,
  nextAction=verify-merge
- Fixture clean-armed-auto-merge → gate=BLOCKED,
  nextAction=none (auto-merge does the babysitting)
- Fixture blocked-by-threads → gate=BLOCKED, unresolvedThreads=3,
  nextAction=resolve-threads

Two fixtures land with v0; matrix coverage (CheckRun SUCCESS/SKIPPED/
NEUTRAL/STALE × StatusContext × pending × mixed × missing-conclusion)
follows in subsequent slices. Per substrate-rate this is a v0 commit;
expanding fixtures and adding a test runner are queued under task #355.

The memory file should stop being the implementation. It now points
to this file. Subsequent PR will add a top-of-memory pointer.

Composes with Aaron's substrate-IS-product framing — executable
substrate IS substrate-quality work; the factory's tooling-product
deserves the same honest-substrate discipline as the substrate-
product. Slice 22 of the TS+Bun migration trajectory (B-0086).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 30, 2026 15:15
@AceHack AceHack enabled auto-merge (squash) April 30, 2026 15:15
…liance

CI lint(tsc tools) caught 4 type errors in parseArgs caused by the
repo's strict tsconfig (`exactOptionalPropertyTypes: true` +
`noUncheckedIndexedAccess: true`):

- `argv[++i]` returns `string | undefined` under noUncheckedIndexedAccess
- The return-object literal with `{ fixture: string | undefined, ... }`
  doesn't satisfy `{ fixture?: string }` under exactOptionalPropertyTypes

Fix: build the return object incrementally, only assigning the
optional fields when their value is actually defined. Hoist the
shape into a named `ParsedArgs` interface for clarity.

This is exactly the kind of catch the dogfood-self-test would have
caught locally if I'd run tsc before pushing — slot for a pre-push
typecheck hint in a follow-up.

Local verification:
  $ bunx tsc --noEmit -p . | grep poll-pr-gate
  (no output)
  $ bun tools/github/poll-pr-gate.ts --fixture tools/github/fixtures/clean-armed-auto-merge.json
  (correct output)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 251a3a7cb1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/github/poll-pr-gate.ts Outdated
Comment thread tools/github/poll-pr-gate.ts Outdated
… + fix-failed-checks action (Codex P1×2)

Two real defects from Codex P1 review on PR #921 v0:

1. **Pagination missing** (line 209): query was `reviewThreads(first:50)`
   which truncates discussion-heavy PRs. Switched to
   `gh api graphql --paginate` with `pageInfo{hasNextPage endCursor}`
   + `after:$endCursor` cursor. Aggregates nodes across all pages.

2. **StatusContext EXPECTED/ERROR not classified** (line 236): the
   normalization treated every non-PENDING state as COMPLETED + raw
   conclusion, but classifyChecks's OK_CONCLUSIONS / BLOCKING_CONCLUSIONS
   sets didn't include `ERROR`. EXPECTED states (StatusContext "queued"
   equivalent) weren't mapped to pending either. Real defect: tools
   would silently miss CI errors on StatusContext-class checks.

   Fix:
   - Added `ERROR` to BLOCKING_CONCLUSIONS
   - Added `EXPECTED` to PENDING_STATE_LITERALS (maps to status=PENDING)
   - Extracted normalization into `normalizeRollup()` so fixture-mode
     and live-mode classify identically (caught only because dogfooding
     against PR #921 itself revealed live had different shape than
     fixtures)

3. **Bonus: fix-failed-checks vs resolve-threads action distinction**
   — previously both code paths returned `resolve-threads` whether the
   block was failed CI or unresolved review threads. Added explicit
   `fix-failed-checks` action so the agent gets a precise next-step
   indicator.

Added a third fixture `status-context-error.json` covering ERROR +
EXPECTED states. Now classifies as: 1 ok / 1 pending / 1 failed →
nextAction=fix-failed-checks. All three fixtures + live PR #921 verified.

The dogfood pattern is working — the Codex flag landed because the
script was running and producing inspectable output that revealed
gaps the reviewer's static analysis caught.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
auto-merge was automatically disabled April 30, 2026 15:22

Pull Request is not mergeable

@AceHack AceHack enabled auto-merge (squash) April 30, 2026 15:23
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an initial Bun+TypeScript implementation of the “poll PR gate” logic used by the autonomous loop, moving the previously-prose/jq approach toward an executable tool with fixture inputs.

Changes:

  • Introduces tools/github/poll-pr-gate.ts to query PR state/checks/threads via gh and emit a normalized JSON report.
  • Adds two JSON fixtures to support offline iteration of the classification logic.
  • Documents intended output shape and required-check conclusion semantics in the script header.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.

File Description
tools/github/poll-pr-gate.ts New Bun/TS CLI that queries GitHub PR gate signals and emits a JSON report.
tools/github/fixtures/clean-armed-auto-merge.json Fixture representing a clean-checks + auto-merge-armed PR shape.
tools/github/fixtures/blocked-by-threads.json Fixture representing a PR blocked by unresolved review threads.

Comment thread tools/github/poll-pr-gate.ts
Comment thread tools/github/poll-pr-gate.ts
Comment thread tools/github/poll-pr-gate.ts
Comment thread tools/github/poll-pr-gate.ts
Comment thread tools/github/poll-pr-gate.ts Outdated
Comment thread tools/github/poll-pr-gate.ts
Comment thread tools/github/poll-pr-gate.ts
Comment thread tools/github/poll-pr-gate.ts
Comment thread tools/github/poll-pr-gate.ts Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 27e63d6243

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/github/poll-pr-gate.ts
Comment thread tools/github/poll-pr-gate.ts
AceHack and others added 2 commits April 30, 2026 11:27
…ion + flag-value validation + doc typo (Copilot P0+P1×4)

Five Copilot-flagged real defects from PR #921 review pass on the
prior commit (a7b8e26):

1. **BEHIND mergeStateStatus not handled** (Copilot P0, line 137)
   — `BEHIND` (base advanced past PR's merge-base — needs rebase)
   was unhandled in classifyGate, causing it to fall through to
   UNKNOWN. Added: `BEHIND` and `DIRTY` both produce gate=DIRTY,
   nextAction=rebase. Verified via new `behind-needs-rebase.json`
   fixture.

2. **spawnSync launch failure not distinguished from gh non-zero**
   (Copilot P1, line 200) — when `gh` is missing from PATH or
   couldn't be launched (ENOENT etc), spawnSync sets `result.error`
   but `result.status` is null, which my prior `status !== 0`
   check would have treated as a non-zero exit (exit code 2).
   Fix: extracted `runGhOrExit()` helper that distinguishes
   `result.error` (exit 1, dependency error) from `result.status
   !== 0` (exit 2, gh-side error). Both branches now have distinct
   stderr context tags.

3. **JSON.parse can throw on non-JSON output** (Copilot P1, line
   202) — `gh` could emit non-JSON on auth errors, truncation,
   etc. Fix: extracted `parseJsonOrExit<T>()` helper that catches
   parse errors, emits the first 200 bytes of input, and exits
   with code 3 (distinct from 1=invocation, 2=gh-side).

4. **--fixture/--owner/--repo missing-value validation** (Copilot
   P1, line 264) — passing `--owner` with no following value
   silently consumed nothing or grabbed an unrelated flag. Added
   `requireValue()` helper that exits 1 with a clear message if
   the next arg is missing or starts with `--`.

5. **Doc typo blocked-with → blocked-by** (Copilot P1, line 19) —
   usage example referenced `blocked-with-threads.json` but the
   file is `blocked-by-threads.json`. Mechanical fix.

Two stale Copilot threads (line 156 fix-failed-checks distinction
+ line 236 StatusContext EXPECTED/ERROR) were already addressed
in the prior commit (27e63d6). Resolving as stale-fixed. The line
295 "export main()" convention nit is deferred — the `tools/*.ts`
harness varies; this can be cleaned up in a follow-up sweep.

Exit code semantics now:
  0 — success
  1 — invocation / argument / dependency-missing
  2 — gh CLI returned non-zero (auth, rate-limit, PR not found)
  3 — gh output couldn't be parsed (truncated, non-JSON)

Verified: all four fixtures + live PR #921 classify correctly.
Missing-value test: `bun ... --owner` exits 1 with "--owner
requires a value" message.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ding (Codex P1)

Codex caught that classifyChecks's PENDING_STATUSES only included
QUEUED/PENDING/EXPECTED. CheckRun also has non-terminal REQUESTED
and WAITING states which would have been counted as neither
in-progress nor pending — silently dropped from the report,
producing false-positive `nextAction=none` when the PR is still
blocked by CI progression.

Added REQUESTED and WAITING to PENDING_STATUSES.

Per the GitHub schema, CheckRun.status is a non-exhaustive set
including IN_PROGRESS / QUEUED / COMPLETED / WAITING / REQUESTED /
PENDING. The script's classification logic now covers all
non-terminal states uniformly.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 30, 2026 15:28
Repo convention is `export function main(): number` + `if
(import.meta.main) { process.exit(main(...)) }` (used in
tools/peer-call/gemini.ts, tools/alignment/audit_*.ts,
tools/backlog/generate-index.ts, etc.).

My v0 used `function main(): void` + `main()` unconditionally,
which prevents the script from being imported as a module
(unconditionally executes side effects on import). The repo's
test harness pattern relies on the import-without-side-effects
shape.

Refactored to match: main() now returns exit code (0/1/2/3),
the import.meta.main guard ensures side effects only run when
invoked directly. Last remaining Copilot P2 thread on this PR
addressed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

Comment thread tools/github/poll-pr-gate.ts
Comment thread tools/github/poll-pr-gate.ts
Comment thread tools/github/fixtures/clean-armed-auto-merge.json Outdated
Comment thread tools/github/poll-pr-gate.ts Outdated
Comment thread tools/github/poll-pr-gate.ts
Comment thread tools/github/poll-pr-gate.ts Outdated
Comment thread tools/github/poll-pr-gate.ts Outdated
Comment thread tools/github/poll-pr-gate.ts
Comment thread tools/github/poll-pr-gate.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc3f4551ef

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/github/poll-pr-gate.ts
AceHack and others added 2 commits April 30, 2026 11:37
…ndling + positive PR-number + maxBuffer + CLOSED-state terminal + exit-code doc (Copilot P1×4 + Codex P2)

Six real defects from Copilot P1×4 + Codex P2 in the latest review wave:

1. **fixture mergeStateStatus mismatched name** (Copilot) — fixture
   `clean-armed-auto-merge.json` had mergeStateStatus=BLOCKED with
   the name promising "clean." With classifyGate now treating CLEAN
   correctly, set the fixture's mergeStateStatus to CLEAN. Now
   classifies as gate=CLEAN, next=none — matches the name's
   intent.

2. **loadFixture no error handling** (Copilot) — JSON.parse +
   readFileSync would throw an unhandled exception for missing /
   invalid fixtures (stack trace, no controlled exit). Wrapped in
   try/catch with controlled exit 1 + clear stderr message.
   Verified: passing a nonexistent fixture path produces
   "failed to load fixture <path>: ENOENT...".

3. **PR number 0 accepted** (Copilot) — `/^\d+$/` matched "0"
   as a valid PR number, but GitHub PR numbers are >0. Added
   parsed-value check that rejects <= 0 with exit 1 and clear
   message. Verified: `bun ... 0` produces "PR number must be a
   positive integer".

4. **spawnSync maxBuffer not set** (Copilot) — default 1 MiB buffer
   could truncate `gh api graphql --paginate` output on
   discussion-heavy PRs, cascading into JSON parse failures.
   Added SPAWN_MAX_BUFFER = 32 MiB constant; passed to spawnSync.

5. **CLOSED state not treated as terminal** (Codex P2) — nextAction
   only treated MERGED as terminal, so a PR in state=CLOSED could
   still be reported as fix-failed-checks/resolve-threads/wait-ci
   based on stale check/thread data. Added CLOSED → next=none
   short-circuit to avoid chasing blockers on intentionally-closed
   PRs.

6. **Exit codes doc inconsistency** (Copilot) — header listed
   0/1/2 only; code introduces 3 for parseJsonOrExit. Aligned the
   header documentation to mention all four exit codes (0=success,
   1=invocation/dependency, 2=gh-side, 3=parse failure).

Two stale Copilot threads from the earlier rounds (yQiO export-main
pattern + the "fix-failed-checks not in PR description" thread)
addressed by my prior commit (cc3f455) — convention-conformance
done. Resolving as stale-fixed.

Three style/convention threads (yQfm eslint suppression, yQh0
persona names in comments) deferred — Otto-279 history-class
attribution carve-out covers persona-name comments in tooling
files; eslint-suppression convention is a project-wide pattern
audit candidate, not this-PR-specific.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…lot)

Two final Copilot threads on PR #921 addressed:

1. **eslint-disable for spawnSync gh** (Copilot) — convention
   across tools/ (audit-packages.ts, pr-preservation/archive-pr.ts,
   peer-call/*, lint/runner-version-freshness.ts) is to suppress
   sonarjs/no-os-command-from-path with an inline rationale comment.
   Added the standard suppression to runGhOrExit's spawnSync call.

2. **Otto-279 role-refs in current-state code** (Copilot) — the
   header comment listed persona first-names ("Amara", "Deepseek",
   "Alexia", "Ani", "Gemini"). Per Otto-279's name-attribution
   carve-out, persona names belong on closed-list history surfaces
   (memory/, docs/ROUND-HISTORY.md, docs/DECISIONS/, docs/research/,
   commit messages) — not on current-state code. Replaced with
   role-ref "5-AI peer-reviewer convergence" + pointer to the
   verbatim attribution in the research doc. Same load-bearing
   provenance (the convergence claim), correct scope discipline.

The third remaining thread (PR description's nextAction list missing
fix-failed-checks) is a doc-only edit to the PR body, addressed
separately via PR description update; resolving with that note.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 30, 2026 15:40
@AceHack AceHack merged commit 0130b7e into main Apr 30, 2026
26 checks passed
@AceHack AceHack deleted the ops/poll-pr-gate-executable-script-v0-2026-04-30 branch April 30, 2026 15:43
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ecb4585e6d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

// the DIRTY gate state per Copilot P0 — semantically the same fix.
if (mergeStateStatus === "DIRTY" || mergeStateStatus === "BEHIND") return "DIRTY";
if (mergeStateStatus === "UNSTABLE") return "UNSTABLE";
if (checks.failed > 0) return "BLOCKED";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Prioritize CLEAN merge state over raw failed-check count

classifyGate marks any PR with checks.failed > 0 as BLOCKED before honoring mergeStateStatus === "CLEAN". GitHub’s MergeStateStatus.CLEAN explicitly means the PR is mergeable even when some commit statuses are non-passing, so this path misclassifies PRs that only have non-required failing checks and drives nextAction into fix-failed-checks unnecessarily. In those cases the loop can keep working a non-blocking failure while the PR is already mergeable.

Useful? React with 👍 / 👎.

if (report.checks.inProgress > 0 || report.checks.pending > 0) {
return "wait-ci";
}
return "none";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Report actionable state when gate is BLOCKED

nextAction falls back to "none" whenever a PR is not merged/closed/dirty and has no failed checks, unresolved threads, or pending checks. That creates a dead-end for valid mergeStateStatus: "BLOCKED" cases caused by other protections (e.g., required Copilot review/ruleset gates), where the PR is still unmergeable but this tool reports no next step. In this repo’s documented protection model, that state exists, so returning none here can stall the autonomous loop on still-blocked PRs.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

if (mergeStateStatus === "DIRTY" || mergeStateStatus === "BEHIND") return "DIRTY";
if (mergeStateStatus === "UNSTABLE") return "UNSTABLE";
if (checks.failed > 0) return "BLOCKED";
if (mergeStateStatus === "BLOCKED") return "BLOCKED";
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: classifyGate can return CLEAN based solely on mergeStateStatus === "CLEAN" even when checks are still pending/in-progress (since checks.inProgress/checks.pending aren’t considered here). That can yield a contradictory report like gate=CLEAN but nextAction=wait-ci. Consider factoring pending/in-progress into the gate classification (or returning UNSTABLE/UNKNOWN until all required checks are terminal+OK).

Suggested change
if (mergeStateStatus === "BLOCKED") return "BLOCKED";
if (mergeStateStatus === "BLOCKED") return "BLOCKED";
// A PR is not clean while required checks are still non-terminal; otherwise
// we can report gate=CLEAN while nextAction=wait-ci, which is contradictory.
if (checks.inProgress > 0 || checks.pending > 0) return "UNKNOWN";

Copilot uses AI. Check for mistakes.
Comment on lines +210 to +224
const result = spawnSync("gh", args, {
encoding: "utf8",
maxBuffer: SPAWN_MAX_BUFFER,
});
if (result.error) {
// ENOENT etc — gh is not on PATH or couldn't be launched
process.stderr.write(`${context}: failed to launch gh: ${result.error.message}\n`);
process.exit(1);
}
if (result.status !== 0) {
process.stderr.write(
`${context}: gh exited ${result.status}: ${result.stderr || result.stdout}\n`,
);
process.exit(2);
}
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: runGhOrExit doesn’t distinguish spawnSync termination-without-exit (result.status === null, possibly with result.signal) from a normal non-zero exit. Right now that path reports gh exited null and exits 2, which hides the real failure mode. Consider mirroring the repo’s common classifySpawnFailure pattern (ENOENT vs signal vs non-zero) and emitting an accurate message + exit code.

Copilot uses AI. Check for mistakes.
Comment on lines +359 to +402
function parseArgs(argv: string[]): ParsedArgs {
const out: ParsedArgs = {
owner: "Lucent-Financial-Group",
repo: "Zeta",
};
const requireValue = (flag: string, v: string | undefined): string => {
if (v === undefined || v.startsWith("--")) {
process.stderr.write(`${flag} requires a value\n`);
process.exit(1);
}
return v;
};
for (let i = 0; i < argv.length; i++) {
const arg = argv[i];
if (arg === undefined) continue;
if (arg === "--fixture") {
out.fixture = requireValue("--fixture", argv[++i]);
} else if (arg === "--owner") {
out.owner = requireValue("--owner", argv[++i]);
} else if (arg === "--repo") {
out.repo = requireValue("--repo", argv[++i]);
} else if (/^\d+$/.test(arg)) {
const parsed = Number.parseInt(arg, 10);
if (parsed <= 0) {
process.stderr.write("PR number must be a positive integer\n");
process.exit(1);
}
out.number = parsed;
} else if (arg === "--help" || arg === "-h") {
process.stdout.write(
"Usage: poll-pr-gate.ts <PR_NUMBER> [--owner X] [--repo Y]\n" +
" poll-pr-gate.ts --fixture path/to/fixture.json\n",
);
process.exit(0);
} else {
process.stderr.write(`unknown arg: ${arg}\n`);
process.exit(1);
}
}
return out;
}

export function main(argv: string[]): number {
const args = parseArgs(argv);
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: parseArgs (and requireValue) call process.exit(...) directly. Since main is exported, this makes the module hard to test/compose (callers can’t recover from parse errors). Consider having parseArgs return a typed {kind:"ok"|"help"|"error"} result and letting main decide the final exit code/output, consistent with other tools/ scripts.

Suggested change
function parseArgs(argv: string[]): ParsedArgs {
const out: ParsedArgs = {
owner: "Lucent-Financial-Group",
repo: "Zeta",
};
const requireValue = (flag: string, v: string | undefined): string => {
if (v === undefined || v.startsWith("--")) {
process.stderr.write(`${flag} requires a value\n`);
process.exit(1);
}
return v;
};
for (let i = 0; i < argv.length; i++) {
const arg = argv[i];
if (arg === undefined) continue;
if (arg === "--fixture") {
out.fixture = requireValue("--fixture", argv[++i]);
} else if (arg === "--owner") {
out.owner = requireValue("--owner", argv[++i]);
} else if (arg === "--repo") {
out.repo = requireValue("--repo", argv[++i]);
} else if (/^\d+$/.test(arg)) {
const parsed = Number.parseInt(arg, 10);
if (parsed <= 0) {
process.stderr.write("PR number must be a positive integer\n");
process.exit(1);
}
out.number = parsed;
} else if (arg === "--help" || arg === "-h") {
process.stdout.write(
"Usage: poll-pr-gate.ts <PR_NUMBER> [--owner X] [--repo Y]\n" +
" poll-pr-gate.ts --fixture path/to/fixture.json\n",
);
process.exit(0);
} else {
process.stderr.write(`unknown arg: ${arg}\n`);
process.exit(1);
}
}
return out;
}
export function main(argv: string[]): number {
const args = parseArgs(argv);
type ParseArgsResult =
| { kind: "ok"; args: ParsedArgs }
| { kind: "help"; message: string }
| { kind: "error"; message: string };
type RequireValueResult =
| { kind: "ok"; value: string }
| { kind: "error"; message: string };
function parseArgs(argv: string[]): ParseArgsResult {
const out: ParsedArgs = {
owner: "Lucent-Financial-Group",
repo: "Zeta",
};
const requireValue = (
flag: string,
v: string | undefined,
): RequireValueResult => {
if (v === undefined || v.startsWith("--")) {
return { kind: "error", message: `${flag} requires a value` };
}
return { kind: "ok", value: v };
};
for (let i = 0; i < argv.length; i++) {
const arg = argv[i];
if (arg === undefined) continue;
if (arg === "--fixture") {
const fixture = requireValue("--fixture", argv[++i]);
if (fixture.kind === "error") return fixture;
out.fixture = fixture.value;
} else if (arg === "--owner") {
const owner = requireValue("--owner", argv[++i]);
if (owner.kind === "error") return owner;
out.owner = owner.value;
} else if (arg === "--repo") {
const repo = requireValue("--repo", argv[++i]);
if (repo.kind === "error") return repo;
out.repo = repo.value;
} else if (/^\d+$/.test(arg)) {
const parsed = Number.parseInt(arg, 10);
if (parsed <= 0) {
return { kind: "error", message: "PR number must be a positive integer" };
}
out.number = parsed;
} else if (arg === "--help" || arg === "-h") {
return {
kind: "help",
message:
"Usage: poll-pr-gate.ts <PR_NUMBER> [--owner X] [--repo Y]\n" +
" poll-pr-gate.ts --fixture path/to/fixture.json\n",
};
} else {
return { kind: "error", message: `unknown arg: ${arg}` };
}
}
return { kind: "ok", args: out };
}
export function main(argv: string[]): number {
const parsedArgs = parseArgs(argv);
if (parsedArgs.kind === "help") {
process.stdout.write(parsedArgs.message);
return 0;
}
if (parsedArgs.kind === "error") {
process.stderr.write(`${parsedArgs.message}\n`);
return 1;
}
const args = parsedArgs.args;

Copilot uses AI. Check for mistakes.
Comment on lines +107 to +109
// StatusContext-class pending state (per Codex P1):
"EXPECTED",
// CheckRun non-terminal states (per Codex P1, second pass):
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: PENDING_STATUSES includes EXPECTED, but StatusContext EXPECTED is normalized in normalizeRollup into status: "PENDING" (and only state carries EXPECTED). Keeping EXPECTED here is dead/ambiguous and suggests a state-vs-status mixup; consider removing it or renaming the sets to make the distinction explicit.

Suggested change
// StatusContext-class pending state (per Codex P1):
"EXPECTED",
// CheckRun non-terminal states (per Codex P1, second pass):
// Normalized pending statuses plus CheckRun non-terminal states
// surfaced in `status` (per Codex P1, second pass):

Copilot uses AI. Check for mistakes.
AceHack added a commit that referenced this pull request Apr 30, 2026
CI's `memory-index-integrity` workflow caught that my prior commit
edited a `memory/*.md` file (poll-the-gate) without a paired
MEMORY.md update. Per the workflow's NSA-001-incident logic, any
add-or-modify to a top-level `memory/*.md` file requires a paired
MEMORY.md edit in the same PR.

Updated the poll-the-gate MEMORY.md row to note the
operationalization at `tools/github/poll-pr-gate.ts` (PR #921),
which is the substantive content my prior commit added to the
memory file body. Paired-edit now satisfies the integrity check.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AceHack added a commit that referenced this pull request Apr 30, 2026
…) (#922)

* memory(poll-the-gate): point at executable implementation (post PR #921)

PR #921 landed `tools/github/poll-pr-gate.ts` as the executable
implementation of the poll-the-gate rule. Per Amara's blade —
*"the memory file should stop being the implementation; it should
point to the implementation"* — adding a top-of-file pointer block
so future readers reach the script first and the rule-documentation
second.

The prose body still documents *why* the rule exists and *what*
the lane-state report should contain. The script documents *how*
the rule is mechanically applied. Both are needed; the pointer
makes the structural relationship explicit.

Composes with Aaron's substrate-IS-product framing — the script
IS substrate-quality work, not a refutation of the substrate-
product. The factory's tooling-product (executable scripts) and
substrate-product (memory rules) are siblings under the same
discipline.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(memory-index): add MEMORY.md paired-edit for poll-the-gate update

CI's `memory-index-integrity` workflow caught that my prior commit
edited a `memory/*.md` file (poll-the-gate) without a paired
MEMORY.md update. Per the workflow's NSA-001-incident logic, any
add-or-modify to a top-level `memory/*.md` file requires a paired
MEMORY.md edit in the same PR.

Updated the poll-the-gate MEMORY.md row to note the
operationalization at `tools/github/poll-pr-gate.ts` (PR #921),
which is the substantive content my prior commit added to the
memory file body. Paired-edit now satisfies the integrity check.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(memory-index): remove duplicate link target (CI duplicate-link lint)

CI's `lint memory/MEMORY.md for duplicate link targets` flagged
that `project_laptop_only_source_integration_*.md` was linked
from two MEMORY.md rows (line 7's no-copy "Composes with" link
+ line 110's canonical 2026-04-27 entry).

The line-110 entry is the canonical row for that file. Line 7's
link was a "Composes with" pointer added in PR #916's index entry.
Removed the link wrapping at line 7, kept the prose pointer ("the
laptop-only-source integration project task — canonical link
below"). Single canonical link target preserved.

This duplicate predated PR #922 (came in via PR #916's MEMORY.md
edit) but the lint check fired now because PR #922 touches MEMORY.md
and triggers a re-scan.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(memory-index): make poll-pr-gate.ts a clickable link (Copilot P2)

Copilot caught that my MEMORY.md addendum referenced
`tools/github/poll-pr-gate.ts` as plain code-text rather than a
markdown link. Made it a clickable link
([\`tools/github/poll-pr-gate.ts\`](../tools/github/poll-pr-gate.ts))
matching the convention used in the memory-file body's pointer
block. Future readers can jump directly from the index entry
to the implementation.

Note: the link target is a `.ts` file, not a `.md` — the
duplicate-link lint counts only `.md` link targets, so this
addition doesn't re-introduce the duplicate-link issue from
the earlier round.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
AceHack added a commit that referenced this pull request Apr 30, 2026
* research: preserve Ani + Alexia v1 feedback packets verbatim

Both peer-AI reviewers responded after PR #921 (poll-pr-gate v0)
+ PR #922 (memory-points-at-script) merged. Per Otto-363
substrate-or-it-didn't-happen, preserving both packets verbatim
at `docs/research/2026-04-30-amara-poll-pr-gate-v1-hardening.md`.

Both packets predominantly "what's working" with smaller actionable
findings. Substantive items overlap with PR #923 (v1 hardening,
already on main) or queued under existing tasks:

- "submit-nuget non-required classification" (both reviewers) —
  shipped in PR #923.
- "Dot-tick discipline still leaky" (Ani #1) — accepted as
  behavior change going forward (no code, no substrate; commitment).
- "Pre-merge mechanical guards" (Ani #3) — persona-name scanner +
  fixture-name validator composed with task #350 (Otto-357
  mechanized auditor) and task #355 (poll-the-gate matrix
  coverage). Defer per substrate-rate.
- "MEMORY.md duplicate-link timing" (Ani #4) — audit candidate;
  the check fired on PR #922 but missed PR #916. Defer.
- "Task list at 58 open" (Ani #5) — pending-task audit overdue;
  composes with task #321 recovery lane.
- "Thread categorization" + "intelligent compaction" (Alexia) —
  research-grade, not yet operational.

No Insight-block commentary added per the discipline accepted in
the prior Claude.ai packet absorption: produce the work, let the
diff carry the evidence.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(research): mark check-github-status path as in-flight at PR #924 (Copilot)

Same in-flight-xref pattern caught earlier this session — code-span
path implied existence-on-main, but the file is on PR #924's branch.
Reworded to make the in-flight status explicit.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* research: preserve Deepseek session-arc retrospective packet

Final feedback packet from Deepseek post-PR #924 merge. Most
findings already shipped:

- submit-nuget transient → PR #923 (v1 hardening, required-vs-
  non-required classification)
- MEMORY.md merge-conflict tax → PR #920 (merge=union driver)
- Stale project-file internals cleanup → B-0112 P2 row filed

New finding: 30+ dot threshold for deferred-task re-audit (not
new lanes, just already-scoped tiny fixes). Composes with Ani's
strict-enforcement framing.

Per Otto-363 substrate-or-it-didn't-happen.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants