ci: add aireceipts pr-check (notice-only cost receipt)#992
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
📝 WalkthroughWalkthroughA new GitHub Actions workflow file, aireceipts.yml, was added. It triggers on pull_request events, sets read/write permissions, and runs a check job that executes aireceipts-cli's pr-check command using the GitHub token. ChangesCI Workflow Addition
Estimated code review effort: 1 (Trivial) | ~2 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/aireceipts.yml (2)
10-10: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the
aireceipts-cliversion instead of using@latest.Using
@latestmeans every workflow run downloads and executes whatever version is currently published. A broken or compromised publish would immediately impact all PRs with no rollback path. Pin to a specific version for reproducibility and supply-chain safety.♻️ Proposed fix
- - run: npx -y aireceipts-cli@latest pr-check + - run: npx -y aireceipts-cli@<specific-version> pr-check🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/aireceipts.yml at line 10, The workflow step invoking aireceipts-cli is using `@latest`, which makes every run pull an unpinned version; update the run command in the aireceipts workflow to reference a specific aireceipts-cli version instead. Keep the existing pr-check invocation, but replace the floating tag with a fixed release so the job is reproducible and safer against unexpected publishes.
7-12: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd
timeout-minutesto the job.Without a timeout, a hung
npxprocess (network stall, interactive prompt) can keep the job running for up to 6 hours. Adding a shorttimeout-minutesis cheap insurance.♻️ Proposed fix
check: runs-on: ubuntu-latest + timeout-minutes: 5 steps:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/aireceipts.yml around lines 7 - 12, The check job is missing a job-level timeout, so the `npx -y aireceipts-cli@latest pr-check` step can hang indefinitely. Add a `timeout-minutes` setting directly on the `check` job in the workflow so the `check` job is capped even if the CLI stalls. Use the existing `check` job definition in the workflow and keep the fix minimal.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/aireceipts.yml:
- Line 10: The workflow step invoking aireceipts-cli is using `@latest`, which
makes every run pull an unpinned version; update the run command in the
aireceipts workflow to reference a specific aireceipts-cli version instead. Keep
the existing pr-check invocation, but replace the floating tag with a fixed
release so the job is reproducible and safer against unexpected publishes.
- Around line 7-12: The check job is missing a job-level timeout, so the `npx -y
aireceipts-cli@latest pr-check` step can hang indefinitely. Add a
`timeout-minutes` setting directly on the `check` job in the workflow so the
`check` job is capped even if the CLI stalls. Use the existing `check` job
definition in the workflow and keep the fix minimal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fdbb0945-fc30-41bf-b9bd-6bd00585c6f2
📒 Files selected for processing (1)
.github/workflows/aireceipts.yml
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f46532174b
ℹ️ 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".
| check: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - run: npx -y aireceipts-cli@latest pr-check |
There was a problem hiding this comment.
Keep the notice-only receipt step from failing PRs
In the .github/workflows/aireceipts.yml workflow I checked, this step is the whole job and it has no continue-on-error or || true, so any non-zero exit from npx—including failing to resolve/download aireceipts-cli@latest before the CLI's own notice-only handling runs—marks the PR workflow as failed. npx --help describes this as running a command from a local or remote npm package, so this adds a remote package-resolution failure mode that can turn a best-effort receipt into a red PR check; make the step/job explicitly non-blocking if the workflow is meant to never fail builds.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,12 @@ | |||
| name: aireceipts | |||
| on: [pull_request] | |||
There was a problem hiding this comment.
Use a writable event for fork PR receipts
For pull requests from forks or Dependabot, this pull_request trigger downgrades GITHUB_TOKEN write scopes to read-only even though the workflow asks for pull-requests: write (GitHub's workflow syntax docs state that forked PR write permissions are changed to read unless the repository explicitly sends write tokens). In that context, a branch that carries an aireceipts receipt ref still cannot have the receipt posted back to the PR, so the new check silently misses the external-contributor cases it is meant to annotate; use a safe pull_request_target/separate trusted posting path or explicitly gate/skip those events.
Useful? React with 👍 / 👎.
| check: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - run: npx -y aireceipts-cli@latest pr-check |
There was a problem hiding this comment.
Pin the npm CLI before granting PR write access
This job runs aireceipts-cli@latest with GH_TOKEN set to a token that requests pull-requests: write; since npx --help confirms this resolves and runs a local or remote npm package at workflow time, any later publish or compromise of that npm package immediately gets write access to PR metadata/comments without a repo change. Pin the package to an audited version (or vend/pin an action by digest/SHA) before passing it the write-scoped token.
Useful? React with 👍 / 👎.
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| jobs: |
There was a problem hiding this comment.
[SUGGESTION]: Add a concurrency group to cancel superseded runs.
This workflow runs on every pull_request event with no concurrency control, so each push to a PR starts a new job (re-downloading aireceipts-cli@latest each time) while earlier runs may still be in flight. A workflow-level concurrency group keyed on the PR ref with cancel-in-progress: true cancels superseded runs and avoids wasted CI across every PR in the repo:
concurrency:
group: aireceipts-${{ github.head_ref || github.run_id }}
cancel-in-progress: trueReply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 file)
Note: the higher-severity concerns on this change (unpinned Fix these issues in Kilo Cloud Reviewed by glm-5.2 · Input: 33.7K · Output: 14K · Cached: 179K Review guidance: REVIEW.md from base branch |
Merge the 5 commits on `main` since this branch was cut (bearer-auth remote MCP #793, session_start `source` telemetry #968, aireceipts CI #992, REVIEW.md docs #983/#984) into the upstream OpenCode v1.17.9 bridge branch. 9 conflicts resolved: - `altimate/telemetry/index.ts` — keep upstream `InstallationVersion`; re-add #968's `Flag`/`source` (per-session client label). - `session/index.ts` — keep upstream `permission` readonly cast AND #968's `metadata: row.metadata`. Session `metadata` already exists upstream (core `SessionTable`, upstream PR #23068), so #968's persistence layer is redundant. - `session/session.sql.ts` — accept upstream deletion (persistence moved to `@opencode-ai/core`); dropped #968's duplicate migration `20260626041744_elite_malcolm_colcord` (would crash "duplicate column: metadata" against upstream's `20260511173437_session-metadata`). - `test/session/session.test.ts` — keep upstream Effect harness (main's metadata cases were the redundant old-harness variant). - `config/config.ts` — take upstream (schema relocated to core Effect Schema); re-apply #793's `headersCommand`/`oauth` normalize passthrough. - `mcp/index.ts` — re-port #793 (bearer-auth) onto upstream's Effect-based MCP rewrite: `resolveHeadersCommand` (execFile, no shell), case-insensitive `mergeHeaders`, lenient `listTools` retry, OAuth auto-disable folded into upstream's `supportsOAuth`, `maskString` on failures. Adapted `Either`→`Result` (effect 4.0.0-beta.74). - `core/v1/config/mcp.ts` — add `headersCommand` to the Effect Schema `Remote` struct as a mutable non-empty string-array record (`Schema.mutable(...).check (Schema.isNonEmpty())`), so `DeepMutable`-wrapped consumers type-check. - `test/mcp/headers.test.ts`, `test/mcp/mcp-bearer-auth.test.ts` — re-expressed #793's schema/integration tests against the Effect Schema and `it.instance` harness (Zod `safeParse` → `Schema.decodeUnknownSync`). - SDK artifacts kept at the upstream-HEAD baseline (regenerated by CI/release). Verification: all 13 packages typecheck clean (`bun turbo typecheck`); MCP + session + telemetry suites 194/194; bearer-auth + headers 32/32. Zero conflict markers. Marker guard non-strict pass (CI mode for upstream-merge PRs). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a notice-only aireceipts cost-receipt check (pr-check) — one self-contained workflow, no external reusable workflow, no org Actions-policy change. Never fails a build; posts an aireceipts cost receipt only once a branch carries an aireceipts receipt ref, otherwise a quiet no-op. Coexists with any existing check; remove anytime by deleting the file. Part of an org-wide aireceipts dogfood.
Summary by cubic
Add a notice-only aireceipts PR check to post AI cost receipts on pull requests. It never fails builds and is a no-op unless the branch has an aireceipts receipt ref.
.github/workflows/aireceipts.ymlthat runs on pull requests.npx -y aireceipts-cli@latest pr-checkwithGH_TOKEN.Written for commit f465321. Summary will update on new commits.
Summary by CodeRabbit