Skip to content

fix(poteto-mode): observe current base branch targets - #245

Merged
michael-denyer merged 4 commits into
michael-denyer:mainfrom
mshk:codex/fix-base-ref-target-oid
Oct 9, 2026
Merged

michael-denyer merged 4 commits into
michael-denyer:mainfrom
mshk:codex/fix-base-ref-target-oid

Conversation

@mshk

@mshk mshk commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Why

The watcher and shipping helper can miss a destination branch advance because PullRequest.baseRefOid can stay fixed. Read baseRef.target.oid with the other PR facts so snapshot validation and cancellation compare the current destination.

Closes #244.

What changed

  • Query the current base ref target in both GitHub readers and normalize it into the existing landing revision field.
  • Reject an unavailable current base for an open PR.
  • Add regression coverage for stale scalar OIDs, snapshot changes, cancellation readback, and missing base refs.

Scope

This change covers the watcher and shipping readers. It keeps the landing record format and CLI behavior. A version bump and the disposable live merge test remain release work.

Tradeoffs

The watcher uses one GraphQL query for head and base facts. A separate query after gh pr view would permit those facts to come from different observations.

Blast Radius

PR monitoring and pending merge cancellation now observe destination branch movement. Missing base data produces an error instead of a valid landing record.

Verification

  • bun run test, bun run typecheck, and bunx prettier@3.6.2 --check . in plugins/pstack/skills/poteto-mode/scripts passed with 232 tests.
  • bun tools/generate.mjs --check passed. bun test tests/ --timeout 30000 passed with 1,185 tests and 40 skips. The default five-second timeout expired in an existing worktree fixture; the longer run passed.
  • Independent code review returned PASS+NOTES. The disposable live merge test was not run.

Follow-up commit

e5cf03d reads every GraphQL envelope through one graphql() method on the reader, so all four queries fail closed on errors. The open-PR landing revision narrows the parsed facts instead of re-parsing a copy of the raw response, ship-pr inspect builds its revision from the GraphQL fields directly, and the gh pr view empty-string review decision and its fixtures are removed.

mshk and others added 4 commits October 9, 2026 20:41
…vers

GhGitHubReader reads every GraphQL envelope through one graphql() method,
which checks errors and returns data.repository.pullRequest for all four
queries. Before, only the PR facts query checked errors.

An open PR's landing revision narrows the parsed facts with text() instead
of re-parsing a copy of the raw response under the old baseRefOid key.
ship-pr inspect builds its revision from the GraphQL fields directly.
parseLandingRevision parses the saved record only, so its optional context
parameter goes.

The empty-string review decision came from gh pr view, which the facts
reader no longer runs. GraphQL returns null, and an empty string is
rejected like any other unknown value. The transport fake's pr view branch
and the baseRefOid decoys in shared fixtures go with it.
@michael-denyer

Copy link
Copy Markdown
Owner

Thank you for this, @mshk. The fix is right and the regression tests made it easy to build on. Reading baseRef { target { oid } } and taking head and base from one GraphQL query were the changes #244 needed.

I pushed one follow-up commit, e5cf03d, on top of your branch:

  • GhGitHubReader reads every GraphQL envelope through one private graphql() method. It checks errors and returns data.repository.pullRequest for all four queries, where before only the PR facts query checked errors.
  • An open PR's landing revision narrows the parsed facts with text() instead of re-parsing a copy of the raw response under the old baseRefOid key. ship-pr inspect builds its revision from the GraphQL fields directly, and parseLandingRevision now parses the saved record only.
  • The empty-string review decision came from gh pr view, which the reader no longer runs. GraphQL returns null, so the special case and its comment are gone and "" is rejected like any other unknown enum value.
  • The transport fake's pr view branch and the baseRefOid decoys in the shared fixtures are removed. The one targeted test keeps its decoy.

Verified in the scripts package with bun test (232 pass, 0 fail), tsc (exit 0), and Prettier 3.6.2. The repo-level bun test tests/ passes with 1189 tests and 36 skips. I will merge once CI is green and cut 0.9.80 right after.

@michael-denyer michael-denyer mentioned this pull request Oct 9, 2026
@michael-denyer
michael-denyer merged commit 472ed21 into michael-denyer:main Oct 9, 2026
14 checks passed
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.

watch-pr and ship-pr miss current base branch changes

2 participants