Skip to content

Make Launchplane authoritative for Owner acceptance - #2168

Merged
cbusillo merged 9 commits into
mainfrom
every-code/cbusillo-launchplane-2164-every-code-cbusillo-launchplane-2164-5201700b0259b1e1
Aug 17, 2026
Merged

Make Launchplane authoritative for Owner acceptance#2168
cbusillo merged 9 commits into
mainfrom
every-code/cbusillo-launchplane-2164-every-code-cbusillo-launchplane-2164-5201700b0259b1e1

Conversation

@cbusillo

@cbusillo cbusillo commented Aug 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • remove Owner shadow-mode fields, routes, actions, UI wording, fixtures, generated contracts, and current documentation
  • make exact-change Owner acceptance authoritative for merge readiness while keeping technical checks, admission, landing, and production authorization separate
  • add the canonical preview-ready Owner handoff with the exact Launchplane deep link, current state, test plan, decision instructions, and staleness warning
  • project clear Owner status into GitHub while keeping GitHub non-authoritative and refreshing the projection during preview feedback
  • archive legacy requirement rows, reset executable scopes to a repairable revision-1 baseline, and require an explicit revision-2 authority write
  • make Owner event projection fail closed so revocation or requested changes cannot strand a stale green GitHub check
  • exercise the real LiveMergeAdmissionEvaluator with persisted Owner policy, requirement, event, preview, and drift evidence

Review Remediation

  • browser events, manual reconciliation, and ready preview feedback use one shared current-ledger projection service under the immutable repository-id/PR lock, including service configurations without browser sessions
  • bindingless stale and unavailable decisions project action_required or failure against the exact resolved target; only not_required intentionally omits the preview Owner action
  • restoration demotes the exact attempted target before any repository re-resolution can fail, then conservatively covers a newly resolved target under the same lock when needed
  • every projection token is revoked through one shared lifecycle; if a usable token fails later expiry, permission, repository-count, repository-id, or repository-name validation, the mint function revokes it before surfacing the original validation error
  • preview feedback runs synchronous reconciliation in Starlette's worker thread so lock waits, storage reads, and synchronous GitHub requests cannot block the ASGI event loop
  • PostgreSQL commits immediately after acquiring the session advisory lock, performs provider work outside an open transaction, then explicitly unlocks, commits, and verifies cleanup on the dedicated unpooled connection
  • the freshly resolved event binding must match the held lock before the conservative projection or append, and final projection is confirmed from the current ledger before releasing the lock
  • idempotent replay is stable across mutable Owner login changes because authorization identity remains bound to the stable GitHub user ID
  • filesystem and SQLite rehearsal locks serialize in-process and cross-process; PostgreSQL waiters cannot exhaust the record-store pool
  • live-store admission coverage includes pending, accepted, changes-requested, revoked, stale, denied authority, unavailable authority, head drift, tree drift, preview-generation drift, Owner-policy drift, and requirement drift

Validation

  • exact refreshed head: 35dcd098d0319c31d4bf89c6173fc845d8c58b0b
  • current main merged into the task branch without rebasing; the merge completed cleanly and git diff --check passed
  • focused refreshed-head gate passed 328 tests across GitHub App identity, Owner HTTP/concurrency, service routes, migrations, and PostgreSQL storage
  • GitHub reports 26 successful checks, zero failed, and zero pending on the refreshed head, including the real PostgreSQL integration gate
  • refreshed-head Opus and Gemini reviews both returned APPROVE MERGE
  • original remediation gates remain recorded: all 12 configured unittest shards passed with 2,952 targets; repository-wide Ruff and mypy passed; frontend validation passed with OpenAPI drift, typecheck, and production build
  • JetBrains changed-file inspection previously returned broad warnings outside the remediation's changed lines; Ruff, mypy, targeted tests, CI, Security, CodeQL, browser smoke, and PostgreSQL integration are green
  • the previously completed Playwright evidence remains 94 desktop/narrow journeys and 30 verified screenshots; browser smoke passed again on the refreshed GitHub head

Search Evidence

  • runtime/API/UI searches find no Owner shadow-mode reader, writer, route, action, field, fixture, or control
  • remaining Owner-shadow text is limited to historical Alembic migration identifiers required to remove and archive the legacy schema

Closes #2164

@cbusillo

Copy link
Copy Markdown
Owner Author

