Skip to content

docs(claude.md): switch PR review to local Codex via openai/codex-plugin-cc - #81

Merged
tomqwu merged 2 commits into
mainfrom
chore-claude-md-github-enforced-gate
May 9, 2026
Merged

tomqwu merged 2 commits into
mainfrom
chore-claude-md-github-enforced-gate

Conversation

@tomqwu

@tomqwu tomqwu commented May 8, 2026 •

Copy link
Copy Markdown
Owner

Replaces the (never-deployed) codex-pr-review-gate GitHub Action approach with local Codex review via openai/codex-plugin-cc. Drops the comment-marker polling logic; keeps the review fully agent-side.

What changed

  • Section renamed: ## GitHub-Enforced PR Review Gate → ## PR Workflow With Codex Review.
  • Merge criteria:
    1. CI green
    2. GitHub mergeable
    3. Codex local review passes
    4. No unresolved comments / merge conflicts
  • Documents the slash commands: /codex:review --base main for sync review; /codex:review --base main --background + /codex:status + /codex:result for larger changes.
  • Explicit prohibitions: don't post LGTM yourself; don't require or wait for the old codex-pr-review-gate GitHub check.
  • ## PR rules bullet 3 now references the new section name.

What this PR does NOT do

  • No branch-protection changes — main is not currently protected on this repo (per gh api repos/.../branches/main/protection returning 404), and rulesets are empty. There is nothing to remove.
  • No workflow deletion — .github/workflows/codex-pr-review-gate.yml was never created; only ci.yml exists.
  • No required-check change — Lint, type-check, and test (the ci.yml job) remains the lone CI check. The Codex review is local-only by design.

Background

The earlier version of this branch documented a server-enforced AI review gate (looking up a required codex-pr-review-gate check). The owner has since asked to switch to local-only Codex review through openai/codex-plugin-cc to avoid maintaining a GitHub Action. Branch force-pushed; the prior content is in this PR's force-push history.

🤖 Generated with Claude Code

@tomqwu tomqwu changed the title docs(claude.md): GitHub-enforced PR review gate docs(claude.md): switch PR review to local Codex via openai/codex-plugin-cc May 8, 2026
tomqwu and others added 2 commits May 8, 2026 22:30
The prior `## PR Review Gate` had the agent read LGTM comments directly,
which is brittle: it relies on the agent honoring a marker comment
rather than GitHub branch protection actually blocking the merge.

This change:
- Renames the section to `## GitHub-Enforced PR Review Gate`.
- Drops the "look at LGTM comment markers" semantics.
- Names a specific required check, `codex-pr-review-gate`, as the
  authoritative signal for independent AI review approval.
- Explicitly forbids the agent from posting `LGTM` or
  `<!-- codex-pr-review: ... -->` markers itself.
- Explicitly forbids reading old LGTM comments to bypass the check.
- Keeps the merge-conflict + Not-LGTM-yet response flows intact, but
  routes the "wait for re-review" step through the GitHub check
  rather than through agent-side comment polling.
- Updates the `## PR rules` reference so its third bullet points at
  the new section name.

No CI workflow, branch protection, or repository-settings change is
made by this commit — those are owner-side actions. The
`codex-pr-review-gate` check is referenced by name on the assumption
that the repo owner will land it (or already has).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the GitHub-enforced `codex-pr-review-gate` section with a
`## PR Workflow With Codex Review` section that points the agent at
the local Codex review plugin (`openai/codex-plugin-cc`).

Why: rather than building/maintaining a server-enforced GitHub Action
+ branch protection rule for AI review, run Codex locally from Claude
Code via `/codex:review --base main`. Removes the GitHub action
dependency, removes the comment-marker polling logic, and keeps the
review fully agent-side.

What changed:
- Section renamed: GitHub-Enforced PR Review Gate → PR Workflow With
  Codex Review.
- Merge criteria: CI green + GitHub mergeable + Codex local review
  passes + no unresolved comments / conflicts.
- Explicit prohibitions: don't post LGTM yourself; don't require or
  wait for the old codex-pr-review-gate GitHub check.
- Documents `/codex:review --base main` (sync) and the
  `/codex:review --background` + `/codex:status` + `/codex:result`
  flow for larger PRs.
- `## PR rules` bullet 3 now references the new section name.

What this PR does NOT do:
- Does NOT change branch protection (this repo has none on main).
- Does NOT remove any required check (no rulesets exist; no CI check
  named codex-pr-review-gate exists in workflows).
- Does NOT delete any workflow file (no codex-pr-review-gate.yml
  was ever created — this branch was the prior PR's content; the
  workflow itself was deferred).

If branch protection is added later, the only required check should
be the existing `Lint, type-check, and test` (and any new CI tier
the team adds). The codex review is intentionally local-only.

