fix(opencode): upgrade manifests written before general_agent existed - #571
Merged
Conversation
manifest() has two return paths. The fresh-install branch was given the new general_agent key in v1.21.0, but the branch that loads an existing manifest returns the file as parsed, and a manifest on disk was written by whatever version wrote it. Every manifest already on disk therefore lacked the key, and main() subscripts previous["general_agent"] directly, so the projection raised KeyError on every install upgrading from <= v1.20.2 - which is every install that had ever run. That aborts upgrade.sh, so a host taking the release stops applying managed configuration partway through. Observed on a live VPS immediately after v1.21.0: 'OpenCode subagent projection failed (projector)'. A default on the fresh-install branch is not a migration; it is the case that cannot be affected by one. Normalize on read instead, so both return paths have the same shape and the existing direct subscripts stay valid. The keys are defaulted as a set rather than one-off, because the next key added will have exactly this problem again. The test writes the real v1.20.2 on-disk state - no general_agent in the manifest, no agent.general in opencode.json, no task.general in either - and asserts the projection upgrades rather than aborting. It reproduces KeyError: 'general_agent' against the unfixed projector.
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.
Regression shipped in v1.21.0
manifest()has two return paths:v1.21.0 added
general_agentto the fresh-install branch, then subscripted it directly inmain():A manifest on disk was written by whatever version wrote it. v1.20.2 persisted:
{"sentinel", "agents", "artifacts", "task_permission", "skill_permission"}No
general_agent. So every install upgrading from <= v1.20.2 raisesKeyError: 'general_agent'— which is every install that had ever run. Only a genuinely fresh install takes the branch that has the key.Impact
The projection is a hard step in
upgrade.sh, so the whole upgrade aborts and the host stops applying managed configuration partway through. Observed on a live VPS immediately after taking v1.21.0:The fix
Normalize on read, so both return paths have the same shape and the existing direct subscripts stay valid:
Adding the key to the fresh-install default is not a migration — that branch is precisely the case a migration cannot affect. Defaulting on read is what makes a new manifest key backward compatible. The keys are defaulted as a set rather than one-off because the next key added will have exactly this problem again, and the function's contract is that its result is safe to subscript.
Note the file was already defensive in validation (
data.get("general_agent")on the line above) and then direct in use. Same split that caused this.Test
Writes the real v1.20.2 on-disk state — no
general_agentin the manifest, noagent.generalinopencode.json, and notask.generalin either, since v1.20.2 wrote none of them — then asserts the projection upgrades and adopts the native subagent.Baselined against the unfixed projector rather than assumed:
Full
tests/opencode-subagents.shpasses, exit 0.Note on how this was found
Caught by running the real nightly maintenance job against a live host after cutting v1.21.0, rather than by CI. CI only ever exercises the fresh-install branch, so no existing test could see it — which is the general shape worth noting: a new key in a persisted structure is an upgrade-path change, and the upgrade path is the branch tests do not take by default.