Skip to content

feat(core): [v2] restore OPENCODE_DISABLE_CLAUDE_CODE - #44725

Closed
malarahfelipe wants to merge 10 commits into
anomalyco:v2from
malarahfelipe:fix/opencode-disable-claude-code
Closed

malarahfelipe wants to merge 10 commits into
anomalyco:v2from
malarahfelipe:fix/opencode-disable-claude-code

Conversation

@malarahfelipe

Copy link
Copy Markdown

Issue for this PR

Part of #36990

Type of change

  • New feature

What does this PR do?

Brings back OPENCODE_DISABLE_CLAUDE_CODE support on the v2 branch.

V1 honored this flag to keep OpenCode from reading ~/.claude (the prompt and skills). On v2 the env var was declared in the flag module but never wired into config discovery, so the flag had no effect: config kept pulling in the global ~/.claude directory plus any .claude dirs found walking up from the project.

The change gates the claude source in Config.discover() on the flag. The global ~/.claude directory and project .claude dirs are both skipped when OPENCODE_DISABLE_CLAUDE_CODE is set, matching V1 behavior. Setting it stops .claude skills and the CLAUDE.md prompt from loading, which is useful when you don't use Claude Code and don't want its content appearing in OpenCode.

I also added disableClaudeCode to Config.Options so callers can control this at the API level as well, not just through the env var. A test covers the discovery behavior (claude sources dropped, .agents untouched).

How did you verify your code works?

  • Ran the config test suite for the core package: all 36 tests pass, including the new one.
  • The new test sets up a global ~/.claude + .agents and a project .agents, then asserts that with the flag on, no claude entries are discovered while agents still are.
  • Lint (oxlint) is clean on the touched files.

Screenshots / recordings

N/A

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@malarahfelipe

Copy link
Copy Markdown
Author

Did this as a partial implementation for the #36990 – can you take a look once you got time @rekram1-node ?

@malarahfelipe
malarahfelipe force-pushed the fix/opencode-disable-claude-code branch 2 times, most recently from 2a354d3 to ccabcb6 Compare September 11, 2026 11:47
@malarahfelipe
malarahfelipe force-pushed the fix/opencode-disable-claude-code branch from ccabcb6 to e2fcbd5 Compare September 11, 2026 11:55
@malarahfelipe

Copy link
Copy Markdown
Author

just rebased and solved conflicts, can you approve the workflows? @rekram1-node

@malarahfelipe malarahfelipe changed the title feat(core): restore OPENCODE_DISABLE_CLAUDE_CODE support in v2 feat(core): v2 – restore OPENCODE_DISABLE_CLAUDE_CODE Sep 15, 2026

@holny holny left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read this against current v2 — the placement looks right to me. discovery.ts is the only place that produces .claude roots, and everything downstream of sources.claude derives from that array: the skills scan in config/plugin/compatibility.ts:40 (via config.compatibility()) and the watch list in config/watch.ts:16. Emptying the array stops both the skill loading and the extra fs watches, and the AGENTS.md instruction plugin never touches .claude anyway, so this covers the whole surface on its own.

One gap: the env var is the part people actually set, but the new test only drives the disableClaudeCode option, so nothing pins the Config.boolean(...) wiring. It does work — I ran a probe against the effect version in this repo from packages/core and OPENCODE_DISABLE_CLAUDE_CODE=1 / yes gives true while 0 / unset gives false — but the config tests don't provide a ConfigProvider, so discovery now reads the ambient environment: with that variable exported in the shell, the assertions at config.test.ts:917 and :1573 flip to an empty claude list. Pinning a provider in one test, e.g. ConfigProvider.layer(ConfigProvider.fromEnv({ env: { OPENCODE_DISABLE_CLAUDE_CODE: "1" } })) the way packages/ai tests do, would cover the env path and keep the suite hermetic.

Minor: config.ts:47 documents what global: false skips; a one-liner on disableClaudeCode noting the env var as the fallback would help the next person wiring a CLI flag.

…_CODE

- testLayer now provides ConfigProvider (empty env by default) so config
  discovery never reads the ambient process environment.
- Add an env-path test alongside the disableClaudeCode option test, with a
  no-flag assertion so it cannot pass trivially.
- Assert via Config.compatibility(); the claude/agents entry types were
  removed on v2, which left the option test red after the rebase.
- Document the OPENCODE_DISABLE_CLAUDE_CODE fallback on the option.
@malarahfelipe

Copy link
Copy Markdown
Author

Thanks @holny — both points addressed in eaa5aa4.

Env path is now pinned. testLayer builds the Config graph with ConfigProvider.layer(ConfigProvider.fromEnv({ env })) (empty env by default), so discovery never reads the ambient environment and the suite is hermetic. I ran test/config/config.test.ts with OPENCODE_DISABLE_CLAUDE_CODE=1, yes, and 0: 42/42 pass in every case (previously 6 failed with the var exported).

New test covers the env var. It pins { OPENCODE_DISABLE_CLAUDE_CODE: "1" }, asserts compatibility.claude === [] with agents untouched, then repeats the same fixture without the flag and asserts the claude source is back — so the empty list can't pass trivially. I also mutation-checked it by renaming the variable in discovery.ts: the test fails, so the Config.boolean(...) wiring is genuinely pinned.

Doc comment added on disableClaudeCode noting the env var fallback.

One thing the rebase surfaced: the option test was still asserting on claude/agents entry types, which v2 removed in favor of Config.compatibility() — it was already red at the branch head. It now asserts through compatibility().

Unrelated heads-up: test/config/reload.test.ts has two failures ("loads skills when ../.claude appears after startup" and the home/.claude variant) that reproduce on base v2; those files are identical to v2, so I left them alone.

@kvnloo

kvnloo commented Sep 15, 2026

Copy link
Copy Markdown

Why this matters

