Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,25 @@ jobs:
advanced-security: false
inputs: .github/workflows

codex-comments:
name: Codex Comments
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: read
steps:
- name: Checkout
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
with:
fetch-depth: 0
persist-credentials: false

- name: Check unresolved Codex comments
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: ./scripts/check_codex_comments.sh ${{ github.event.pull_request.number }}

publish-main:
name: Publish GHCR :main
needs: [test, lint, lint-actions]
Expand Down
129 changes: 129 additions & 0 deletions .mux/skills/pull-requests/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
---
name: pull-requests
description: Guidelines for creating and managing Pull Requests in this repo
---

# Pull Request Guidelines

## Attribution Footer

Public work (issues/PRs/commits) must use 🤖 in the title and include this footer in the body:

```md
---

_Generated with `mux` • Model: `<modelString>` • Thinking: `<thinkingLevel>` • Cost: `$<costs>`_

<!-- mux-attribution: model=<modelString> thinking=<thinkingLevel> costs=<costs> -->
```

Always check `$MUX_MODEL_STRING`, `$MUX_THINKING_LEVEL`, and `$MUX_COSTS_USD` via bash before creating or updating PRs—include them in the footer if set.

## Lifecycle Rules

- Before submitting a PR, ensure the branch name reflects the work and the base branch is correct.
- PRs are always squash-merged into `main`.
- Often, work begins from another PR's merged state; rebase onto `main` before submitting a new PR.
- Reuse existing PRs; never close or recreate without instruction.
- Force-push minor PR updates; otherwise add a new commit to preserve the change timeline.
- If a PR is already open for your change, keep it up to date with the latest commits; don't leave it stale.
- Never enable auto-merge or merge into `main` yourself. The user must explicitly merge PRs.

## CI & Validation

- After pushing, you may use `./scripts/wait_pr_checks.sh <pr_number>` to wait for CI to pass.
- Use `wait_pr_checks` only when there's no more useful work to do.
- Waiting for PR checks can take 10+ minutes, so prefer local validation first (for this repo: `make verify-vendor`, `make test`, `make build`) to catch issues early.
- If asked to fix an issue in CI, first replicate it locally, get it to pass locally, then use `wait_pr_checks` to wait for CI to pass.

## Status Decoding

| Field | Value | Meaning |
| ------------------ | ------------- | ------------------- |
| `mergeable` | `MERGEABLE` | Clean, no conflicts |
| `mergeable` | `CONFLICTING` | Needs resolution |
| `mergeStateStatus` | `CLEAN` | Ready to merge |
| `mergeStateStatus` | `BLOCKED` | Waiting for CI |
| `mergeStateStatus` | `BEHIND` | Needs rebase |
| `mergeStateStatus` | `DIRTY` | Has conflicts |

If behind: `git fetch origin && git rebase origin/main && git push --force-with-lease`.

## Codex Review Workflow

When posting multi-line comments with `gh` (e.g., `@codex review`), **do not** rely on `\n` escapes inside quoted `--body` strings (they will be sent as literal text). Prefer `--body-file -` with a heredoc to preserve real newlines:

```bash
gh pr comment <pr_number> --body-file - <<'EOF'
@codex review

<message>
EOF
```

### Handling Codex Comments

Use these scripts to check, resolve, and wait on Codex review comments:

- `./scripts/check_codex_comments.sh <pr_number>` — Lists unresolved Codex comments (both regular comments and review threads). Outputs thread IDs needed for resolution.
- `./scripts/resolve_pr_comment.sh <thread_id>` — Resolves a review thread by its ID (e.g., `PRRT_abc123`).
- `./scripts/wait_pr_codex.sh <pr_number>` — Waits for Codex to respond to the latest `@codex review` request. When the PR looks good, Codex leaves an explicit approval comment (e.g., it will say `Didn't find any major issues`).

- `./scripts/wait_pr_ready.sh <pr_number>` — Convenience wrapper that runs Codex waiting first and CI checks second. Use it when you are done coding and want to block until the PR is ready or actionable feedback appears.

When Codex leaves review comments, you **must** address them before the PR can merge:

1. Push your fixes
2. Resolve each review thread: `./scripts/resolve_pr_comment.sh <thread_id>`
3. Comment `@codex review` to re-request review
4. Run `./scripts/wait_pr_codex.sh <pr_number>` to wait for the next Codex response (either new comments to address, or an explicit approval comment)

### Required Loop Discipline

After a PR is open, stay in a review loop until completion:

1. Run local validation and push fixes.
2. Request review with `@codex review`.
3. Run `./scripts/wait_pr_codex.sh <pr_number>` and wait for Codex to respond.
4. If Codex leaves comments, address them, resolve each thread, push, and repeat from step 2.
5. Once Codex explicitly approves, run `./scripts/wait_pr_checks.sh <pr_number>` and wait for required checks.

Only stop the loop early if a reviewer is clearly misunderstanding the change intent and further edits would be counterproductive. In that case, leave a clarifying PR comment and pause for human direction.

## PR Title Conventions

- Title prefixes: `perf|refactor|fix|feat|ci|tests|bench`
- Example: `🤖 fix: handle workspace rename edge cases`
- Use `tests:` for test-only changes (test helpers, flaky test fixes, storybook)
- Use `ci:` for CI config changes

## PR Bodies

### Structure

PR bodies should generally follow this structure; omit sections that are N/A or trivially inferable for the change.

- Summary
- Single-paragraph executive summary of the change
- Background
- The "why" behind the change
- What problem this solves
- Relevant commits, issues, or PRs that capture more context
- Implementation
- Validation
- Steps taken to prove the change works as intended
- Avoid boilerplate like `ran tests`; include this section only for novel, change-specific steps
- Do not include steps implied by passing PR checks
- Risks
- PRs that touch intricate logic must include an assessment of regression risk
- Explain regression risk in terms of severity and affected product areas

## Upkeep

Once the code is pushed to the remote (even if not yet a Pull Request), do your best to commit
and push all changes before responding to ensure its visible to the user. Commits on the working branch
are for yourself to understand the change, they do not have to follow repository conventions as the
PR body and title become the commit subject and body respectively.

Whenever generating a compaction summary, include whether or not a Pull Request was opened
and the general state of the remote (e.g. CI checks, known reviews, divergence).
21 changes: 16 additions & 5 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,20 @@ Run from repository root.

### Pull request descriptions
- Include: what changed, why, validation commands run, and any follow-up work.
- For public mux-generated PRs/commits in this environment, include footer:
- For public mux-generated PRs/commits in this environment, include the attribution footer defined in `.mux/skills/pull-requests/SKILL.md`.

```md
---
_Generated with [`mux`](https://github.com/coder/mux) • Model: `<modelString>` • Thinking: `<thinkingLevel>`_
```
## PR Workflow (Codex)

- Before creating or updating any PR, commit, or public issue, read `.mux/skills/pull-requests/SKILL.md` and follow it.
- Use `./scripts/wait_pr_ready.sh <pr_number>` for a one-command wait flow after requesting review.
- Prefer `gh` CLI for GitHub interactions over manual web/curl flows.

When a PR exists, stay in this loop until ready:
1. Push your latest fixes.
2. Run local validation (`make verify-vendor`, `make test`, `make build`).
3. Request review with `@codex review`.
4. Run `./scripts/wait_pr_codex.sh <pr_number>` and wait for Codex.
5. If Codex leaves comments, address them, resolve threads with `./scripts/resolve_pr_comment.sh <thread_id>`, push, and repeat.
6. After explicit Codex approval, run `./scripts/wait_pr_checks.sh <pr_number>`.

Only stop the loop early if the reviewer is clearly misunderstanding the intended change and further churn would be counterproductive. In that case, leave a clarifying PR comment and wait for human direction.
203 changes: 203 additions & 0 deletions scripts/check_codex_comments.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
#!/usr/bin/env bash
set -euo pipefail

