Repository navigation
fix(upgrade): reject stale current playbook - #541
Conversation
taco3064
left a comment
taco3064
left a comment
There was a problem hiding this comment.
CHANGES_REQUIRED
Reviewed exact head 7719584269ef58c1de561e2a3885db100dab1813 against #540.
Candidate identity and mechanical verification are clean:
- head tree:
e078850a860f8f59041d823ef211673562c42a12 - PR body records independent Accept: ACCEPTED for that exact tree
- exact-head CI #897: success
- Windows / Ubuntu / Node 18 / ESLint 10 / operational / docs / topology replay: success
- changed-production mutation shard + aggregate: success
- PR verification: success
The direct R5 reproducer is fixed correctly: a fully settled current lifecycle with stale blueprint-upgrade.md now fails, names the artifact, preserves both playbook and lifecycle, and the same behavior is covered under --dry-run.
I found one remaining decision-fidelity blocker.
A current lifecycle with install drift still bypasses the stale-playbook refusal and can later delete that playbook automatically
#540's product decision is keyed by the durable lifecycle already being current while a stale blueprint-upgrade.md exists. In that contradictory state Blueprint must preserve the playbook, report it, and require manual review/removal. The ticket explicitly says not to auto-delete or reinterpret it as recoverable semantic work.
The current implementation narrows the refusal further:
if (input.facts.checkpoint.kind === 'state' && settled
&& input.facts.upgradePlaybook !== null) {
return refuse(...)
}So the stale artifact is refused only when every installed application already matches the running target.
The new decision test intentionally codifies the opposite behavior for install drift:
current lifecycle
+ stale blueprint-upgrade.md
+ installed 4.0.0 or missing
→ proceed(mode: start)
That is not just an untested edge. Following the existing upgrade flow, this path records an empty pending repair, repairs the install, then completion calls recordCompletion(); its default retire is removePlaybook(), which does:
fs.rmSync(path.join(root, UPGRADE_PLAYBOOK_FILE), { force: true });So a stale playbook that #540 says is contradictory/user-review-required can be silently consumed by an install-repair path merely because the installed package drifted from the already-current lifecycle.
Please keep install repair behavior, but make stale-playbook ownership authoritative whenever the durable lifecycle itself is current. In other words:
lifecycle says current
+ stale blueprint-upgrade.md
→ preserve + explicit non-zero refusal
regardless of whether the local installed package currently needs repair
The operator can remove/review the stale artifact first, then rerun upgrade and let the existing install-repair path do its normal work.
Add a focused control for at least:
- current lifecycle + stale playbook + older installed package;
- current lifecycle + stale playbook + missing installed package;
proving no lifecycle/install mutation runs and the playbook remains byte-for-byte unchanged.
I do not currently see another #540 requirement-fidelity blocker on this head.
taco3064
left a comment
taco3064
left a comment
There was a problem hiding this comment.
CHANGES_REQUIRED
Re-reviewed exact head 2d1e55be03c407220954899a64a3a73d30702c5d against #540.
The previous install-drift blocker is fixed correctly. Current lifecycle + stale blueprint-upgrade.md now refuses before older/missing package repair, preserves lifecycle/playbook/install state, and same-target pending !== null still resumes normally.
I found one remaining requirement-fidelity blocker.
The new refusal priority changes existing authoring / transformation workflow refusal behavior
#540 explicitly requires:
Existing authoring/transformation workflow refusal behavior remains unchanged.
The current decision order is:
catalogRefusal(...)
?? stalePlaybookRefusal(...)
?? readinessRefusal(...)
?? checkpointRefusal(...)
?? historyRefusal(...)But readinessRefusal() is where existing facts.workflows becomes:
{ kind: 'pending-workflow', files: facts.workflows }So this repository state:
lifecycle = 4.1.0
pending = null
blueprint-upgrade.md exists
blueprint-authoring.md or blueprint-transformation.json also exists
now returns stale-upgrade-playbook first instead of the existing pending-workflow refusal.
That changes a pre-existing workflow safety surface even though #540 deliberately says not to broaden or replace it. The stale playbook should outrank config/install readiness so the missing/older-package case stays fixed, but it should not steal authority from an actual active authoring/transformation workflow.
Please preserve the existing workflow refusal priority while keeping the repaired install-drift behavior. A narrow shape would be conceptually:
active authoring / transformation workflow
→ existing pending-workflow refusal
otherwise, current lifecycle + stale upgrade playbook
→ stale-upgrade-playbook refusal
then config/install readiness
→ existing behavior
Add a focused control proving that current lifecycle + stale blueprint-upgrade.md + an active authoring/transformation workflow still returns the existing pending-workflow refusal and preserves all files.
The exact-head CI #900 is still running at review time; all completed jobs, including the mutation aggregate, are green so far. This blocker is independent of CI status.
I do not currently see another #540 requirement-fidelity blocker on this head.
taco3064
left a comment
taco3064
left a comment
There was a problem hiding this comment.
CHANGES_REQUIRED
只看程式，不看 CI。
上一個 blocker 已修掉：current lifecycle + stale playbook + active authoring/transformation workflow 現在會回既有的 pending-workflow，而 stale playbook 不會搶掉它。
但這次把 workflowRefusal() 整體拉到 readinessRefusal() 前面，造成一個新的 collateral behavior change。
原本 #540 之前的順序在同一個 readinessRefusal() 裡是：
config-unreadable
→ not-adopted
→ pending-workflow
現在變成：
catalogRefusal(...)
?? workflowRefusal(facts)
?? stalePlaybookRefusal(...)
?? readinessRefusal(input)因此即使 沒有 stale playbook，只要 repo 同時有 active workflow：
- config unreadable + workflow：以前回
config-unreadable，現在變成pending-workflow - no adopted application + workflow：以前回
not-adopted，現在也變成pending-workflow
這是 #540 之外的既有 refusal precedence 改動，而且 ticket 明確要求 existing authoring/transformation workflow refusal behavior 不要被改。
不要再把 workflow refusal 全域升權。比較窄的做法是：
如果有 stale-current residue：
有 active workflow
→ 仍沿用舊 readiness precedence 決定是否真的該回 pending-workflow
沒有 active workflow
→ stale-upgrade-playbook 可優先於 config/install readiness
如果沒有 stale-current residue：
→ 完整維持原本 readinessRefusal() 行為與順序
也就是修 #540 的 precedence 應該只影響 stale-current 這個新分支，不該重排所有 upgrade refusal。
請補至少兩個 regression controls：
- no stale playbook + config unreadable + active workflow → 仍是
config-unreadable - no stale playbook + no adopted application + active workflow → 仍是
not-adopted
目前除此之外，我沒有看到新的 #540 blocker。
taco3064
left a comment
taco3064
left a comment
There was a problem hiding this comment.
CHANGES_REQUIRED
只看程式，不看 CI。
上一輪要求的「不要全域重排 refusal priority」已經修回來了：沒有 stale-current residue 時，readinessRefusal() 仍維持原本：
config-unreadable
→ not-adopted
→ pending-workflow
但 stale-current + active workflow 這個分支還差最後一個 precedence 邊界。
現在 stalePlaybookRefusal() 直接做：
return state.blueprint === target && state.pending === null
&& facts.upgradePlaybook !== null
? facts.workflows.length
? refuse({ kind: 'pending-workflow', files: facts.workflows })
: refuse({ kind: 'stale-upgrade-playbook', file: facts.upgradePlaybook })
: null;而它仍然在 readinessRefusal() 前面。
所以當 stale-current residue 與 active workflow 同時存在時：
- config unreadable + workflow + stale playbook → 現在回
pending-workflow，舊 precedence 應該仍是config-unreadable - no adopted application + workflow + stale playbook → 現在回
pending-workflow，舊 precedence 應該仍是not-adopted
這正是上一輪 review 裡「有 active workflow 時，仍沿用舊 readiness precedence 決定是否真的該回 pending-workflow」尚未完全落地的部分。
目前新增的 runtime control只覆蓋：
current + stale + active workflow
+ config readable
+ adopted
→ pending-workflow
這條是對的，但還不足以證明既有 workflow refusal behavior 完整保持。
建議把判斷收成：
stale-current + active workflow
→ 先走原本 readiness precedence
config-unreadable / not-adopted / pending-workflow
stale-current + no active workflow
→ stale-upgrade-playbook
no stale-current
→ 原本 readinessRefusal() 完全不變
請補兩個 focused controls：
- stale-current + config unreadable + active workflow →
config-unreadable - stale-current + no adopted application + active workflow →
not-adopted
除此之外，我目前沒有看到新的 #540 blocker。
taco3064
left a comment
taco3064
left a comment
There was a problem hiding this comment.
APPROVED
只看程式，不看 CI。
這一版把前幾輪的 precedence 問題收乾淨了。
目前 preflightRefusal() 的行為是：
stale-current + no active workflow
→ stale-upgrade-playbook
stale-current + active workflow
→ 回到原本 readinessRefusal()
config-unreadable
→ not-adopted
→ pending-workflow
no stale-current
→ 完整沿用原本 readinessRefusal()
這符合 #540 的窄範圍要求，也沒有再全域重排既有 upgrade refusal。
我重新檢查了幾個前面出過問題的邊界：
- current + stale + installed current → 明確拒絕 stale playbook
- current + stale + installed older / missing → stale refusal 先於 install repair，不會偷偷消耗 playbook
- current + stale + active workflow → 保留既有 workflow safety surface
- current + stale + active workflow + config unreadable →
config-unreadable - current + stale + active workflow + no adopted app →
not-adopted - no stale + active workflow → 原本 readiness precedence 不變
- same-target
pending !== null+ playbook → 正常 resume，不被誤判 stale - older lifecycle + playbook → 正常進 upgrade plan，不把合法/既有 playbook 誤判成 current residue
- bootstrap / pre-lifecycle state + playbook → 不會套用 #540 的 stale-current refusal
stalePlaybookRefusal() 也只在：
state present
+ state.blueprint === running target
+ pending === null
+ blueprint-upgrade.md exists
時成立，邊界夠窄，沒有碰 #532 completion ordering，也沒有自動刪除或重寫 playbook。
目前從程式與 #540 decision fidelity 來看，我沒有再看到 blocker。
Shaper 結論：APPROVED。
1 participant
Closes #540.
What changed
blueprint-upgrade.mdas a dedicated upgrade factpending: null, preserve and explicitly refuse contradictory stale residueFinal precedence model
The current head uses one preflight rule instead of globally reordering refusal functions:
stale-upgrade-playbookconfig-unreadable→not-adopted→pending-workflowThis keeps stale residue ahead of config/install repair only where #540 requires it, while preserving the complete pre-existing workflow safety surface.
Evidence
Independent Acceptance
ACCEPTED0fe648fd988361da0c08077f761ceebec1a97f4200ca6198d8ca92a98b1135ee620a8dca83b774402a1b3ebc88a2424039893abbcca05658a5dd3b9cPR CI and changed-code mutation remain authoritative for this exact head.