Supersedes the prior version of this branch (was titled
"GitHub-enforced PR review gate"); the substantive content is now
the local-Codex flow.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tomqwu
tomqwu force-pushed the chore-claude-md-github-enforced-gate branch from b8aedce to 0b4366c Compare May 9, 2026 02:30
@tomqwu
tomqwu merged commit fdaa0e2 into main May 9, 2026
1 check passed
@tomqwu
tomqwu deleted the chore-claude-md-github-enforced-gate branch May 9, 2026 02:41
tomqwu added a commit that referenced this pull request May 9, 2026
Squash-merges PR #83 (Sprint 9.6 Android Fastlane).

Adds the deploy lane and Play Console upload runbook for Android releases.

Process:
- Stale base trap: original branch was forked before #78/#79/#81/#82
  landed and the diff would have reverted ~2103 lines of recently-merged
  work. Rebased onto current main; post-rebase diff is the intended 3
  files (Appfile, Fastfile, ANDROID_RELEASE.md).

Codex review surfaced no P0/P1/P2 findings.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
tomqwu added a commit that referenced this pull request May 9, 2026
… 9.4b) (#84)

Squash-merges PR #84 (Sprint 9.4b invitation accept JWT pair).

Backend /invitations/{token}/accept now returns the same access+refresh
JWT pair as /auth/login + /auth/signup, so accepted invitations land in
the mobile app already authenticated and the dio refresh interceptor
(#82) can rotate the access token without re-prompting.

Process:
- Stale base (forked before #78/#79/#81/#82/#83 landed); rebased onto
  current main. Post-rebase diff is the intended 8 files.

Codex review surfaced 1 P2, no P0/P1:
- mobile/lib/auth/invitation_repository.dart should mirror the fix from
  PR #82: when the response omits refresh_token, clear any stale
  refresh credential in secure storage. Same 3-line pattern as
  login_repository.dart and signup_repository.dart. Deferred follow-up.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
tomqwu added a commit that referenced this pull request May 12, 2026
Squash-merges PR #86 (Sprint 9 P2 cleanup bundle).

Closes the deferred follow-up list across Sprint 9 PRs #78, #81, #82,
#84, #85 so the closeout (9.8) ships against a clean slate.

Six iterations to resolve every P0/P1; final state ships with three
narrow residual P2s documented inline as known limitations.

What landed:

- /forgot-password concurrent-request race: with_for_update() Person
  re-lookup scoped to the critical section (after the audit-log
  commit that would release any earlier-acquired lock), filtered by
  both id and org_id per tenancy convention.
- Settings.EMAIL_ENABLED default flipped to False to match the
  EmailService env-read gate; notification_service no longer queues
  Celery email jobs that the worker silently no-ops.
- Mobile refresh-in-flight signOut/login race: SecureTokenStorage now
  exposes a monotonic sessionGeneration counter that bumps on every
  mutator. _attemptRefresh snapshots gen at start, returns a tri-state
  _RefreshOutcome (success/failure/stale). Stale means storage changed
  during round-trip; interceptor doesn't replay and doesn't clearAll.
  Failure path also gated on the gen check so a refresh that fails
  after a fresh login doesn't wipe the new session.
- invitation_repository clears refresh slot when server omits
  refresh_token (mirrors login/signup #82 P2).
- email_smoke.py narrow override: dotenv_values() reads .env without
  side effects, only force-clears SENDGRID_API_KEY when .env
  explicitly sets it blank; runbook adds `unset SENDGRID_API_KEY`
  prelude for Path A.
- CLAUDE.md Codex review invocations use --base origin/main with a
  git fetch prelude.

Codex iter sequence:
- iter 1 → 3 P2: half-fixes (FOR UPDATE released by audit commit;
  refresh-stale wiped fresh login; load_dotenv too broad)
- iter 2 → P0 (missing org_id) + P1 (TOCTOU on stale check)
- iter 3 → P1 (inter-write race)
- iter 4 (inter-write rollback) introduced regression: clearToken
  could wipe fresh login's access. Reverted in iter 5.
- iter 5 → P1 (Dart scoping: genAtStart inside try, used in catch)
- iter 6 → 1 P2 (hypothetical server-side rotation interaction);
  no P0/P1, merging per rule.

Three residual P2s carried forward to follow-up:
- Inter-write race in _attemptRefresh persist (microsecond window in
  Dart single-isolate cooperative scheduler; fully closing needs
  package:synchronized or atomic compareAndWriteTokens on
  SecureTokenStorage).
- Server-side refresh-token rotation interaction (discarded rotated
  refresh after stale path can in theory leave a same-account
  re-login at a stale version; depends on backend rotation semantics).
- The two P2s previously deferred from #78's commit body remain.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.

1 participant