fix: Codespaces — skip GITHUB_TOKEN auto-enable, cap retries, fix phantom command#415
Merged
anandgupta42 merged 1 commit intomainfrom Mar 24, 2026
Merged
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review.
Tip: disable this comment in your organization's Code Review settings.
📝 WalkthroughWalkthroughAddresses issue Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
anandgupta42
force-pushed
the
fix/codespaces-github-models-rate-limit
branch
2 times, most recently
from
3c83332 to
e2253d3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/opencode/test/provider/codespace-e2e.test.ts (1)
38-45: Lettmpdirown the config setup here.
withEnv()and the two standalone tmp dirs only writeopencode.json, so using the fixture’sconfigoption will simplify the setup and keep these tests on the repo’s standard path.Suggested refactor
- await using tmp = await tmpdir({ - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ $schema: "https://altimate.ai/config.json" }), - ) - }, - }) + await using tmp = await tmpdir({ + config: { + $schema: "https://altimate.ai/config.json", + }, + })As per coding guidelines, "Use the
configoption to write anopencode.jsonconfiguration file when tests need project configuration".Also applies to: 228-238, 254-269
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/provider/codespace-e2e.test.ts` around lines 38 - 45, The test manually writes opencode.json inside the tmpdir init callback; instead use the tmpdir fixture's config option so the fixture owns config setup—replace the tmpdir({... init: async (dir) => await Bun.write(path.join(dir, "opencode.json"), ... ) }) usage with tmpdir({ config: { $schema: "https://altimate.ai/config.json" } }) (and apply the same change for the other tmpdir instances referenced around the test and lines 228-238, 254-269), removing the explicit Bun.write and leaving withEnv() and other fixtures to operate on the repo-standard config path.packages/opencode/test/provider/provider.test.ts (1)
2288-2297: Usetmpdir({ config })for these new cases.Each block only needs a minimal
opencode.json, so the fixture can create it directly and remove the repeatedBun.writesetup.Suggested refactor
- await using tmp = await tmpdir({ - init: async (dir) => { - await Bun.write( - path.join(dir, "opencode.json"), - JSON.stringify({ - $schema: "https://altimate.ai/config.json", - }), - ) - }, - }) + await using tmp = await tmpdir({ + config: { + $schema: "https://altimate.ai/config.json", + }, + })As per coding guidelines, "Use the
configoption to write anopencode.jsonconfiguration file when tests need project configuration".Also applies to: 2312-2321, 2338-2347
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/provider/provider.test.ts` around lines 2288 - 2297, The test uses tmpdir with an init callback that writes a minimal opencode.json via Bun.write; replace that pattern by passing the config option to tmpdir instead (e.g., tmpdir({ config: { $schema: "https://altimate.ai/config.json" } })) so the fixture creates the opencode.json automatically; update the three blocks that currently call tmpdir({ init: async (dir) => Bun.write(path.join(dir, "opencode.json"), ...) }) to use tmpdir({ config: { ... } }) and remove the init/Bun.write code (refer to the tmpdir call in the failing tests and the opencode.json payload to copy the minimal config).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/docs/reference/security-faq.md`:
- Line 171: Update the sentence about project-scoped MCP configs to reflect
actual behavior: they are auto-loaded by default (no per-server trust prompt)
controlled by the global flag auto_mcp_discovery, and the opt-out is the config
key experimental.auto_mcp_discovery: false; do not state they are "disabled by
default" or require explicit approval via mcp_discover—instead say they are
auto-loaded at startup and can be opted out with
experimental.auto_mcp_discovery: false.
In `@packages/opencode/src/provider/provider.ts`:
- Line 1088: Add "github-copilot-enterprise" to the skipGithubProviders set so
the enterprise variant is also excluded from the env-based auto-enable path;
update the declaration of skipGithubProviders (the Set created as const
skipGithubProviders = new Set([...])) to include "github-copilot-enterprise"
alongside "github-models" and "github-copilot" to prevent Codespaces/Actions
from auto-enabling the enterprise provider using GITHUB_TOKEN/GH_TOKEN.
- Line 1086: The check for machine CI (const isMachineEnv) incorrectly treats
any non-empty CODESPACES or GITHUB_ACTIONS value as true; change it to test for
the literal string "true" (e.g., env["CODESPACES"] === "true" ||
env["GITHUB_ACTIONS"] === "true") so that values like "false" don't enable the
flag; update the isMachineEnv assignment in provider.ts to use strict equality
against "true" for both env["CODESPACES"] and env["GITHUB_ACTIONS"] (referencing
the isMachineEnv variable and env map).
In `@packages/opencode/test/provider/codespace-e2e.test.ts`:
- Around line 125-135: The test "github-copilot is available outside Codespace
with GITHUB_TOKEN" currently makes no assertions; update the callback that calls
withEnv and Provider.list() to assert that providers includes the
"github-copilot" entry (e.g. providers["github-copilot"] is defined) and
optionally verify its source is "env" so the non-Codespace path cannot regress;
locate the assertion around the withEnv callback after the call to
Provider.list() and add expectations for providers["github-copilot"].
---
Nitpick comments:
In `@packages/opencode/test/provider/codespace-e2e.test.ts`:
- Around line 38-45: The test manually writes opencode.json inside the tmpdir
init callback; instead use the tmpdir fixture's config option so the fixture
owns config setup—replace the tmpdir({... init: async (dir) => await
Bun.write(path.join(dir, "opencode.json"), ... ) }) usage with tmpdir({ config:
{ $schema: "https://altimate.ai/config.json" } }) (and apply the same change for
the other tmpdir instances referenced around the test and lines 228-238,
254-269), removing the explicit Bun.write and leaving withEnv() and other
fixtures to operate on the repo-standard config path.
In `@packages/opencode/test/provider/provider.test.ts`:
- Around line 2288-2297: The test uses tmpdir with an init callback that writes
a minimal opencode.json via Bun.write; replace that pattern by passing the
config option to tmpdir instead (e.g., tmpdir({ config: { $schema:
"https://altimate.ai/config.json" } })) so the fixture creates the opencode.json
automatically; update the three blocks that currently call tmpdir({ init: async
(dir) => Bun.write(path.join(dir, "opencode.json"), ...) }) to use tmpdir({
config: { ... } }) and remove the init/Bun.write code (refer to the tmpdir call
in the failing tests and the opencode.json payload to copy the minimal config).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3e25d8cf-2741-4ca2-8577-ed5b20695266
📒 Files selected for processing (12)
.devcontainer/devcontainer.json.devcontainer/post-create.sh.github/meta/commit.txtdocs/docs/configure/tools.mddocs/docs/reference/security-faq.mdpackages/opencode/src/mcp/index.tspackages/opencode/src/provider/provider.tspackages/opencode/src/session/processor.tspackages/opencode/src/session/retry.tspackages/opencode/test/provider/codespace-e2e.test.tspackages/opencode/test/provider/provider.test.tspackages/opencode/test/session/retry.test.ts
anandgupta42
force-pushed
the
fix/codespaces-github-models-rate-limit
branch
2 times, most recently
from
b8cb0bd to
2a9d1ac
Compare
…x phantom command Closes #413 - Skip auto-enabling `github-models` and `github-copilot` providers in machine environments (Codespaces: `CODESPACES=true`, GitHub Actions: `GITHUB_ACTIONS=true`) when only machine-scoped tokens (`GITHUB_TOKEN`, `GH_TOKEN`) are available. The Codespace/Actions token lacks `models:read` scope needed for GitHub Models API. - Cap retry attempts at 5 (`RETRY_MAX_ATTEMPTS`) to prevent infinite retry loops. Log actionable warning when retries exhaust. - Replace phantom `/discover-and-add-mcps` toast with actionable message. - Add `.devcontainer/` config (Node 22, Bun 1.3.10) for Codespaces. - Add 32 adversarial e2e tests covering full Codespace/Actions env simulation, `GH_TOKEN`, token variations, config overrides, retry bounds. - Update docs to reference `mcp_discover` tool.
anandgupta42
force-pushed
the
fix/codespaces-github-models-rate-limit
branch
from
2a9d1ac to
2b2d41c
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/opencode/test/provider/provider.test.ts (1)
2286-2333: Good test coverage for the core Codespaces gating scenarios.The tests correctly verify:
github-modelsexclusion whenCODESPACES=truewith onlyGITHUB_TOKENgithub-modelsinclusion whenGITHUB_TOKENis set outside machine environmentsgithub-copilotexclusion in CodespacesThe defensive
Env.remove()calls in test 2 (lines 2308-2309) are good practice to prevent CI environment leakage.Consider adding tests for these additional scenarios (or confirm they're covered in
codespace-e2e.test.ts):
GITHUB_ACTIONS=trueenvironment detectiongithub-copilot-enterpriseprovider exclusion- Mixed token scenario (e.g.,
GITHUB_TOKEN+ customCOPILOT_API_KEY) where the provider should NOT be skipped🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/provider/provider.test.ts` around lines 2286 - 2333, Add tests covering the missing Codespaces/CI scenarios: create new tests using Instance.provide/tmpdir that (1) set Env.set("GITHUB_ACTIONS","true") with only GITHUB_TOKEN and assert Provider.list() excludes "github-models"/"github-copilot", (2) set a machine env and verify exclusion of "github-copilot-enterprise", and (3) set both Env.set("GITHUB_TOKEN","...") and a custom COPILOT_API_KEY (or similar) and assert the relevant provider is NOT skipped; reuse the same test pattern and helpers (Instance.provide, tmpdir, Env.set/Env.remove, Provider.list) and add assertions mirroring the existing tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/opencode/test/provider/provider.test.ts`:
- Around line 2286-2333: Add tests covering the missing Codespaces/CI scenarios:
create new tests using Instance.provide/tmpdir that (1) set
Env.set("GITHUB_ACTIONS","true") with only GITHUB_TOKEN and assert
Provider.list() excludes "github-models"/"github-copilot", (2) set a machine env
and verify exclusion of "github-copilot-enterprise", and (3) set both
Env.set("GITHUB_TOKEN","...") and a custom COPILOT_API_KEY (or similar) and
assert the relevant provider is NOT skipped; reuse the same test pattern and
helpers (Instance.provide, tmpdir, Env.set/Env.remove, Provider.list) and add
assertions mirroring the existing tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 12b8d5dc-492f-49ee-a106-c525e0971731
📒 Files selected for processing (14)
.devcontainer/devcontainer.json.devcontainer/post-create.sh.github/meta/commit.txtdocs/docs/configure/providers.mddocs/docs/configure/tools.mddocs/docs/reference/security-faq.mddocs/docs/usage/github.mdpackages/opencode/src/mcp/index.tspackages/opencode/src/provider/provider.tspackages/opencode/src/session/processor.tspackages/opencode/src/session/retry.tspackages/opencode/test/provider/codespace-e2e.test.tspackages/opencode/test/provider/provider.test.tspackages/opencode/test/session/retry.test.ts
✅ Files skipped from review due to trivial changes (6)
- packages/opencode/src/mcp/index.ts
- docs/docs/usage/github.md
- .devcontainer/post-create.sh
- docs/docs/reference/security-faq.md
- .devcontainer/devcontainer.json
- .github/meta/commit.txt
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/opencode/test/session/retry.test.ts
- packages/opencode/src/session/retry.ts
- packages/opencode/src/session/processor.ts
- docs/docs/configure/tools.md
- packages/opencode/test/provider/codespace-e2e.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes three issues reported by Brian Waligorski when running altimate-code in a fresh GitHub Codespace:
Skip
github-models/github-copilotauto-enable in Codespaces — The CodespaceGITHUB_TOKENis machine-scoped for repo operations, not model inference. Auto-enabling these providers led to immediate 429 "Too Many Requests" errors with infinite retry loops (attempt chore(deps-dev): Bump @parcel/watcher-darwin-arm64 from 2.5.1 to 2.5.6 #11, 33m backoff).Cap retry attempts at 5 — Added
RETRY_MAX_ATTEMPTS = 5to prevent infinite retry loops for any provider. Previously there was no upper bound.Fix phantom
/discover-and-add-mcpscommand — Toast notification referenced a non-existent slash command. Updated to "Ask the assistant to add them."Add
.devcontainer/config — Node 22 + Bun 1.3.10 for reproducible Codespace environments.Type of change
Issue for this PR
Closes #413
How did you verify your code works?
RETRY_MAX_ATTEMPTSassertion)--markers --base main --strict)Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
Tests