fix(ci): guard apply workflow exit code output #655

Merged
pdurlej merged 1 commit from codex/207-apply-shell-exit-guard into main 2026-06-01 14:45:09 +02:00
Collaborator

Canary status: missing — fire canary 3+3 manually before merge

Summary

Hardens the manual apply workflow so early shell failures still produce a conservative exit_code for audit and evidence consumers.

Closes #207

Canary Context Pack

Product story

When apply fails, operators and agents need deterministic evidence. A missing steps.apply.outputs.exit_code should not create ambiguous audit output.

What changed

  • The Apply step now initializes overall=1 and registers a trap that writes exit_code on early EXIT/INT/TERM.
  • Normal successful apply paths still overwrite the output with the actual aggregate exit code.
  • Audit log finish normalizes missing or non-numeric exit_code to 1.
  • Workflow contract tests assert the guard and audit fallback.

Why it changed

Issue #207 identified that early shell failures could bypass the normal output write path and leave audit finish with null/empty exit code evidence.

Files touched

  • control-plane/forgejo-actions/apply.yaml
  • control-plane/platformctl/tests/test_forgejo_ci_scripts_contract.py

Relevant context

  • #207 SHELL-CRASH-DETECT-01
  • Existing apply workflow evidence/audit contract tests.

Runtime evidence

No production/runtime mutation. Local workflow contract tests and module validation only.

Known constraints

This does not change platformctl apply runtime semantics. It only hardens workflow evidence behavior around shell failure paths.

Explicit out-of-scope

  • No runtime apply.
  • No apply status artifact persistence changes.
  • No fsync/status permissions work.

Requested decision

Approve if early apply workflow failures now produce deterministic non-success evidence.

Merge blockers

  • Apply workflow can still emit empty/null exit_code for ordinary shell failure paths.
  • Audit log finish can still render invalid JSON when apply output is missing.

Spec sources read

  • https://git.pdurlej.com/pdurlej/platform/issues/207 — issue scope and acceptance criteria.
  • control-plane/forgejo-actions/apply.yaml — manual apply workflow.
  • control-plane/platformctl/tests/test_forgejo_ci_scripts_contract.py — workflow contract tests.

Validation

  • UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_forgejo_ci_scripts_contract.py -k 'apply_workflow' — 3 passed.
  • UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_forgejo_ci_scripts_contract.py platformctl/tests/test_pr_sanity.py — 70 passed.
  • PYTHONPATH=control-plane UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run --project control-plane python -m platformctl.cli validate all --json — exitCode 0.
