Repository navigation
fix(storage): keep the scenario manifest key deployments registered under - #469
Closed
BobbyZhouZijian wants to merge 1 commit into
Closed
BobbyZhouZijian wants to merge 1 commit into
BobbyZhouZijian wants to merge 1 commit into
Conversation
…nder The storage simplification (#412) renamed the artifact manifest key that carries a scenario's registration from `scenario_snapshot` to `scenario_commit_record`. The key is written into `reef-artifact.json` on the scenario ref of every deployment, so after an upgrade the factory finds no registration, the fork it retries is a no-op because the ref exists, and every existing scenario fails to load with "scenario backend did not persist registration metadata". Restore the persisted key and pin it in the tests that read the manifest. The Python name `SCENARIO_METADATA_KEY` and the `CommitRecord` vocabulary stay as #412 left them; only the on-disk value returns to what registered deployments hold. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Merge Oncall: @BobbyZhouZijian The merge oncall will route reviews and keep this pull request moving toward a clear decision. |
Contributor
Author
|
Deployment check on the EC2 harness-evolve box (reef
🤖 Generated with Claude Code |
Contributor
Author
|
Closing: the maintainer wants the key to stay |
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.
Motivation
#412 renamed the artifact manifest key that carries a scenario's registration from
scenario_snapshottoscenario_commit_record. That key is written intoreef-artifact.jsonon the scenario ref of every deployment, so a deployment upgraded past #412 finds no registration for any of its scenarios: the factory retriesfork(), which is a no-op because the ref already exists, and every scenario fails to load withObserved on the harness-evolve deployment behind api.reefinfra.ai after moving from #353 to #432: all six scenarios listed
loaded: falseand every harness route answered 404.Changes
reef/storage/commits.py:SCENARIO_METADATA_KEYisscenario_snapshotagain, with a comment saying the value is the on-disk contract and does not follow code renames. The Python name and theCommitRecordvocabulary from refactor: simplify scenario storage and remove package dependency cycles #412 stay.tests/reef_service/test_commit_log.py,tests/reef_service/test_reef_git_lfs.py: the tests that read the manifest pin the literal key so a future rename is caught.docs/advanced_topics/state-model.rst: names the persisted key.Compatibility and operational impact
Persisted data: restores the manifest key every deployment registered before #412 holds. A scenario registered by a build between #412 (2026-09-12) and this fix has
scenario_commit_recordin its manifest and would need to be re-registered; no such deployment is known besides local development runs. No API, wire, or configuration change.Verification
Deployed this branch on the EC2 harness-evolve deployment (reef main #432 plus this commit): the six pre-#412 scenarios load again after restart. Results are in the PR discussion once the restart is confirmed.
AI assistance
Claude Code (Claude Fable 5.1) diagnosed the failure on the deployment, wrote the change, the test updates and this description. The author reviewed every line and ran the checks above.
Checklist
README.i18n.yaml, or do not affect README content.🤖 Generated with Claude Code