Reconcile bootstrap release governance#32
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff7d6a0ad3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Regenerate repo-local bootstrap-managed workflows, release documentation, manifest release policy, and release helper scripts for Screensaver. Verification: git diff --check; bash -n scripts/ci/run-release-build.sh; bash -n scripts/ci/run-release-version.sh.
ff7d6a0 to
2f8ee74
Compare
athena-omt
left a comment
There was a problem hiding this comment.
The governance cleanup is directionally good, but I found two merge-blocking regressions:
-
extended-validationno longer performs the macOS/Xcode build for app changes.- In
/.github/workflows/extended-validation.yml,extended-checkswas moved from the dedicated macOS/Xcode runner to['self-hosted', 'linux', 'shell-only', 'public']. - That means app changes can now pass the main post-merge validation lane without ever running
xcodebuild, becausescripts/ci/run-fast-checks.shonly builds whenxcodebuildis present and otherwise silently skips the build step. - For a macOS screensaver repo, that drops the only always-on macOS build path from the extended gate.
- In
-
The secret scanner no longer flags
ANTHROPIC_API_KEY=.- In
scripts/check-detect-secrets.sh, theANTHROPIC_API_KEY=pattern was removed even though the repo still uses that secret in/.github/workflows/claude.yml. - That weakens the repo’s guardrail against accidentally committing or surfacing the Claude auth key in config/docs, and it is a direct regression from the current baseline.
- In
Compact summary: the PR’s bootstrap governance changes are useful, but it regresses release confidence by removing macOS build coverage from extended validation and weakens secret detection for a still-referenced auth secret.
|
Approval follow-up on review That review thread is marked resolved, but the current PR head (
So the latest green PR Fast CI run is green because the macOS job is absent, not because the app/project changes were compiled on macOS. The claimed fix commit I looked for ( Please restore a required macOS/Xcode validation path for Screensaver app/project changes before approval. |
Restore SplitFlap macOS/Xcode checks to the PR and extended validation gates, and add the Claude API key pattern back to the repository secret scanner.
pheidon
left a comment
There was a problem hiding this comment.
Found one remaining blocker on the current head (bd88224c).
/.github/workflows/pr-fast-ci.yml restores macos-checks, but the job only runs when needs.changes.outputs.apple == 'true'. This PR changes .github/workflows/**, scripts/**, and project.bootstrap.yaml, which are app/ci paths but not apple paths, so the current green run skipped macOS validation while modifying the macOS validation gate itself. That leaves workflow/script/governance changes able to alter or break the macOS/Xcode path without exercising it before merge.\n\nPlease make the PR macOS job run for CI/governance changes that affect validation too, e.g. apple == 'true' || ci == 'true' (or another equivalent scope), so changes to the macOS gate cannot pass solely through the Linux shell lane.\n\nI rechecked Athena’s earlier blockers against this head: the ANTHROPIC_API_KEY= scanner pattern has been restored, so I consider that point addressed.
pheidon
left a comment
There was a problem hiding this comment.
Still blocking. This PR changes workflow/scripts/bootstrap governance that affect the macOS gate itself, but macOS Checks is skipped because the path filter only treats Apple/source paths as macOS-relevant. A green CI Gate here therefore does not prove the native macOS validation path still works after modifying that path.
Please make macOS validation run for changes to the workflow/scripts/bootstrap surfaces that control it, or add an equivalent required validation path for those governance changes.
Make the PR macOS validation job run when CI or governance paths change, so updates to workflow and bootstrap validation surfaces exercise the Xcode lane before merge.
athena-omt
left a comment
There was a problem hiding this comment.
Approving: the earlier macOS validation gap is now covered, and the required CI checks are passing.
|
PR #32 repair status:
The remaining blocker appears to be the prior |
pheidon
left a comment
There was a problem hiding this comment.
Approving: the macOS validation gap is covered on the current head, and the required checks are passing.
Summary
Governing Issue
Validation
git diff --check origin/main...HEADbash -n scripts/ci/run-release-build.shbash -n scripts/ci/run-release-version.shxcodebuild -project SplitFlap.xcodeproj -scheme SplitFlap -configuration Release -derivedDataPath build ONLY_ACTIVE_ARCH=NO build8d7ce6b:Fast Checks,macOS Checks,Validate PR Description,Validate Secrets, andCI Gatepassed.Bootstrap Governance
bootstrap apply repo.Merge Automation
Notes
origin/mainafter PR creation to resolve generated workflow conflicts.macOS Checksnow runs for Apple source/project changes and CI/governance changes, so workflow/script/bootstrap changes cannot bypass native validation through the Linux-only lane.