ci: notarize Commander Developer ID releases - #400
Conversation
|
|
|
|
|
🤝 Merged No AI resolution was needed by merge time; the branch was updated with a plain merge commit.
Please review the merge commit before relying on it. |
# Conflicts: # graphify-out/GRAPH_REPORT.md # graphify-out/graph.json # graphify-out/manifest.json
Structural `graphify update` completed (graphify 0.9.4); LLM semantic extraction failed, so the verified structural graph was preserved. Refreshed by the resolve-pr-conflicts workflow: https://github.com/lopugit/thingtime/actions/runs/32833404228
Lopu repository reviewLopu reviewed this PR against github-actions as Thingtime's principal PR and repository manager. Using Claude Opus 5. Lopu made justified improvements and pushed f472d9c to codex/commander-release-developer-id. PR #400 —
|
| Check | Result |
|---|---|
python3 yaml.safe_load of commander-release.yml |
parses; 13 steps |
ruby -ryaml load of the same file |
parses |
Step-gate audit (printed every step's if:) |
6 setup steps ungated, 6 build/publish steps share exists != 'true', teardown always() |
| Import↔teardown path cross-check (parsed both step bodies, compared literals) | all 3 artifacts covered: keychain, .p12, .p8 via AuthKey_*.p8 |
bash -n on the teardown body |
ok |
Teardown simulated with no keychain/files present (the exists == 'true' path) |
exit 0 — always() cannot redden a skip-run |
git diff --check |
clean |
Full verify job — node --check ×13 .mjs, bash -n ×3 .sh, git diff --check, build-all-branch --self-test, classify-claude-credential-failure --self-test, electron-pr-release-contract |
all pass |
| All 10 deterministic contract advisories (develop-preview, workflow control-plane, signed Electron PR release, conflict-resolver routing, rebase ownership routing, promotion-worker routing, promotion-worker behavior, promotion changelog, feature promoter, all-branch builder) | 10/10 OK, 0 failures |
graphify-out/ was not regenerated: the binary is not installed on this runner. Being
precise about the cost — the graph indexes this workflow at step granularity for its
notable steps (github_workflows_commander_release, ..._release,
..._import_credentials, ..._notarize), so unlike a pure reordering, my added step
means the committed graph is short one node until refresh. That is re-stamped by the
repo's automated post-merge refresh, which has already produced four such commits on
this branch (most recently 1e029523). Running a differently versioned graphify here
would have rewritten the whole ~21k-line pair for one node.
Assessment
The design is sound and the contract with #263 genuinely holds — I verified it rather
than taking the description's word for it. One real gap closed. The remaining blockers
are sequencing decisions for the owner, not defects in this diff.
|
🤝 Merged No AI resolution was needed by merge time; the branch was updated with a plain merge commit.
Please review the merge commit before relying on it. |
# Conflicts: # graphify-out/manifest.json
Structural `graphify update` completed (graphify 0.9.4); LLM semantic extraction failed, so the verified structural graph was preserved. Refreshed by the resolve-pr-conflicts workflow: https://github.com/lopugit/thingtime/actions/runs/32858961684
|
🤝 Merged No AI resolution was needed by merge time; the branch was updated with a plain merge commit.
Please review the merge commit before relying on it. |
# Conflicts: # graphify-out/GRAPH_REPORT.md # graphify-out/graph.json # graphify-out/manifest.json
Structural `graphify update` followed by LLM semantic `graphify extract` (graphify 0.9.4, claude-cli backend); unchanged content served from the tracked semantic cache. Refreshed by the resolve-pr-conflicts workflow: https://github.com/lopugit/thingtime/actions/runs/32899518951
|
🤝 Merged No AI resolution was needed by merge time; the branch was updated with a plain merge commit.
Please review the merge commit before relying on it. |
# Conflicts: # graphify-out/GRAPH_REPORT.md # graphify-out/graph.json # graphify-out/manifest.json
Structural `graphify update` followed by LLM semantic `graphify extract` (graphify 0.9.4, claude-cli backend); unchanged content served from the tracked semantic cache. Refreshed by the resolve-pr-conflicts workflow: https://github.com/lopugit/thingtime/actions/runs/32946000595
|
Lopu — reviewer note (Thingtime PR manager). Full report lands in my review comment; here are the two things that need your call, and what I pushed to the branch. The red check is not yours
#395 already analysed this exact false-red and deliberately left it alone (a per-PR group would license concurrent force-pushes of the single generated Landing order — needs your decisionThis lane cannot run yet, for two independent reasons:
Do you want the What I changed on this branch
The rest verified clean: every variable this workflow exports matches #263's |
|
🤝 Merged No AI resolution was needed by merge time; the branch was updated with a plain merge commit.
Please review the merge commit before relying on it. |
# Conflicts: # graphify-out/GRAPH_REPORT.md # graphify-out/graph.json # graphify-out/manifest.json
Structural `graphify update` followed by LLM semantic `graphify extract` (graphify 0.9.4, claude-cli backend); unchanged content served from the tracked semantic cache. Refreshed by the resolve-pr-conflicts workflow: https://github.com/lopugit/thingtime/actions/runs/32980937883
|
🤝 Merged No AI resolution was needed by merge time; the branch was updated with a plain merge commit.
Please review the merge commit before relying on it. |
# Conflicts: # graphify-out/GRAPH_REPORT.md # graphify-out/graph.json # graphify-out/manifest.json
Structural `graphify update` followed by LLM semantic `graphify extract` (graphify 0.9.4, claude-cli backend); unchanged content served from the tracked semantic cache. Refreshed by the resolve-pr-conflicts workflow: https://github.com/lopugit/thingtime/actions/runs/33017064997
|
Lopu — reviewer note (Thingtime PR manager). Re-reviewed The check I called a false-red is now green
What I changed: the missing half of the credential contractThe import step in this PR mirrors The reason this is worth fixing here rather than shrugging off as ephemeral-runner hygiene is that Commander's exposure window is strictly larger than the lane it copied:
So the lane with the bigger window was the one missing the mitigation. Restructuring the script to test-before-sign belongs to #263, not to a control-plane branch — the teardown is the in-scope fix. I did not touch the keychain search-list handling or the exported-but-unused Verified rather than assumed: the #263 contract holdsEvery variable this workflow exports lines up with Still needs your call — landing orderUnchanged from my last note, and still the only thing standing between this and a working lane:
Do you want the ValidationYAML parses under both Python and Ruby; step-gate audit clean across all 13 steps; the full control-plane One honesty note on |
Summary
Validation
git diff --checkDependency
Requires the Commander signing-policy change in #263 so the build script enforces the same release boundary.