fix: stop the data.reference seed from wiping CoS settings on the #6182 update - #6236
Merged
Conversation
… update update.sh runs scripts/setup-data.js (which copies every data.reference file the install is missing) before scripts/run-migrations.js. The commit that added migration 339 — lift the durable CoS config out of data/cos/state.json into data/cos/config.json — also added a data.reference/cos/config.json seed. So on the single update that crossed that commit, setup-data wrote the seed first, 339 saw its output "already exists" and no-opped, loadConfig() preferred the seed over its legacy state.json read, and the next saveState() stripped the real config slice off disk permanently. The seed carried alwaysOn: false where DEFAULT_CONFIG carried true, so the visible symptom was CoS refusing to start after an update until started by hand, with concurrency caps and Persistent Mind grants back at defaults. Installs that updated before or after that commit are unaffected. - Delete the seed. data/cos/config.json is derived, not seeded — loadConfig() already returns DEFAULT_CONFIG when it is absent, so the defaults now have one source instead of two that drifted. - Add scripts/lib/migrationOwnedPaths.js, honored by both setup-data copy paths, with a test that fails if such a seed reappears. - Add migration 340: where config.json is still the shipped seed, redo 339's lift from state.json, or from the newest quarantined state.json.corrupted.* when the live slice is already gone, or delete it so the in-code defaults apply. A config.json carrying real settings is never touched — but if a quarantined backup holds settings it lacks, the migration names that file so a user who re-entered settings by hand can recover the rest. - Default DEFAULT_CONFIG.alwaysOn to false. It read true only because the seed masked it; with the seed gone, a fresh install would start CoS and its autonomous jobs unprompted, which "No cold-bootstrap LLM calls" forbids. - Record the rule in AGENTS.md: a migration that derives data/<x> from existing records ships no seed, and gates on the presence of its input rather than the absence of its output. Reordering update.sh to run migrations first was considered and rejected: run-migrations creates data/, which would send fresh installs down setup-data's existing-install branch and skip the __PORTOS_ROOT__ substitution, and setup.sh / setup.ps1 / npm run setup do not run migrations at all.
…d seed, fix doc drift The fallback reviewer found no live defect but four gaps, all fixed here: - docs/features/chief-of-staff.md documented alwaysOn's default as true; the seed deletion makes the in-code false the only value a fresh install reads. - Nothing pinned DEFAULT_CONFIG.alwaysOn = false — now the only thing keeping a never-configured install from auto-starting CoS and every work-generation flag that defaults on. The nearest existing assertion had also gone vacuous: it stored alwaysOn: false in a legacy slice and asserted false, which now passes even if the legacy fallback is deleted. Store true there instead, and add a test on the default itself. - Every migration-340 test wrote RETIRED_SEED as its own fixture, so the equality guard compared the constant to itself and would pass for any value it held — a mistranscribed byte would make the repair no-op on 100% of affected installs with CI green. Check in the seed extracted verbatim from ed19786 as an independent witness and assert the constant against it. - rm(configPath) without force: an ENOENT race would propagate out of runMigrations() and fail server boot, when "already gone" is what that branch wants anyway.
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.
Summary
CoS stopping after
update.sh, and its settings reverting to shipped defaults, are one bug.update.shrunsscripts/setup-data.js— which copies everydata.reference/file the install is missing — beforescripts/run-migrations.js. The commit that added migration 339 (lift the durable CoS config out ofdata/cos/state.jsonintodata/cos/config.json) also added adata.reference/cos/config.jsonseed. So on the one update that crossed that commit:data/cos/config.jsonloadConfig()preferred the seed file, so its legacystate.jsonread never firedsaveState()stripped the realconfigslice off disk permanentlyThe seed carried
alwaysOn: falsewhereDEFAULT_CONFIGcarriedtrue, which is why CoS also stopped auto-starting after each update. Installs that updated before or after that commit never see it — which matches a second machine on the same version being unaffected.Changes
data.reference/cos/config.json. It is a derived file, not a seed —loadConfig()already returnsDEFAULT_CONFIGwhen it is absent, so the defaults now have one source instead of two that had drifted.scripts/lib/migrationOwnedPaths.js— declares paths a migration derives from existing records; honored by both setup-data copy paths, with a test that fails if such a seed reappears.config.jsonis still the shipped seed, redo 339's lift fromstate.json, or from the newest quarantinedstate.json.corrupted.*when the live slice is already gone, or delete it so the in-code defaults apply. Aconfig.jsoncarrying real settings is never touched — but if a quarantined backup holds settings it lacks, the migration names that file so a user who re-entered settings by hand can recover the rest.DEFAULT_CONFIG.alwaysOnnow defaults tofalse. It readtrueonly because the seed masked it; with the seed gone, a fresh install would start CoS and its autonomous jobs unprompted, which "No cold-bootstrap LLM calls" forbids. Every existing install has a stored value and is unaffected.AGENTS.md— records the rule: a migration that derivesdata/<x>from existing records ships no seed, and gates on the presence of its input rather than the absence of its output.Rejected alternative
Reordering
update.shto run migrations first:run-migrationscreatesdata/, which would send fresh installs down setup-data's existing-install branch and skip the__PORTOS_ROOT__substitution.setup.sh/setup.ps1/npm run setupalso do not run migrations at all.Test plan
RETIRED_SEEDis asserted against the seed extracted verbatim from the commit that shipped it (scripts/migrations/__fixtures__/) — without that witness the equality guard compares the constant to itself and would pass for any value it held.alwaysOndefault pin were both bypass-probed (re-adding the seed / flipping the default each fails the suite).