Apply agentOverrides to user/project custom agents (frontmatter wins per field)#219
Open
jjuraszek wants to merge 1 commit into
Open
Apply agentOverrides to user/project custom agents (frontmatter wins per field)#219jjuraszek wants to merge 1 commit into
jjuraszek wants to merge 1 commit into
Conversation
Previously, settings.json overrides only resolved against builtin agents. Matches against custom user-scope or project-scope agents were silently dropped, even though the setting key (agentOverrides, not builtinAgentOverrides) and the docs prose imply broader scope. This change applies overrides to custom agents too, with one safety rule: frontmatter wins per-field. The override only fills fields the agent's frontmatter left unset. So existing custom agents that pin their own model / thinking / etc. keep their pin; agents that left a field unset now inherit the override that previously sat idle. Use case: a shared persona file (.pi/agents/<name>.md) under version control with no model: in frontmatter, paired with a per-harness ~/.pi/agent.<flavor>/settings.json that supplies the local model identifier. Lets one repo serve developers running different Pi distributions (e.g. direct Anthropic vs Bedrock) without committing a provider choice into the repo. Behavior preserved: - Builtin overrides: unchanged. Builtins ship with no frontmatter, so fill-in semantics collapse to the existing overwrite behavior. - Custom agent with frontmatter model: unchanged (test renamed to make the per-field rule explicit). - agentOverrides entry matching no agent: unchanged (still a no-op). - disableBuiltins: still touches only builtins. Tests: 6 added covering project-fill, user-fill, project-precedence over user, user-applies-to-project-agent, no-match-no-op, and disableBuiltins-does-not-touch-custom. All 19 tests in agent-overrides.test.ts pass. Pre-existing failures in unrelated files (fork-context, nested-control, render-helpers, widget-nested-render) are identical before and after this change. Note: agent-overrides.test.ts does not isolate PI_CODING_AGENT_DIR in beforeEach, so the suite reads the developer's real config when that env var is set. Workaround: env -u PI_CODING_AGENT_DIR before running the tests locally. Worth fixing separately.
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
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.
Apply
agentOverridesto user/project custom agents (frontmatter wins per field)Fixes #218.
What this changes
subagents.agentOverrides.<name>now resolves against any agent with a matching name — user-scope, project-scope, or builtin. Previously it resolved only against builtins; matches against custom agents were silently dropped.One safety rule for custom agents: frontmatter wins per-field. The override fills only fields the agent's frontmatter left unset. Builtins continue to behave exactly as before because they ship with no frontmatter values, so per-field fill-in collapses to the existing overwrite behavior.
Why
Detailed motivation in the linked issue. The short version: teams ship a custom persona in
.pi/agents/<name>.mdso the prompt is shared and version-controlled, while individual developers run different Pi distributions (direct Anthropic, Bedrock, ...) whose model identifiers differ. There's no clean way today to keep the persona body shared and let each harness pick its own model —agentOverrideslooks like it should do exactly that, but doesn't reach custom agents.Implementation
Single source file:
src/agents/agents.ts. New helpers, sibling to the existingapplyBuiltinOverride/applyBuiltinOverrides:Wired into both discovery entry points:
disableBuiltinsdeliberately stays out of the custom path — bulk-disabling builtins shouldn't bulk-disable a user's hand-written agents.The helper name
applyBuiltinOverridesis now slightly misleading since a sibling function does the same for non-builtins. A rename (applyAgentOverridesBuiltin/applyAgentOverridesCustom, or merging the two) is a clean follow-up; kept out of this PR to keep the diff focused on behavior.Behavior summary
agentOverrides[name]entrymodelmodelmodeldefaultModeldisableBuiltins: trueOnly the bold row is new behavior.
Tests
6 new unit tests in
test/unit/agent-overrides.test.ts:fills in unset fields on a custom project agent from project agentOverridesfills in unset fields on a custom user agent from user agentOverridesapplies user agentOverrides to a custom project agent when project settings have no entryprefers project agentOverrides over user agentOverrides on a custom project agentleaves a custom agent untouched when no agentOverrides entry matches its namedisableBuiltins does not disable custom agentsThe pre-existing test asserting "frontmatter wins when a project agent shadows a builtin" stays green; renamed to
frontmatter wins per-field over agentOverrides for a shadowing project agentto spell out the rule that's now generalized.All 19 tests in
agent-overrides.test.tspass. Pre-existing failures in unrelated test files (fork-context,nested-control,render-helpers,widget-nested-render) are identical before and after this PR — verified withgit stash; git checkout main; node --test ...and back.Test isolation caveat
agent-overrides.test.tsresetsHOMEper-test but doesn't unsetPI_CODING_AGENT_DIR. If a developer has that env var pointing to a real~/.pi/agent.*/settings.jsonwithagentOverridesentries (typical when running tests from inside a Pi session), several existing tests fail because the suite reads real config instead of the temp fixture. Workaround:All 19 pass under that invocation. Worth tightening
beforeEachto also clearPI_CODING_AGENT_DIR, but I left it out of this PR — it's an orthogonal cleanup and would muddy the diff.CHANGELOG
Entry added under
[Unreleased] / Added:Happy to move it under
ChangedorFixedif you prefer — the behavior change is small enough that all three are defensible.Out of scope
applyBuiltinOverrides→applyAgentOverridesBuiltin(mechanical follow-up).agentOverrides[name]entry matches zero agents anywhere.beforeEachinagent-overrides.test.tsto clearPI_CODING_AGENT_DIR.Each is small and unrelated; happy to send separate PRs.
Verification
Reproduced locally against a real project-scope custom agent (
gridstrong/.pi/agents/implementer.md, nomodel:in frontmatter) with a user-scopeagentOverrides.implementer.model = "anthropic/claude-sonnet-4-6".Before:
discoverAgents(...)returns{ source: "project", model: undefined, override: undefined }, subagent dispatch runs onclaude-opus-4-7(the default).After:
discoverAgents(...)returns{ source: "project", model: "anthropic/claude-sonnet-4-6", override: { scope: "user", ... } }, subagent dispatch runs onclaude-sonnet-4-6. Builtin overrides forworker/reviewercontinue to apply identically.