if [ $# -eq 0 ]; then
echo "Usage: $0 <pr_number>"
exit 1
fi

PR_NUMBER=$1
BOT_LOGIN_GRAPHQL="chatgpt-codex-connector"

if ! [[ "$PR_NUMBER" =~ ^[0-9]+$ ]]; then
echo "❌ PR number must be numeric. Got: '$PR_NUMBER'"
exit 1
fi

echo "Checking for unresolved Codex comments in PR #${PR_NUMBER}..."

REPO_INFO=$(gh repo view --json owner,name --jq '{owner: .owner.login, name: .name}')
OWNER=$(echo "$REPO_INFO" | jq -r '.owner')
REPO=$(echo "$REPO_INFO" | jq -r '.name')

# Depot runners sometimes hit transient network timeouts to api.github.com.
# Retry the GraphQL request a few times before failing the required check.
MAX_ATTEMPTS=5
BACKOFF_SECS=2

# shellcheck disable=SC2016 # Single quotes are intentional - these are GraphQL queries.
COMMENTS_QUERY='query($owner: String!, $repo: String!, $pr: Int!, $cursor: String) {
repository(owner: $owner, name: $repo) {
pullRequest(number: $pr) {
comments(first: 100, after: $cursor) {
pageInfo {
hasNextPage
endCursor
}
nodes {
id
author { login }
body
createdAt
isMinimized
}
}
}
}
}'

# shellcheck disable=SC2016 # Single quotes are intentional - these are GraphQL queries.
THREADS_QUERY='query($owner: String!, $repo: String!, $pr: Int!, $cursor: String) {
repository(owner: $owner, name: $repo) {
pullRequest(number: $pr) {
reviewThreads(first: 100, after: $cursor) {
pageInfo {
hasNextPage
endCursor
}
nodes {
id
isResolved
comments(first: 1) {
nodes {
id
author { login }
body
createdAt
path
line
}
}
}
}
}
}
}'

fetch_graphql_with_retry() {
local query="$1"
shift

local attempt
local backoff
backoff="$BACKOFF_SECS"

for ((attempt = 1; attempt <= MAX_ATTEMPTS; attempt++)); do
if gh api graphql \
-f query="$query" \
-F owner="$OWNER" \
-F repo="$REPO" \
-F pr="$PR_NUMBER" \
"$@"; then
return 0
fi

if [ "$attempt" -eq "$MAX_ATTEMPTS" ]; then
echo "❌ GraphQL query failed after ${MAX_ATTEMPTS} attempts"
return 1
fi

echo "⚠️ GraphQL query failed (attempt ${attempt}/${MAX_ATTEMPTS}); retrying in ${backoff}s..."
sleep "$backoff"
backoff=$((backoff * 2))
done
}

COMMENTS_CURSOR=""
ALL_COMMENTS='[]'

while true; do
if [ -n "$COMMENTS_CURSOR" ]; then
COMMENTS_RESULT=$(fetch_graphql_with_retry "$COMMENTS_QUERY" -F cursor="$COMMENTS_CURSOR")
else
COMMENTS_RESULT=$(fetch_graphql_with_retry "$COMMENTS_QUERY")
fi

if [ "$(echo "$COMMENTS_RESULT" | jq -r '.data.repository.pullRequest == null')" = "true" ]; then
echo "❌ PR #${PR_NUMBER} does not exist in ${OWNER}/${REPO}."
exit 1
fi

PAGE_COMMENTS=$(echo "$COMMENTS_RESULT" | jq '.data.repository.pullRequest.comments.nodes')
ALL_COMMENTS=$(jq -cn --argjson all "$ALL_COMMENTS" --argjson page "$PAGE_COMMENTS" '$all + $page')

HAS_NEXT=$(echo "$COMMENTS_RESULT" | jq -r '.data.repository.pullRequest.comments.pageInfo.hasNextPage')
if [ "$HAS_NEXT" != "true" ]; then
break
fi