Canary status: missing — fire canary 3+3 manually before merge ## Summary Hardens the manual `apply` workflow so early shell failures still produce a conservative `exit_code` for audit and evidence consumers. Closes #207 ## Canary Context Pack ### Product story When apply fails, operators and agents need deterministic evidence. A missing `steps.apply.outputs.exit_code` should not create ambiguous audit output. ### What changed - The `Apply` step now initializes `overall=1` and registers a trap that writes `exit_code` on early EXIT/INT/TERM. - Normal successful apply paths still overwrite the output with the actual aggregate exit code. - `Audit log finish` normalizes missing or non-numeric `exit_code` to `1`. - Workflow contract tests assert the guard and audit fallback. ### Why it changed Issue #207 identified that early shell failures could bypass the normal output write path and leave audit finish with null/empty exit code evidence. ### Files touched - `control-plane/forgejo-actions/apply.yaml` - `control-plane/platformctl/tests/test_forgejo_ci_scripts_contract.py` ### Relevant context - #207 SHELL-CRASH-DETECT-01 - Existing apply workflow evidence/audit contract tests. ### Runtime evidence No production/runtime mutation. Local workflow contract tests and module validation only. ### Known constraints This does not change `platformctl apply` runtime semantics. It only hardens workflow evidence behavior around shell failure paths. ### Explicit out-of-scope - No runtime apply. - No apply status artifact persistence changes. - No fsync/status permissions work. ### Requested decision Approve if early apply workflow failures now produce deterministic non-success evidence. ### Merge blockers - Apply workflow can still emit empty/null `exit_code` for ordinary shell failure paths. - Audit log finish can still render invalid JSON when apply output is missing. ## Spec sources read - `https://git.pdurlej.com/pdurlej/platform/issues/207` — issue scope and acceptance criteria. - `control-plane/forgejo-actions/apply.yaml` — manual apply workflow. - `control-plane/platformctl/tests/test_forgejo_ci_scripts_contract.py` — workflow contract tests. ## Validation - `UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_forgejo_ci_scripts_contract.py -k 'apply_workflow'` — 3 passed. - `UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run pytest platformctl/tests/test_forgejo_ci_scripts_contract.py platformctl/tests/test_pr_sanity.py` — 70 passed. - `PYTHONPATH=control-plane UV_CACHE_DIR=/private/tmp/codex-uv-cache uv run --project control-plane python -m platformctl.cli validate all --json` — exitCode 0.
fix(ci): guard apply workflow exit code output
All checks were successful
canary-required / collect-diff (pull_request) Successful in 6s
platformctl plan / auto-apply scope (pull_request) Successful in 28s
pyfallow / Pyfallow gate (control-plane) (pull_request) Successful in 29s
python-ci / Python 3.11 (pull_request) Successful in 51s
python-ci / Python 3.12 (pull_request) Successful in 54s
python-ci / Python 3.13 (pull_request) Successful in 54s
base-is-main / guard (pull_request) Successful in 2s
patchwarden-client-dry-run / collect-diff (pull_request) Successful in 6s
patchwarden-pr-sanity / collect-diff (pull_request) Successful in 7s
canary-required / canary (pull_request) Successful in 17s
patchwarden-client-dry-run / dry-run (pull_request) Successful in 27s
patchwarden-pr-sanity / sanity (pull_request) Successful in 7m13s
2dcc595a4d
Author
Collaborator

Patchwarden PR sanity

  • Status: advisory_findings
  • PR: 655
  • Commit: 2dcc595a4db9406b36377af84e0eef01459e487a
  • Security-sensitive label: present
  • Authority: advisory model review plus deterministic blockers only
  • 3+3 canary: still alive; this does not replace it

Deterministic findings

No deterministic findings.

Model reviewers

global-glm / glm-5.1:cloud

  • Status: error
  • Verdict: -
  • Note: ReadTimeout: The read operation timed out
  • Findings: none

global-deepseek / deepseek-v4-pro:cloud

  • Status: ok

  • Verdict: NOT_OK

  • medium overall reset to 0 before apply loop weakens early-exit guard

    • Evidence: control-plane/forgejo-actions/apply.yaml: after initializing overall=1 and setting the trap, the script resets overall=0 before the apply loop. If the workflow is interrupted (SIGINT/SIGTERM) after this reset but before any apply command ru
    • Next: Keep overall=1 until the apply loop completes, then set to 0 only if all apply commands exit with 0, or use a separate 'started' flag to prevent false zero exit codes on early termination.

