Conversation
|
Someone is attempting to deploy a commit to the World Monitor Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Adds a dedicated GitHub Actions workflow to automatically deploy Convex production after the existing “Deploy Gate” workflow succeeds on main, and only when Convex-related files changed.
Changes:
- Introduces
.github/workflows/deploy-convex.ymltriggered viaworkflow_runafter “Deploy Gate” completes successfully onmain. - Adds a pre-check using
gh apito skip deployment unlessconvex/**,package.json, orpackage-lock.jsonchanged. - Runs
npx convex deploy --cmd "npm run build" --yesusingCONVEX_DEPLOY_KEYfrom repo secrets.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| concurrency: | ||
| group: convex-prod-deploy | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
concurrency.cancel-in-progress: true can cancel an in-flight Convex deploy when a subsequent merge to main completes the Deploy Gate workflow. If the later merge doesn’t touch the watched paths, it will skip deployment and the earlier (cancelled) Convex changes may never get deployed. Consider setting cancel-in-progress: false so deploys serialize instead of being cancelled (or adjust the concurrency grouping so only superseding deploying runs cancel each other).
| cancel-in-progress: true | |
| cancel-in-progress: false |
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| REPO: ${{ github.repository }} | ||
| SHA: ${{ github.event.workflow_run.head_sha }} | ||
| CONVEX_DEPLOY_KEY: ${{ secrets.CONVEX_DEPLOY_KEY }} |
There was a problem hiding this comment.
CONVEX_DEPLOY_KEY is exported at the job level, which makes it available to steps that don’t need it (including the pre-check that prints output). To reduce accidental exposure risk, scope the secret to only the deploy step (or at least only to steps gated by should_deploy == 'true').
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80ecb1306b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| jobs: | ||
| deploy: | ||
| if: >- | ||
| github.event.workflow_run.conclusion == 'success' && |
There was a problem hiding this comment.
workflow_run.conclusion == 'success' is not a reliable signal that the merge gate passed. In .github/workflows/deploy-gate.yml, the gate job sets commit status to pending/failure but exits 0 in those branches, so the workflow conclusion remains success; this means this workflow can deploy on the first (still-pending) gate run and even after failed checks. Use the gate commit status result (or make Deploy Gate fail when checks fail) before allowing production deploy.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR adds
Confidence Score: 1/5Not safe to merge — the deploy job will never execute due to a secrets-in-job-if bug, and two additional logic issues pose production correctness risks once fixed. A P0 logic bug (secrets context always empty in job-level if) makes the entire workflow permanently inert on every run. A second P1 issue (cancel-in-progress: true) would risk corrupting the Convex production backend state on concurrent merges once the first bug is fixed. A third P1 issue (300-file GitHub API cap) can silently suppress deploys on large commits. All three must be addressed before this workflow can safely ship. .github/workflows/deploy-convex.yml — lines 17-21 (secrets job-if bug), lines 13-14 (cancel-in-progress), lines 42-49 (API truncation). Important Files Changed
Sequence DiagramsequenceDiagram
participant GH as GitHub (push to main)
participant CI as Test / Typecheck
participant Gate as Deploy Gate
participant DC as Deploy Convex
participant API as GitHub Commits API
participant Prod as Convex Production
GH->>CI: triggers on push to main
CI-->>Gate: workflow_run completed
Gate->>Gate: verify unit + typecheck both passed
Gate-->>DC: workflow_run completed (conclusion=success)
DC->>DC: check head_branch==main && repo guard
Note over DC: ⚠️ secrets check here always false
DC->>API: GET /repos/{repo}/commits/{sha}
Note over API: Max 300 files returned (no pagination)
API-->>DC: changed files list (possibly truncated)
alt convex/** or package.json changed
DC->>Prod: npx convex deploy --cmd 'npm run build' --yes
Note over DC,Prod: ⚠️ cancel-in-progress may interrupt this
Prod-->>DC: deploy complete
else no relevant files changed
DC->>DC: echo skip message
end
Reviews (1): Last reviewed commit: "ci: deploy Convex after merge gate" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if: >- | ||
| github.event.workflow_run.conclusion == 'success' && | ||
| github.event.workflow_run.head_branch == 'main' && | ||
| github.repository == 'koala73/worldmonitor' && | ||
| secrets.CONVEX_DEPLOY_KEY != '' |
There was a problem hiding this comment.
secrets context in job-level if always evaluates to empty — deploy never runs
secrets context is not available in job-level if conditions in GitHub Actions — it is always treated as an empty string at that evaluation point. This means secrets.CONVEX_DEPLOY_KEY != '' always evaluates to false, causing the entire deploy job to be silently skipped on every run. The workflow is currently a permanent no-op.
Remove the secrets sub-expression from the job if. If the secret is absent, npx convex deploy will emit a clear authentication error, which is a more visible and actionable failure mode than silent skipping. The fork guard is already handled by the github.repository check.
| if: >- | |
| github.event.workflow_run.conclusion == 'success' && | |
| github.event.workflow_run.head_branch == 'main' && | |
| github.repository == 'koala73/worldmonitor' && | |
| secrets.CONVEX_DEPLOY_KEY != '' | |
| if: >- | |
| github.event.workflow_run.conclusion == 'success' && | |
| github.event.workflow_run.head_branch == 'main' && | |
| github.repository == 'koala73/worldmonitor' |
| cancel-in-progress: true | ||
|
|
There was a problem hiding this comment.
cancel-in-progress: true can corrupt a production Convex deploy
For a production deployment workflow, cancel-in-progress: true means a second merge to main will forcibly terminate an already-running npx convex deploy — potentially mid-way through pushing schema changes or cloud functions. Cancelling a Convex deploy at that point can leave the backend in an inconsistent, partially-deployed state.
Use cancel-in-progress: false instead. Deploys are still serialised behind the convex-prod-deploy concurrency group, but the in-progress deploy is allowed to finish cleanly before the queued one starts.
| cancel-in-progress: true | |
| concurrency: | |
| group: convex-prod-deploy | |
| cancel-in-progress: false |
| result = subprocess.run( | ||
| ["gh", "api", f"repos/{repo}/commits/{sha}"], | ||
| check=True, | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| payload = json.loads(result.stdout) | ||
| changed_files = [entry["filename"] for entry in payload.get("files", [])] |
There was a problem hiding this comment.
files at 300 entries — large commits silently skip deploy
GET /repos/{owner}/{repo}/commits/{sha} caps the files array at 300 entries and this endpoint does not support pagination. If a single merge commit touches more than 300 files (large refactors across this codebase's 86+ components and 120+ service files can reach this), changed_files will be incomplete and should_deploy can evaluate to false even when convex/ files were actually modified — silently skipping the deploy with no error or warning.
Consider adding a cap-detection fallback that deploys unconditionally when the response likely hit the limit:
files = payload.get("files", [])
# GitHub caps the files list at 300 with no pagination support.
# If we hit the limit, deploy unconditionally to avoid silently missing a Convex change.
if len(files) >= 300:
should_deploy = True
else:
changed_files = [entry["filename"] for entry in files]
watched_paths = ("convex/", "package.json", "package-lock.json")
should_deploy = any(
path.startswith("convex/") or path in watched_paths[1:]
for path in changed_files
)
|
Addressed the review feedback in follow-up commit Changes in this push:
Local validation:
|
Summary
Deploy Gatesucceeds onmainconvex/**,package.json, orpackage-lock.jsonnpx convex deploy --cmd "npm run build" --yeswithCONVEX_DEPLOY_KEYfrom repository secretsWhy
Issue #1889 asks for Convex production deploys to stop being manual after merge. This workflow keeps deployment behind the repo's existing unit + typecheck gate instead of firing on every push immediately.
Notes
koala73/worldmonitorso forks do not fail on missing deploy secretsTesting
.github/workflows/deploy-convex.ymlsuccessfully as YAMLgit diff --checkFixes #1889