Skip to content

fix(core): keep v1 theme color names when migrating agents - #54257

Open
4sDriven wants to merge 1 commit into
anomalyco:devfrom
4sDriven:fix/v1-agent-theme-color
Open

4sDriven wants to merge 1 commit into
anomalyco:devfrom
4sDriven:fix/v1-agent-theme-color

Conversation

@4sDriven

@4sDriven 4sDriven commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Issue for this PR

Related to #54057 -- the "agent is silently dropped" half is already fixed upstream by #57910, so this PR now only covers the part that is still broken: V1 migration.

Type of change

  • Bug fix

What does this PR do?

V2 only accepts a six-digit hex for an agent color, but V1 also accepted theme color names (primary, secondary, accent, success, warning, error, info). The V1 migration replaced every named color with the grey fallback #aaaaaa, so a migrated legacy agent lost its intended color even though the name is still present in the V1 schema.

migrateAgent now resolves the known theme names to the default theme's hex and leaves hex values untouched:

  • packages/core/src/v1/config/agent.ts -- expose the theme color names plus the name -> hex table and resolveThemeColor.
  • packages/core/src/v1/config/migrate.ts -- use it instead of the hard-coded #aaaaaa.
  • packages/core/test/config/agent.test.ts -- the existing legacy file-based agent now uses color: accent and asserts #9d7cd8.

Native agent files are intentionally unchanged: #57910 made the V2 schema ignore unsupported colors, and this PR does not re-open that decision.

How did you verify your code works?

  • bun test test/config/agent.test.ts -> 36 pass / 0 fail.
  • bun run typecheck in packages/core -> clean.
  • Reverting the migrate.ts hunk while keeping the new assertion fails with #aaaaaa, so the regression is covered.
  • bun test test/config has the same 6 failures on unmodified dev in this environment (subagent background runs + global config discovery); they are unrelated to this change.

Screenshots / recordings

N/A -- covered by the test above.

Checklist

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

Added by the opencode bot (AI-generated).

Related: #54057

Public API: v1 theme color names on migrated agents become default-theme hex instead of #aaaaaa (backwards compatible; schema unchanged)

@github-actions github-actions Bot added needs:compliance This means the issue will auto-close after 2 hours. needs:issue labels Oct 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I read the change but did not get to run it: the dependency install did not finish in time, so the reproduction from #54057, the tests and the type check were not run. From reading the code, the approach looks right. Theme names are mapped to hex at the load boundary and during v1 migration, the hex-only v2 schema stays unchanged, and the hex values match the default opencode theme's dark palette.

Public API: the config schema is unchanged. Agent color values that were already valid still are, and v1 theme names now load as default-theme hex values instead of being dropped (native files) or turned into #aaaaaa (legacy files). This is backwards compatible.

Things to be aware of:

  • This reverses the choice made in #38414, which mapped named colors to #aaaaaa on purpose. With this change, names resolve against the default theme's dark palette rather than the user's active theme, as v1 did. Maintainers should confirm that trade-off is acceptable.
  • Any other color that is not a six-digit hex still drops the whole agent with nothing logged. That part of #54057 is not addressed here.
  • resolveThemeColor uses color in ThemeColorHex, which also matches inherited keys like constructor. It has no practical effect today, but Object.hasOwn would be the correct check.

Related: #38414

@github-actions github-actions Bot removed the needs:compliance This means the issue will auto-close after 2 hours. label Oct 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for updating your PR! It now meets our contributing guidelines. 👍

@thdxr
thdxr changed the base branch from v2 to dev October 10, 2026 19:40
@4sDriven
4sDriven force-pushed the fix/v1-agent-theme-color branch from 372fb70 to 4ec2ef5 Compare October 12, 2026 02:44
@4sDriven

4sDriven commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks for the review, addressed below:

  • color in ThemeColorHex -> now Object.hasOwn(ThemeColorHex, color) (pushed in 4ec2ef5). You are right that in walks the prototype chain, so e.g. constructor would have been treated as a theme name.
  • Other non-hex colors: out of scope here - this PR only restores the v1 theme color names that agents: V1 theme color names silently drop agent files in V2 #54057 lost. A color that is neither six-digit hex nor a known theme name still drops the agent; happy to open a follow-up that keeps/resolves those instead if that is wanted.
  • Reversal of fix(core): migrate named agent colors #38414: intentional. v1 resolved names against the default theme palette; fix(core): migrate named agent colors #38414 mapped them to #aaaaaa only because v2 had no name support at the time. This restores v1 semantics (default-theme hex) and leaves the hex-only v2 schema unchanged. If maintainers would rather resolve against the user active theme at render time, I can do that instead.

Tests: packages/core bun run script/test.ts test/config/agent.test.ts -> 36 pass; bun run typecheck clean.

V1 accepted theme color names for an agent color (primary, �ccent, ...) but V2 only accepts
six-digit hex, so migration replaced every theme name with the grey fallback \#aaaaaa\. Resolve the
known names to the default theme's hex instead, leaving hex values and unknown colors untouched.
@4sDriven
4sDriven force-pushed the fix/v1-agent-theme-color branch from 4ec2ef5 to c61f8b0 Compare October 22, 2026 02:20
@4sDriven 4sDriven changed the title fix(core): keep agents using v1 theme color names fix(core): keep v1 theme color names when migrating agents Oct 22, 2026
@4sDriven

Copy link
Copy Markdown
Contributor Author

Rebased onto dev (248bfbe1) and narrowed the scope after #57910 landed: that PR already keeps agents with unsupported colors by ignoring the color, so this one no longer touches the native agent path. What is left is the V1 migration still rewriting a named color to the grey fallback #aaaaaa; it now resolves to the theme hex instead. Native-only frontmatter is untouched, so the behaviour #57910 added stays as-is.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I ran the #54057 reproduction from source. On dev, a legacy agent file (with a v1 permission block) and color: warning migrates to #aaaaaa. With this PR it gets #f5a742. The agent config tests and the packages/core type check pass. The in check from my last review now uses Object.hasOwn, and #57910 has fixed the silent drop.

Public API: the config schema is unchanged. The v1 theme color names on migrated agents now become default-theme hex values instead of #aaaaaa. This is backwards compatible.

One thing for maintainers to decide: #57910 chose to ignore theme-name colors on native agent files, so they get no color and the TUI picks one from its rotating palette. With this PR, the same color: accent resolves to #9d7cd8 only when the file also has a v1-only key. Which color a user sees then depends on unrelated keys in the file. Either both paths should map names to hex, or both should drop them. Mapping is also a reversal of the #aaaaaa fallback chosen in #38414, and it uses the default theme's dark palette rather than the user's active theme.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant