chore(config): remove deprecated default_agent schema field (closes #241)#550
Conversation
📝 WalkthroughWalkthroughThis PR removes the deprecated ChangesRemove default_agent infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/opencode/src/agent/agent.ts (1)
285-290: 💤 Low valueConsider sourcing
defaultAgentfrom the same sorted ordering aslist().
Object.values(agents).find(...)returns the first visible primary in insertion order. Today that yields"build"for the common case (since natives are seeded first) and the first config-declared custom primary whenbuildis disabled. Reusing the same sort key aslist()(build first, then alphabetical by name) would make the disabled-build tie-break deterministic regardless of how users order theiragentmap, and keepslist()[0]anddefaultAgent()consistent.♻️ Optional refactor
const defaultAgent = Effect.fnUntraced(function* () { yield* Effect.void - const visible = Object.values(agents).find((a) => a.mode !== "subagent" && a.hidden !== true) + const sorted = pipe( + agents, + values(), + sortBy([(x) => x.name === "build", "desc"], [(x) => x.name, "asc"]), + ) + const visible = sorted.find((a) => a.mode !== "subagent" && a.hidden !== true) if (!visible) throw new Error("no primary visible agent found") return visible.name })🤖 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 `@packages/opencode/src/agent/agent.ts` around lines 285 - 290, The defaultAgent selection uses Object.values(agents).find(...) which picks the first visible primary by insertion order; change defaultAgent (the Effect.fnUntraced function) to derive its result using the same ordering as list() so ties are deterministic: filter agents to mode !== "subagent" && hidden !== true && primary, then sort using the list() sort key (prefer "build" first, then alphabetical by name) and return the first entry's name; this ensures defaultAgent() and list()[0] are consistent regardless of agents map insertion order.
🤖 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 `@packages/opencode/src/agent/agent.ts`:
- Around line 285-290: The defaultAgent selection uses
Object.values(agents).find(...) which picks the first visible primary by
insertion order; change defaultAgent (the Effect.fnUntraced function) to derive
its result using the same ordering as list() so ties are deterministic: filter
agents to mode !== "subagent" && hidden !== true && primary, then sort using the
list() sort key (prefer "build" first, then alphabetical by name) and return the
first entry's name; this ensures defaultAgent() and list()[0] are consistent
regardless of agents map insertion order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b704966b-ca25-4b5a-8806-1e3ed0182bdb
⛔ Files ignored due to path filters (1)
packages/sdk/js/src/v2/gen/types.gen.tsis excluded by!**/gen/**
📒 Files selected for processing (8)
packages/opencode/src/agent/agent.tspackages/opencode/src/cli/cmd/github.tspackages/opencode/src/config/config.tspackages/opencode/src/config/migrate-default-agent.tspackages/opencode/test/agent/agent.test.tspackages/opencode/test/config/migrate-default-agent.test.tspackages/opencode/test/config/pawwork-global-config.test.tspackages/sdk/openapi.json
💤 Files with no reviewable changes (3)
- packages/opencode/src/config/migrate-default-agent.ts
- packages/opencode/test/config/migrate-default-agent.test.ts
- packages/sdk/openapi.json
There was a problem hiding this comment.
Code Review
This pull request removes the default_agent configuration field from the system, including its schema definition, migration logic, and associated tests. A read-time shim was added to normalizeLoadedConfig to strip the deprecated key from legacy configurations. The review feedback suggests optimizing this shim to avoid unnecessary object allocations and simplifying the agent-related Effect functions by replacing generator-based logic with synchronous wrappers, as the configuration context is no longer required in those paths.
First-pass Review — PR #550P0-P3 findings
1.
|
| Path | Expected | Actual | Result |
|---|---|---|---|
packages/opencode/src |
Only the normalize shim (1 site) | 2 hits (comment + delete) | ✓ Pass |
packages/opencode/test |
Only legacy fixtures / test names / assertions | 6 hits in pawwork-global-config.test.ts fixtures |
✓ Pass |
packages/sdk |
0 | 0 | ✓ Pass |
docs / cli |
0 | 0 | ✓ Pass |
Sole src reference (matches expectation):
// packages/opencode/src/config/config.ts:113-115
// Legacy compat for v0.2.13-era configs: ignore removed default_agent before strict schema decode.
delete copy["default_agent"]2. PR body completeness
- ✓ Summary section
- ✓ Verification section (
bun test,typecheck,rg default_agentresults) - ✓ Legacy config compatibility section
- ✓ Title follows Conventional Commits:
chore(config): remove deprecated default_agent schema field (closes #241) - ⚠ Labels:
enhancement/P3/harness/taskpresent, but area label missing (suggestconfig)
3. Issue #241 acceptance criteria match
| Acceptance item | Status | Evidence |
|---|---|---|
config.ts schema no longer defines default_agent |
✓ Pass | sed -n '164,320p' config.ts shows no default_agent field |
migrate-default-agent.ts and its test deleted |
✓ Pass | ls returns No such file or directory |
loadGlobal removes the migration call site |
✓ Pass | loadGlobal no longer calls migrateDefaultAgent |
Agent.defaultAgent() no longer reads c.default_agent |
✓ Pass | Simplified to find first visible primary |
| Release notes explain legacy config behavior | ✓ Pass | PR body "Legacy config compatibility" section |
4. Change summary
+18 -407 lines (9 files)
- Deleted migrate-default-agent.ts + its test
- config.ts: removed the schema field and the loadGlobal migration call
- agent.ts: defaultAgent simplified to first visible primary
- SDK: openapi.json + types.gen.ts no longer expose default_agent
Conclusion
P3 minor: add area label config (or app if config is not in the repo label set).
Acceptance pass: every #241 acceptance item is satisfied, reference scan matches the expected shape, and the diff is clean.
First-pass review by @glm (OpenCode). Rewritten in English by @Opus on behalf of GLM after a quota outage (2026-05-11 18:38).
There was a problem hiding this comment.
Opus second-pass review (product layer / three questions / user experience)
Conclusion: approve
PR is clean, matches the A+ scope, and does not break product semantics.
Three questions
Could this be even less? Already the minimum: a single-key normalize shim. Check.
Is what remains good enough?
agent.list/defaultAgentbehavior is identical (buildstill sorts first and remains the default primary).- Legacy configs are silently ignored, not written back, and the deleted migration path is not reintroduced.
- Regression tests cover both fallback branches (
defaultAgent returns build by defaultandpicks custom primary when build is disabled). - Check.
Is it reassuring? Legacy user upgrade path traced end-to-end:
- A v0.2.13 config on disk still contains
default_agent: "...". normalizeLoadedConfigdrops the field before schema decode.- Effective config exposes no
default_agent. - Agent list still puts
buildfirst; default primary unchanged. - Disk file is untouched, so the user sees no change. Check.
Findings
P3-1 (not blocking, fixable in merge): the config label does not exist in the repo, so I added the app area label instead. The existing harness label leans toward model harness / session mechanics; app more accurately reflects the changed surface (config schema + agent fallback wiring).
P3-2 (nit, do not change in this PR): the "Risk Notes", "Behavior change for legacy configs", and "Legacy config compatibility" sections in the PR body restate the same point three times. Content is complete, so this is a stylistic concern. Worth consolidating in a future PR template pass.
Approve
GPT-X engineering final reviewResult: pass. I found no P0/P1/P2 issues. Engineering checks
Binary check
NotesCodeRabbit's suggestion to make The prompt-input agent-pill cleanup mentioned in issue #241 is also outside this PR. It is a separate UI/prompt cleanup, not required for removing the config field. |
Summary
Removed the deprecated
default_agentconfig field from the opencode config schema, runtime dependency paths, migration file, tests, and SDK config types.Why
Closes #241.
default_agentwas deprecated after the default-agent fallback cleanup shipped, and v0.2.14+ has had time to migrate most active user configs. The runtime should no longer expose or depend on this config field.Related Issue
Closes #241
Human Review Status
Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.
Review Focus
default_agentare limited to the narrow legacy read-time shim innormalizeLoadedConfig().default_agentare ignored without disk writeback.buildstill sorts first and remains the default primary agent.default_agent.Risk Notes
Legacy user config files from v0.2.13 or earlier may still contain
default_agenton disk. The loader ignores that field at read time and does not rewrite the file. No agent naming, agent ordering, or future autonomous-agent semantics are changed in this PR.Behavior change for legacy configs
Legacy user config files that still contain
default_agentare silently ignored by the loader. No automatic writeback is performed. Users may remove the field manually if they want a clean config file.This intentionally accepts harmless dead noise in legacy configs to avoid maintaining the migration path indefinitely. v0.2.14+ has shipped for 2 weeks with the migration; subsequent loads after that release already cleaned most user configs in the wild.
Legacy config compatibility
Info.zodis strict on unknown keys in thenormalizeLoadedConfig()decode path. Legacy user configs from v0.2.13 and earlier that still containdefault_agentwould otherwise fail validation. The loader keeps a single-key shim innormalizeLoadedConfig()that dropsdefault_agentbefore schema decode.migrate-default-agent.tsmigration pathHow To Verify
Screenshots or Recordings
Not applicable. No visible UI changes.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
New Features
Chores
default_agentconfiguration field.small_modelconfiguration field to the schema.