fix(agents): canonicalize stale persona harness pins - #4631
Open
wpfleger96 wants to merge 1 commit into
Open
Conversation
… command resolver The previous apply_persona_snapshot stale-pin check used known_acp_runtime_exact + known_acp_runtime with a pointer comparison to decide whether to drop a create-time agent_command_override. This had two failure modes: 1. Aliases (e.g. "claude-code-acp") are not the primary command and known_acp_runtime returns a different static slot — the pointer comparison treats them as different harnesses and drops the pin even when the persona stays on Claude. 2. Preset harnesses (e.g. openclaw) are not in KNOWN_ACP_RUNTIMES so known_acp_runtime_exact returns None and the pin is silently kept, meaning a Goose→OpenClaw persona switch leaves a stale Goose override running. Replace with canonical_harness_command, a three-tier resolver (builtins → static presets → loaded registry) that accepts either a runtime id or any command form (alias, path prefix, bare name). Comparison is on canonical primary commands so switching harnesses always drops the stale pin, while same-harness path overrides (e.g. /usr/local/bin/goose) are kept. Also consolidate the two-step known_acp_runtime_exact/lookup_loaded_harness_by_id pattern in record_agent_command, effective_agent_command, and try_record_agent_command into command_for_runtime_id — a shared three-tier lookup that adds the static preset tier so preset harnesses resolve correctly even with a cold registry. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96
changed the title
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.
Replace the stale
agent_command_overridedrop logic inapply_persona_snapshotwith a three-tier canonical command resolver.What this fixes
The old code dropped a create-time harness pin when the persona switched to a different runtime, but it had two failure modes:
Preset harnesses invisible.
known_acp_runtime_exact()only searchesKNOWN_ACP_RUNTIMES(builtins). Preset harnesses such as OpenClaw live inPRESET_HARNESSES, so the destination lookup returnedNoneand the outerif letbranch never executed — a Goose→OpenClaw persona switch left the stale Goose override in place, keeping the agent running Goose instead of OpenClaw.Pin-side canonical resolution incomplete. The pin was resolved by
known_acp_runtime(), which searches by id/command/alias and returns a&KnownAcpRuntimeentry correctly. However, if the pin named an alias (e.g.claude-code-acp) and the destination was a preset harness absent from builtins, the outer guard still failed for the same reason as (1). The alias regression test pins the requirement that the canonical resolver must handle both sides: alias pins must be recognised and drops must fire when the destination is a known preset.How it works now
canonical_harness_command(input)accepts any form a stored override can take — bare command, alias, path prefix, or runtime id — and resolves it to the harness primary command through three tiers:KNOWN_ACP_RUNTIMES, matched by id/command/alias.PRESET_HARNESSES, matched by id or normalised command.command_for_runtime_id(id-only input, same three tiers) replaces the two-stepknown_acp_runtime_exact/lookup_loaded_harness_by_idpattern inrecord_agent_command,effective_agent_command, andtry_record_agent_command, adding the static preset tier so preset harnesses resolve correctly even without a warm registry.Changed files
discovery/presets.rs—preset_command_for_id,command_for_runtime_id,canonical_harness_commanddiscovery.rs— re-export new functions; makenormalize_command_identitypub(crate); refactor three command-resolution functions to usecommand_for_runtime_idcustom_harnesses.rs—loaded_harness_registryvisibilityfn→pub(super)(needed bycanonical_harness_command)persona_events.rs— replace two-stepknown_acp_runtime_exact/known_acp_runtime+ pointer comparison with canonical-command comparisonpersona_events/stale_pin_tests.rs(new) — four regression tests: Goose→OpenClaw drop, OpenClaw→Goose drop, claude-code-acp alias→OpenClaw drop, same-harness path keeppersona_events/tests.rs—sample_record/sample_personaexposed aspub(super)for the new test module