Skip to content

docs: close gaps found in parallel PR review - #434

Merged
oxwen11 merged 3 commits into
mainfrom
docs/parallel-review-rules
Oct 5, 2026
Merged

oxwen11 merged 3 commits into
mainfrom
docs/parallel-review-rules

Conversation

@oxwen11

@oxwen11 oxwen11 commented Oct 5, 2026

Copy link
Copy Markdown
Owner

Requirement

Parallel PR review and verification hit failures the current rules still allow or contradict. Record those outcomes so the next pass does not repeat them.

Expected behavior

  • Required CI is for the exact head. Same-named checks all pass. One diagnosed external retry is allowed; looping until green is not. REST SHAs win when metadata disagrees.
  • The author of a version cannot independently review it. A blocked or unexecuted check is a gap.
  • The primary checkout stays clean on main. Each candidate has its own worktree. A merge invalidates other candidates' bases.
  • Parallel drives use the owned root's browser script. Shim refusal and explicit cleanup never adopt or kill another run.
  • Real-model verification keeps the operator's Pi configuration. Do not empty PI_CODING_AGENT_DIR or guess-restore shared settings.
  • Public evidence is cropped to the relevant UI.

Changes and risks

Rules only. No product, persistence, or verify-helper change. The seeded E2E PI_CODING_AGENT_DIR override stays limited to that fake provider.

Verification

Diff review against the failures below. No runtime surface is affected.

  • Fixed ports and the API dev script ignoring PIE_PORT sent two launches at 4180. Occupied ports must fail, not attach.
  • Cleanup followed current and could stop the other run. Explicit cleanup now has to target that run and refuse foreign or corrupt targets before stopping anything.
  • Bare agent-browser and ambient env could bind the wrong surface. A shim refusal stopped the drive; it is not permission to kill the other run.
  • Empty PI_CODING_AGENT_DIR was treated as isolation and hid real model config. The toolchain rule now limits that override to seeded E2E.
  • Creating sessions with an explicit model wrote Pi's global default. Record the write; do not guess a previous value and restore it.
  • GraphQL/gh metadata went stale or disagreed with REST. Required preview checks share a name; one SUCCESS is not enough.
  • A push cancelled the previous CI run. Review only the current head.
  • Sandbox socket failures were easy to report as passed browser tests.
  • Main advanced under other open reviews, invalidating their bases. The primary checkout must stay on main.
  • Screenshots included local paths until cropped. Before/after frames are that drive, not a reconstructed baseline.

Record the constraints that failed in practice: pinned worktrees, exact CI identity, owned cleanup, and real Pi configuration.
@pkg-pr-new

pkg-pr-new Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
npx https://pkg.pr.new/oxwen11/pie/@getpie/cli@434

commit: b026a2a

iamdin added 2 commits October 5, 2026 05:08
Launch now rejects the isolation mistake that hid the operator's Pi configuration. Rules only point at that check.
The CI poll waited for transitions that were still running on a slow runner. Finish them after the dialog opens so geometry checks see the settled state.
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit b026a2a.

@oxwen11
oxwen11 merged commit dd85b29 into main Oct 5, 2026
6 checks passed
@oxwen11
oxwen11 deleted the docs/parallel-review-rules branch October 5, 2026 09:24
@oxwen11

oxwen11 commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

Merge record

Squash-merged at the Developer's explicit request.

  • Head: b026a2a66c243173b463d275a8108ad98f48ff6a
  • Base: 18ba5813572c2f3499c3c5771c11410ad8032e3a
  • Merge: dd85b29a1968040987ed4b75ff88862b3102d0bd
  • Merged at: 2026-10-05T09:24:15Z
  • Required checks on that head were all SUCCESS immediately before merge: Check, react-doctor, and both Publish @getpie/cli preview checks. Compare was ahead 3, behind 0, MERGEABLE.
  • No independent review was recorded. The author did not self-approve.

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