COMMENTS_CURSOR=$(echo "$COMMENTS_RESULT" | jq -r '.data.repository.pullRequest.comments.pageInfo.endCursor')
if [ -z "$COMMENTS_CURSOR" ] || [ "$COMMENTS_CURSOR" = "null" ]; then
echo "❌ Assertion failed: comments pagination cursor missing while hasNextPage=true"
exit 1
fi
done

THREADS_CURSOR=""
ALL_THREADS='[]'

while true; do
if [ -n "$THREADS_CURSOR" ]; then
THREADS_RESULT=$(fetch_graphql_with_retry "$THREADS_QUERY" -F cursor="$THREADS_CURSOR")
else
THREADS_RESULT=$(fetch_graphql_with_retry "$THREADS_QUERY")
fi

if [ "$(echo "$THREADS_RESULT" | jq -r '.data.repository.pullRequest == null')" = "true" ]; then
echo "❌ PR #${PR_NUMBER} does not exist in ${OWNER}/${REPO}."
exit 1
fi

PAGE_THREADS=$(echo "$THREADS_RESULT" | jq '.data.repository.pullRequest.reviewThreads.nodes')
ALL_THREADS=$(jq -cn --argjson all "$ALL_THREADS" --argjson page "$PAGE_THREADS" '$all + $page')

HAS_NEXT=$(echo "$THREADS_RESULT" | jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.hasNextPage')
if [ "$HAS_NEXT" != "true" ]; then
break
fi

THREADS_CURSOR=$(echo "$THREADS_RESULT" | jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.endCursor')
if [ -z "$THREADS_CURSOR" ] || [ "$THREADS_CURSOR" = "null" ]; then
echo "❌ Assertion failed: review thread pagination cursor missing while hasNextPage=true"
exit 1
fi
done

# Filter regular comments from bot that aren't minimized, excluding:
# - "Didn't find any major issues" (no issues found)
# - "usage limits have been reached" (rate limit error, not a real review)
REGULAR_COMMENTS=$(echo "$ALL_COMMENTS" | jq "[.[] | select(.author.login == \"${BOT_LOGIN_GRAPHQL}\" and .isMinimized == false and (.body | test(\"Didn't find any major issues|usage limits have been reached\") | not))]")
REGULAR_COUNT=$(echo "$REGULAR_COMMENTS" | jq 'length')

# Filter unresolved review threads from bot
UNRESOLVED_THREADS=$(echo "$ALL_THREADS" | jq "[.[] | select(.isResolved == false and .comments.nodes[0].author.login == \"${BOT_LOGIN_GRAPHQL}\")]")
UNRESOLVED_COUNT=$(echo "$UNRESOLVED_THREADS" | jq 'length')

TOTAL_UNRESOLVED=$((REGULAR_COUNT + UNRESOLVED_COUNT))

echo "Found ${REGULAR_COUNT} unminimized regular comment(s) from bot"
echo "Found ${UNRESOLVED_COUNT} unresolved review thread(s) from bot"

if [ "$TOTAL_UNRESOLVED" -gt 0 ]; then
echo ""
echo "❌ Found ${TOTAL_UNRESOLVED} unresolved comment(s) from Codex in PR #${PR_NUMBER}"
echo ""
echo "Codex comments:"

if [ "$REGULAR_COUNT" -gt 0 ]; then
echo "$REGULAR_COMMENTS" | jq -r '.[] | " - [\(.createdAt)]\n\(.body)\n"'
fi

if [ "$UNRESOLVED_COUNT" -gt 0 ]; then
echo "$UNRESOLVED_THREADS" | jq -r '.[] | " - [\(.comments.nodes[0].createdAt)] thread=\(.id) \(.comments.nodes[0].path // "comment"):\(.comments.nodes[0].line // "")\n\(.comments.nodes[0].body)\n"'
echo ""
echo "Resolve review threads with: ./scripts/resolve_pr_comment.sh <thread_id>"
fi

echo ""
echo "Please address or resolve all Codex comments before merging."
exit 1
fi

echo "✅ No unresolved Codex comments found"
exit 0
Loading
Loading