[agent review] Reviewed exact head 93681fae3bd9017066c2cc3bdf53f42e6942186bCHANGES REQUIRED.

  1. High — the authority-cutover migration leaves every migrated scope permanently invalid. f0a2c4e6b8d1_remove_owner_shadow_mode.py deletes canonical requirement revisions 1..N and inserts only revision N+1. product_owner_service._current_scoped_record() requires a contiguous 1..N+1 chain, while supported writes can only append N+2; live evaluation therefore returns requirement_history_invalid forever. Preserve a valid canonical chain or intentionally reset each scope to a repairable revision-1 record while separately archiving legacy rows, and test evaluation plus a subsequent supported write after migration.

  2. High — a prior green GitHub Owner check can remain stale after revocation or request-changes. Accepted/not-required decisions project success, but event writes commit first and call project_current_decision_best_effort(), which swallows every projection failure. A later authoritative negative decision can therefore leave the old success check green and misleading. Add a fail-closed/reconciliation design that cannot strand stale success, with transition tests.

  3. Medium — the new state matrix does not prove the real live-admission path. It injects synthetic decisions into the pure merge-readiness evaluator. Add real-store LiveMergeAdmissionEvaluator coverage for pending, stale, revoked, changes-requested, denied/unavailable, accepted exact binding, and head/tree/runtime/policy drift.

  4. Medium — restricted-owner action reset is broken. EngineeringOwnerAcceptanceRoute.tsx resets to setAction("accepted") after a successful write. For a viewer with can_accept=false, the controlled select then has no rendered matching option and future submission remains disabled. Reset with ownerAcceptanceDefaultAction(eligibility) and cover the restricted-owner journey.

  5. Validation claim needs correction and rerun. The recorded full unittest run did not have shards 0-10 all pass: shard 7 failed on test_preview_pr_feedback_ready_requires_active_preview_url before 93681fae. The PR reran shard 11, not shard 7. Rerun the affected shard (and preferably the configured aggregate gate) on the final head before claiming it green.

Do not merge this head.

@cbusillo

Copy link
Copy Markdown
Owner Author

[agent remediation review] The current working-tree remediation still needs changes before push.

  1. High — concurrent writes can restore stale green. The final route projection uses result.decision, calculated from the request's pre-append prior_events snapshot. If accepted and changes-requested/revoked writes overlap, both pre-project action_required; the negative event can append/project first, then the slower accepted request can PATCH the check back to success even though the negative event is latest. Idempotent replay has the same ordering risk. Final projection must re-evaluate the current ledger after persistence and serialize/order per subject, rather than projecting an event-local snapshot.

  2. Medium — live-store admission matrix is still incomplete. Add the requested authority-denied case; assert top-level live readiness/admission is blocked for every negative state, not only the Owner facet; and prove an exact runtime-bound accepted decision is admissible before testing that runtime drift blocks it.

Migration reset/archive and the restricted-owner UI reset look correct in the current remediation. Do not push the final update until the race and matrix gaps are resolved and gates rerun.

@cbusillo

Copy link
Copy Markdown
Owner Author

[agent final exact-head review] Reviewed 706681212bb94d4346d9360cc3abf47ec6375492CHANGES REQUIRED.

  1. High — preview-ready feedback bypasses the projection lock/reconciliation service. control_plane/http_app.py directly evaluates and calls project_owner_acceptance_decision(). An accepted decision evaluated before a concurrent changes_requested/revoked event can therefore write success after the negative event completes. Route every Owner projection, including preview feedback, through the same repository-id/PR locked current-ledger reconciliation path.

  2. Medium — restoration can fail before demoting an already-delivered success. After a successful green write followed by confirmation failure, restoration immediately repeats evidence/current-state resolution. If that transient read still fails, the delivered green remains. First demote the exact projected target to the conservative non-green state, then attempt current-state reconciliation.

  3. Medium — PostgreSQL session lock can be lost during the remote critical section. pg_try_advisory_lock() auto-begins a transaction and leaves it idle while GitHub projection runs. idle_in_transaction_session_timeout can terminate the connection and release the lock while the request continues. Commit after acquiring the session-level advisory lock; explicitly unlock and verify cleanup in finally on the dedicated unpooled connection.

  4. Medium — preview feedback leaks the minted GitHub App token. The inline token passed to project_owner_acceptance_decision() is never revoked on success or failure. This should disappear by routing through the shared projection service, or must be wrapped in guaranteed revocation.

  5. Low — idempotent replay identity includes mutable owner_login. A GitHub username change rejects an otherwise identical replay. Prefer stable GitHub ID plus immutable request semantics.

Confirmed: internal merge readiness excludes projected checks; no executable Owner shadow mode remains; the exact head is clean and all 26 CI checks are green. However stale-green prevention is not complete, so this head is not approved and must not merge.

@cbusillo

Copy link
Copy Markdown
Owner Author