OPENCODE_DISABLE_CLAUDE_CODE was declared on v2 but never gated discovery, so ~/.claude / project .claude still loaded (#36990).

Evidence

Tip eaa5aa4 (base v2, mergeable / blocked):

  • packages/core/src/config/discovery.ts — disableClaudeCode from option or OPENCODE_DISABLE_CLAUDE_CODE; empty claude sources when set
  • packages/core/src/config.ts — wires option through
  • live: bun test test/config/config.test.ts (DISABLE_CLAUDE|claude|OPENCODE_DISABLE) → 3/3 pass on tip; 1/1 on v2 (only the older global-disable case — the two new flag tests are tip-only)
  • CI: standards / compliance success

Confirms the flag actually skips Claude sources. Happy to help land.

Ask (design, light) — prospective

  1. Is “env declared but unwired” a one-off, or should discovery sources be the checklist for every config kill-switch?
  2. For never-again: one gate helper for optional config roots (claude/agents/…), or keep per-source if sites?

Extract the duplicated claude/agents root building into an optionalRoots
helper: roots vanish when disabled, and the global directory joins only
while the global scope is enabled. No behavior change; covered by the
config discovery tests.
@malarahfelipe

Copy link
Copy Markdown
Author

Follow-up, @kvnloo — took the helper option and landed it in 02f41dd.

// Optional roots vanish when disabled; their global directory joins only
// while the global scope is enabled.
const optionalRoots = (name: string, globalPath: AbsolutePath, enabled = true) => …

claude: optionalRoots(".claude", globalClaudeDirectory, !disableClaudeCode),
agents: optionalRoots(".agents", globalAgentsDirectory),

On your two questions:

  1. Discovery should stay the checklist — Sources is the single producer, so a kill-switch that isn't referenced there is visible in one file, and the env test pins the Config.boolean(...) read.
  2. Went with a shared builder rather than a generic gate registry: future roots/switches add one call site. Kept positional params (3, trailing enabled = true) since internal helpers in this package do the same (normalizeSkills(input, encoded, diagnostics), request(daemon, value, start = false)); object options here are reserved for exported/schema-backed surfaces like discover(options?).

Behavior unchanged: 42/42 config tests with the var unset and =1, plus packages/core typecheck and oxlint clean.

@malarahfelipe malarahfelipe changed the title feat(core): v2 – restore OPENCODE_DISABLE_CLAUDE_CODE feat(core): [v2] restore OPENCODE_DISABLE_CLAUDE_CODE Sep 16, 2026
@malarahfelipe
malarahfelipe requested a review from holny September 16, 2026 19:21
@malarahfelipe

Copy link
Copy Markdown
Author

Can you help me land that @kvnloo ? cc @holny @rekram1-node

@kvnloo

kvnloo commented Sep 17, 2026

Copy link
Copy Markdown

taking a look @malarahfelipe

@kvnloo

kvnloo commented Sep 17, 2026

Copy link
Copy Markdown

Live-validated tip bb6fce9 (base v2):

  • leaf: packages/core/src/config/discovery.ts — optionalRoots + disableClaudeCode / OPENCODE_DISABLE_CLAUDE_CODE; option wired in config.ts
  • live: bun test test/config/config.test.ts (DISABLE_CLAUDE|claude|OPENCODE_DISABLE filter) → 3/3 pass (two new flag tests tip-only vs v2)
  • CI: pr-standards success; required test + check still action_required (no jobs) — mergeable but blocked on workflow approval, not a code failure

Happy to help land once those workflows are approved/re-run. No competing PR from us.

@kvnloo

kvnloo commented Sep 17, 2026

Copy link
Copy Markdown

Thanks @malarahfelipe for landing the optionalRoots helper — that closes the shared-builder follow-up cleanly.

Live on tip bb6fce9 (base v2): bun test test/config/config.test.ts -t "DISABLE_CLAUDE|claude|OPENCODE_DISABLE" → 3/3 pass. Leaf is in good shape (discovery.ts gates claude via optionalRoots(..., !disableClaudeCode); option wired in config.ts).

CI: pr-standards green; required test + check are action_required (fork workflow approval), not a code failure. I can’t approve those from this account.

Next action: @rekram1-node — on this PR’s Actions banner, click Approve and run workflows for the waiting test / check runs. No new PR and no further code change needed from us.

Config.withDefault only covers absent input, so a malformed
OPENCODE_DISABLE_CLAUDE_CODE value leaked ConfigError into discovery and,
through Config.node, broke replacement typing in @opencode/server tests.
Fall back to false on any ConfigError.
@malarahfelipe

Copy link
Copy Markdown
Author

Update on the two red jobs at 8897b80f:

typecheck — fixed, and it was ours. Config.withDefault only covers absent input, so a malformed OPENCODE_DISABLE_CLAUDE_CODE value leaked ConfigError into discover → Config.node → Generate.node.replace(...) in packages/server/test/generate.test.ts, which the replacement typing correctly rejects (the fake Generate node depends on Config.node). discovery.ts now falls back on any ConfigError (Effect.orElseSucceed(() => false)), so discovery stays infallible.

unit (windows) — fixed by the v2 update. The failure was browser-idle.test.ts timing out; upstream v2 passes that exact job, and the fix arrived with the newer location-services.ts lifecycle rework now merged here.

Local verification on the new tip: full turbo typecheck 35/35; config tests 42/42 with the flag unset and =1; packages/server/test/generate.test.ts and packages/core/test/browser-idle.test.ts pass.

@kvnloo could you approve the workflows on 8897b80f? pr-standards is green; test + check are action_required (fork approval) — unit (windows) and typecheck should be green this time.

@malarahfelipe

Copy link
Copy Markdown
Author

can you approve the workflow to run it again @rekram1-node ? thxxxx

@malarahfelipe

Copy link
Copy Markdown
Author

cc @holny

@github-actions

Copy link
Copy Markdown
Contributor

Automated PR Cleanup

Thank you for contributing to opencode.

Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions.

This PR was closed because it matched the following cleanup criteria:

  • The PR was created more than 1 month ago
  • The PR had fewer than 2 positive reactions
  • Positive reactions are counted as thumbs-up, heart, celebration, or rocket reactions on the PR

PRs created within the last month are not affected by this cleanup.

If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate.

Thanks again for taking the time to contribute.

@malarahfelipe

Copy link
Copy Markdown
Author

I think this should be reopened

@malarahfelipe

Copy link
Copy Markdown
Author

Superseded by #51888. This PR can't be reopened by the author (viewerCanReopen=false — reopening is limited to users with write access), so the replacement carries the same branch and review context. Thanks @holny @kvnloo for the review here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants