Skip to content

GitHub: pull request reads and writes - #42

Merged
tomkaczocha merged 7 commits into
mainfrom
t3/placeholder
Oct 15, 2026
Merged

tomkaczocha merged 7 commits into
mainfrom
t3/placeholder

Conversation

@rawphp

@rawphp rawphp commented Oct 15, 2026 •

Copy link
Copy Markdown

Closes #14

Contract

  • done: tracker/github.ts gains pullRequest, markPrReady, markPrDraft, commentOnPr, compare and checks, each with the argv softfac's github.rs uses; branchTip already landed with GitHub: issue reads and writes through the gh CLI #12. softfac's test cases for these calls are ported to github.test.ts and pass.
  • out: wiring these into a gate or daemon; PR feedback, approval and ready-by queries; Linear.
  • risk: none
  • test: vp test run apps/server/src/factory/tracker/github.test.ts → pass (119)

Acceptance criteria

  • View a pull request (repo, head repo and ref, base, draft, state, merged, head sha) — pullRequest reads the real shape with the exact call, pullRequest from a fork and into another base says so, a pullRequest that does not exist is null
  • Mark it ready and back to draft — markPrReady runs gh pr ready, markPrDraft runs gh pr ready --undo, the already-ready / already-draft / closed-or-merged cases
  • Comment — commentOnPr appends the marker
  • Read a branch tip sha — already on main from GitHub: issue reads and writes through the gh CLI #12 (branchTip tests)
  • Compare two shas — compare asks for the status of the two refs with the exact call, compare reads each status GitHub gives
  • List check runs for a sha — checks reads the check runs of the commit with the exact call and the ranking, paging, skipped and link cases
  • softfac's src/github.rs cases for these calls are ported and pass — 38 cases ported one to one; one argv note below

One deliberate divergence from softfac: a ref refused before any gh call (a .. in compare, a sha that is not a full id in checks) is GitHubInvalidArgument, as the already-landed branchTip and fileAt do, where softfac reports a parse error. The argv and every answer are otherwise the same.

Gate

  • vp test run apps/server/src/factory → exit 0 (19 files, 504 tests)
  • vp run --filter t3 typecheck → exit 0

Run on 44cd981e3. Logs in the run's .factory/gate-1.log and gate-2.log.

Models

  • maker: claude-fable-5-1 (this session; no models block in .dw/config.yml, Tom's roster applied)
  • checker: claude-opus-5-5

Review

  • cycles: 1
  • fixed: correctness-1 — markPrDraft only read the pull request after a non-zero exit; softfac does it on every error, so a timed out, rate limited or oversized undo on a closed PR is now done too (32b4a1851, 249fae93e).
  • reliability: approve, no findings.
  • testing (mutation pass on 249fae93e): 12 of 12 named mutants killed, none unreached; nit testing-1 taken as 44cd981e3.
  • leftovers: none

Lessons

  • On a fork, pass --repo to gh pr create. Symptom: the draft opened on the upstream repository (pingdotgg/t3code#19893, closed at once with an apology) instead of the fork the issue lives in. Cause: in a fork checkout gh resolves the parent as the default repo for pr create. Default: always gh pr create --repo <owner/repo> --head <branch> with the issue's repo, and read the printed URL's owner before reporting it.
  • Run the repo's lint on the touched files even when the gate does not. Symptom: tests and typecheck were green, and a lint rule the repo enforces (prefer-catch-tags) only showed up because the maker ran vp lint on the two files by hand. Cause: the gate lists tests and typecheck only. Default: after the first green, lint the contract files with the repo's own lint command before Check, so reviewers and CI see the tip that passes it.
  • An issue's /tmp instruction for a reference clone yields to § Scratch. Symptom: the issue said to clone the reference repo into /tmp/softfac-ref. Default: clone it under <worktree>/.factory/ instead and give reviewers that path; the worktree's removal then takes the clone with it.

Commits

  • 9b690bb63 test(factory): port softfac's pull request, compare and check-run cases — Make slice (red: 38 failing, file did not typecheck)
  • 9a1137638 feat(factory): pull request reads and writes through gh — Make slice (green)
  • 32b4a1851 test(factory): markPrDraft reads the PR state after any failure — Check cycle 1 (red: 1 failing)
  • 249fae93e fix(factory): markPrDraft reads the PR state after any failure — Check cycle 1 (green)
  • 44cd981e3 test(factory): a check's conclusion and link come from the deciding run — Check nit (already green)

Files

  • apps/server/src/factory/tracker/github.ts — the six calls, their schemas and types
  • apps/server/src/factory/tracker/github.test.ts — softfac's cases ported to @effect/vitest

No new seam: both files are under the factory's own directory.

Worktree note: the factory's worktree was cut at e4bcce9f6, behind origin/main after #12 and #39 merged; it was fast-forwarded to 952129a80 before any write. Labels: all seven factory:* labels already existed; none created.

Red: github.ts has no pullRequest, markPrReady, markPrDraft, commentOnPr,
compare or checks yet, so the 38 new cases fail and the file does not
typecheck.

Issue: #14
tracker/github.ts gains pullRequest, markPrReady, markPrDraft, commentOnPr,
compare and checks, each with the argv softfac's github.rs uses. branchTip
landed with #12. A ref refused before the call (a `..` in compare, a sha
that is not a full id in checks) is a GitHubInvalidArgument, as branchTip
and fileAt already do, where softfac reports it as a parse error.

Issue: #14
Red: markPrDraft only reads the pull request after a non-zero exit, so a
timed out, rate limited or oversized undo on a closed PR is still an error.
softfac's mark_pr_draft falls back on every error.

Issue: #14
A timed out, rate limited or oversized `gh pr ready --undo` on a pull
request that is closed or merged by now is done, as in softfac; one that is
still open keeps the original error.

Issue: #14
Already green: covers what the several-runs case only asserted by state.

Issue: #14
@rawphp
rawphp marked this pull request as ready for review October 15, 2026 13:03
@rawphp

rawphp commented Oct 15, 2026

Copy link
Copy Markdown
Author

Factory gate: passed (attempt 1, run by the factory in a fresh checkout of the pull request's head 44cd981 merged with the base branch tip 952129a)

Command Exit Time
factory/scripts/check.sh 0 22.0 s

Tests pin the check ranking against a skipped run, the tie between two
failed runs, and lookalike GitHub links; the CheckStatus doc says a skipped
run keeps its conclusion.
@rawphp

rawphp commented Oct 15, 2026

Copy link
Copy Markdown
Author

Factory gate: passed again (attempt 2, run by the factory in a fresh checkout of the pull request's head 7f0d01c merged with the base branch tip fdaee1f)

Command Exit Time
factory/scripts/check.sh 0 26.2 s

@tomkaczocha
tomkaczocha merged commit dd768b6 into main Oct 15, 2026
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.

GitHub: pull request reads and writes

2 participants