[agent exact-head re-review] Reviewed 67359d4f045ced779892eb28be70db9fce2f13e9CHANGES REQUIRED.

  1. High — bindingless stale/unavailable decisions skip demotion. OwnerAcceptanceProjectionService.reconcile_if_required() performs no GitHub write whenever decision.binding is None, not only for not_required. If the same head previously projected accepted/success and policy, evidence, or Owner authority later becomes unavailable/stale without a binding, preview reconciliation leaves stale green intact. Preserve the exact resolved target separately and project failure/action-required for bindingless negative decisions; only not_required should intentionally omit an Owner action.

  2. High — validation failure after token mint leaks the installation token. The default token provider mints first, then validates expiry, permissions, repository count/ID/name. If those post-mint checks fail, the provider raises before the projection service receives the token and its cleanup starts; no revocation DELETE occurs. Add guaranteed revocation inside mint_repository_installation_token() for every post-mint validation failure and test repository-ID/name/permission/expiry mismatch paths.

  3. Medium — preview feedback blocks the ASGI event loop. The async preview-feedback route calls synchronous reconciliation directly; it can wait on filesystem/SQLite/PostgreSQL locks and performs synchronous GitHub requests. Run the shared reconciliation service in a worker thread (and cover the production route), rather than relying on tests that call asyncio.to_thread externally.

Verified: PostgreSQL acquisition now commits before provider work and explicitly unlocks/verifies cleanup; restoration demotes before re-resolution; event/manual paths share the exact lock; replay no longer depends on mutable login; no executable Owner shadow mode remains.

Do not merge this head.

@cbusillo

Copy link
Copy Markdown
Owner Author

[agent final review] Reviewed exact head 448d7b9bc2349d4b472f5d8db58cdbfdd2db0bafAPPROVE.

Verified independently:

  • Bindingless stale and unavailable Owner decisions project non-green conclusions against the exact resolved target; only not_required intentionally skips projection.
  • Post-mint validation failures revoke every usable installation token while preserving the primary validation error.
  • Preview-feedback reconciliation runs through Starlette's worker thread rather than blocking the ASGI event loop.
  • Accepted-versus-negative ordering is serialized by the repository-id/PR projection lock, with current-ledger reconciliation preventing an older accepted request from restoring stale green.
  • Restoration demotes the exact attempted target before re-resolution and conservatively covers a changed current target under the same lock.
  • PostgreSQL advisory locking uses a dedicated unpooled connection, commits acquisition before provider I/O, explicitly unlocks, commits cleanup, and verifies unlock success.
  • No executable Owner shadow-mode route, action, field, or control remains; historical migration identifiers are retained only for archive/removal history.
  • The worktree is clean; all 12 local shards passed with 2,952 targets; repo-wide Ruff, mypy across 727 files, frontend validation, and all 26 GitHub checks are green. No unanswered external comments remain.

Non-blocking follow-up: if a projection body and post-use token revocation both fail, the cleanup exception can mask the primary projection exception in diagnostics. The fail-closed restoration path still runs, so this is not an authority bypass or stale-green blocker, but preserving the primary exception would improve operator diagnosis.

No merge performed.

@cbusillo

cbusillo commented Aug 17, 2026

Copy link
Copy Markdown
Owner Author

Closeout refresh for exact head 448d7b9bc2349d4b472f5d8db58cdbfdd2db0baf on August 17, 2026:

  • Read issue Remove Owner acceptance shadow mode and make Launchplane authoritative #2164 and every comment, all PR Make Launchplane authoritative for Owner acceptance #2168 review comments, the linked parent/blocker issues, VeriReel PR feat(every-code): record PR feedback webhooks #343 and its managed Owner handoff, plus the available desktop/narrow browser-smoke evidence.
  • Confirmed no linked GitHub image/file attachments were omitted; local browser evidence shows authoritative Owner review states and responsive narrow layouts without shadow wording or no-effect controls.
  • Fresh focused tests passed: 47 tests across tests.test_github_app_identity, tests.test_preview_pr_feedback, and tests.test_owner_acceptance_http.
  • Fresh PR snapshot: 26 passed, 0 failed, 0 pending, no new review items, mergeable, and ready_to_merge (GitHub reports the branch behind main).
  • git diff --check passed; the worktree is clean; repository searches find no live Owner shadow-mode reader/writer/control. Remaining Owner-shadow text is limited to historical migration material that archives/removes the legacy schema.
  • External-comment radar is clear. No merge, rebase, or merge-forward was performed; the exact approved head and its evidence remain unchanged.

Love Gate

  • Love: One exact-change Launchplane Owner path now owns the authorization event, while GitHub remains a clear fail-closed projection and routing surface. The implementation, migration, tests, docs, and responsive UI move together without a dormant shadow fallback.
  • Do not love: The PR is behind main. That is visible and non-conflicting, and I intentionally did not change the approved exact head; any future head change must refresh checks and review evidence. No material implementation compromise remains.

PR #2168 already contains Closes #2164. It is ready for an explicit merge decision; do not merge automatically.

@cbusillo
cbusillo merged commit 4179e6c into main Aug 17, 2026
32 checks passed
@cbusillo
cbusillo deleted the every-code/cbusillo-launchplane-2164-every-code-cbusillo-launchplane-2164-5201700b0259b1e1 branch August 17, 2026 03:43
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.

Remove Owner acceptance shadow mode and make Launchplane authoritative

2 participants