redteam / kimi-k2.6:cloud

  • Status: ok

  • Verdict: NOT_OK

  • high Audit finish step skipped when apply fails

    • Evidence: control-plane/forgejo-actions/apply.yaml: The Apply step ends with exit $overall(line 175), and the immediately following Audit log finish step (line 180) has noif: always()orcontinue-on-error: trueon the Apply step. Whenoveral`
    • Next: Add if: always() to the Audit log finish step (and the Upload apply evidence artifacts step) so the audit trail and evidence artifacts are emitted even when the Apply step fails.

Policy notes

  • GLM 5.1 + DeepSeek V4 Pro are the operator-required model mix for this bot.
  • Optional red-team model is enabled only when PLATFORMCTL_PR_SANITY_REDTEAM_MODEL is configured.
  • Auto-merge is not enabled here.
<!-- patchwarden-pr-sanity:pdurlej/platform:PR-655 --> # Patchwarden PR sanity - Status: `advisory_findings` - PR: `655` - Commit: `2dcc595a4db9406b36377af84e0eef01459e487a` - Security-sensitive label: `present` - Authority: advisory model review plus deterministic blockers only - 3+3 canary: still alive; this does not replace it ## Deterministic findings No deterministic findings. ## Model reviewers ### `global-glm` / `glm-5.1:cloud` - Status: `error` - Verdict: `-` - Note: ReadTimeout: The read operation timed out - Findings: none ### `global-deepseek` / `deepseek-v4-pro:cloud` - Status: `ok` - Verdict: `NOT_OK` - **`medium`** overall reset to 0 before apply loop weakens early-exit guard - Evidence: `control-plane/forgejo-actions/apply.yaml: after initializing overall=1 and setting the trap, the script resets overall=0 before the apply loop. If the workflow is interrupted (SIGINT/SIGTERM) after this reset but before any apply command ru` - Next: Keep overall=1 until the apply loop completes, then set to 0 only if all apply commands exit with 0, or use a separate 'started' flag to prevent false zero exit codes on early termination. ### `redteam` / `kimi-k2.6:cloud` - Status: `ok` - Verdict: `NOT_OK` - **`high`** Audit finish step skipped when apply fails - Evidence: `control-plane/forgejo-actions/apply.yaml: The Apply step ends with `exit $overall` (line 175), and the immediately following Audit log finish step (line 180) has no `if: always()` or `continue-on-error: true` on the Apply step. When `overal` - Next: Add `if: always()` to the Audit log finish step (and the Upload apply evidence artifacts step) so the audit trail and evidence artifacts are emitted even when the Apply step fails. ## Policy notes - GLM 5.1 + DeepSeek V4 Pro are the operator-required model mix for this bot. - Optional red-team model is enabled only when `PLATFORMCTL_PR_SANITY_REDTEAM_MODEL` is configured. - Auto-merge is not enabled here.
pdurlej deleted branch codex/207-apply-shell-exit-guard 2026-06-01 14:45:10 +02:00
Sign in to join this conversation.
No reviewers
No labels
W6d-automerge-calibration
agent/claude-code
agent/codex
agent/hermes
agent/iskra
agent/ollama
agent/patchwarden
automerge-candidate
class/security-sensitive
cutover-gate
dependency/blocked
dependency/blocks-others
dependency/cross-repo
dependency/needs-confirmation
domain:agents
domain:ci
domain:docs
domain:forgejo
domain:infra
domain:memory
domain:runtime
domain:signal
domain:ux
flow/architecture
flow/blocked
flow/deployed
flow/done
flow/implementation
flow/intake
flow/maintained
flow/observed
flow/ready
flow/refining
flow/retired
flow/review
iterating
judge/codex-candidate
judge/hermes-candidate
judge/low-confidence
judge/needs-refinement
judge/operator-needed
judge/p0
judge/p1
judge/p2
judge/p3
judge/park
judge/patchwarden-candidate
judge/stale-priority
kind/adr
kind/bug
kind/chore
kind/feature
kind/infra
kind/ops
kind/refactor
kind/research
large-impact
merge/auto
merge/manual
merge/manual-dependency-conflict
merge/manual-failing-tests
merge/manual-merge-conflict
merge/manual-missing-review
merge/manual-operator-preference
merge/manual-red-zone
merge/manual-security-sensitive
merge/manual-unclear-scope
merge/manual-unknown
meta
mode:operator-only
mode:patchwarden-iskra-approved
mode:safe-auto
needs-operator-decision
needs-triage
not-ready
observed/erroring
observed/needs-followup
observed/pending
observed/retire-candidate
observed/unused
observed/used
operator-emotional
owner-attention
phase/02
phase/03
priority:p0
priority:p1
priority:p2
priority:p3
proposed
ready-for-agent
ready-for-operator
recovery
review:claude-reviewed
review:codex-reviewed
review:dziadek-reviewed
review:needs-human
risk/exposure
risk/process
risk/product
risk/runtime
safety:external-write
safety:no-prod-mutation
safety:prod-impact
safety:secret-touch
size/large
size/medium
size/small
size/tiny
size/unknown
source/adr
source/agent-generated
source/manual
source/operator-chat
source/voice-note
status:blocked
status:codex-ready
status:merged:pending-evidence
status:needs-evidence
status:operator-needed
status:parked
tier/full
tier/lite
tier/stacked
tier:0-platform-substrate
tier:1-iskra-value-layer
tier:2-tools-products-modules
type:bug
type:chore
type:docs
type:feat
type:policy
type:research
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
pdurlej/platform!655
No description provided.