Apply reviewed labels without risking PR creation

[hamzamerzic]

Summary

• apply at most the two labels visible in Contribute review, and only when those labels still exist in the upstream repository
• persist requested, confirmed-applied, missing, and nonfatal label outcomes on the contribution record
• keep an accepted pull request open when GitHub label lookup or application times out, cannot launch, is refused, or is otherwise unconfirmed

Recovery contract

PR creation is the durable boundary. Every label operation happens afterward and is best-effort. Timeout and OSError paths now return a truthful warning patch instead of escaping into the submit endpoint and leaving an already-created PR presented as a failed or stranded submission.

Verification

• backend/tests/test_github_routes.py: 90 passed
• focused exact-head label regressions: 7 passed
• Python compile checks
• git diff --check

Rebuilt from production commit db1c2ddb onto current upstream main 7b1eb41. Core PR: review only; do not merge without the required independent reviews.

[hamzamerzic]

Verdict for exact head 989432f: CHANGES_REQUESTED

Blocker — recover a possibly accepted PR when pr create loses its response via TimeoutExpired or OSError.

At backend/app/routes/github.py:1839, _gh(... pr create ..., check=False) may raise either exception directly. The only inner handler at line 1866 catches ContributionSubmitError, so this bypasses _find_existing_pr at lines 1843-1850. The endpoint then takes the generic exception path at lines 2560-2572, returns 500, and moves the ledger back to prepared without the durable PR URL/number. If GitHub accepted the create before the timeout/transport loss, the response and ledger are therefore untruthful and the next retry risks rediscovery only indirectly.

Expected transition: after the branch push, treat TimeoutExpired/OSError from pr create as an ambiguous create result; run the same exact head/base _find_existing_pr lookup used for a nonzero return. If found, apply labels best-effort and persist/return status=open plus URL/number and the truthful label patch. If no PR can be confirmed, keep status=prepared, preserve the pushed-stage patch, and return the actionable retry error. Please add regressions for both TimeoutExpired and OSError proving the found-existing path lands open and records label outcomes.

The revised first-two-visible label boundary itself looks correct: malformed/blank inputs are filtered like the review UI, then two visible labels are selected before validation/dedupe, so an invalid or duplicate visible label cannot promote a hidden third.

[hamzamerzic]

Concrete reproduction on exact head 989432f, with _find_existing_pr stubbed to return https://github.com/mobius-os/app-demo/pull/77:

TimeoutExpired escaped_as=TimeoutExpired find_existing_calls=0
OSError escaped_as=OSError find_existing_calls=0

The harness drove _submit_prepared_pr through successful validation, push, and into the pr create call, then injected each transport exception. In both cases the existing-PR reconciliation hook was never called. This confirms the source-level transition described in the review comment.

[hamzamerzic]

Blocking create/recovery integrity finding on exact head 3949d7cb61263859078fdaecd7b2b61dd78a2641:

Standalone PR creation and recovery still do not pin the base branch. _assert_merges_with_upstream() resolves and verifies the upstream default branch and records it as merge_patch['last_submit_upstream_branch'], but create_args adds --base only for direct_base stacks. The subsequent _find_existing_pr() also receives base_branch=direct_base, so standalone recovery lists every open PR for the head regardless of base.

This is not merely an omitted filter: GitHub CLI documents that when gh pr create --base is absent it first honors local branch.<current>.gh-merge-base, and only otherwise uses the repository default: https://cli.github.com/manual/gh_pr_create. These staged repos are durable checkouts and can carry that config. A stale branch.fix/demo-polish.gh-merge-base=release therefore makes Contribute verify the reviewed diff against main but create the approved PR against release. On an ambiguous response, exact headRefOid alone can likewise recover an exact-head PR for the wrong base.

Please use the already-proven upstream branch as the explicit standalone base for both operations:

• append --base <merge_patch['last_submit_upstream_branch']> to standalone pr create
• pass the same branch to _find_existing_pr() so pr list includes --base

Keep direct_base for stack layers. Add regressions asserting the non-stack create and the timeout/OSError recovery probe both contain --base main; the current transition test asserts only head and SHA, so it misses this target mismatch.

[hamzamerzic]

Final exact-head review on 139fc05ec2561aa3ce673d5802c07fe1e2c2da51: the standalone create and lost-response recovery paths now both pin the already-verified upstream base, while stack layers retain their explicit direct base. I rechecked label visibility (first two reviewed slots before validation/dedup), partial/unknown label outcomes, exact-head recovery, and create timeout/OSError paths. All 95 GitHub route tests pass. No remaining finding; hosted backend/e2e are the final gate.

[hamzamerzic]

APPROVED on exact head 139fc05ec2561aa3ce673d5802c07fe1e2c2da51.

The previous wrong-base blocker is closed. Standalone create now pins --base to the branch resolved and merge-tested by _assert_merges_with_upstream, and the ambiguous-response probe filters by that same resolved base plus the exact pushed head SHA. Stack submissions keep their reviewed direct parent as the explicit base. The accepted/mismatch/absent transitions remain conservative: one create and one probe only; an exact head+base match becomes durable open with labels, while absent/wrong-head stays prepared with the pushed stage/SHA for a safe retry.

Verification: all 95 GitHub route tests pass locally; exact-head GitHub privacy/backend/frontend/packager/core-apps/E2E checks are green; diff is clean. The PR is currently behind main, so this approval is for the exact SHA above and should be revalidated if conflict resolution changes code.

[hamzamerzic]

Landed on protected main through reviewed integration #147 (merge 226106c). The integrated patch matches this PR's reviewed head; closing the superseded source PR to keep the queue truthful.