ci(patchwarden): soft-fail advisory review failures #789

Merged
pdurlej merged 4 commits from codex/orders/patchwarden-review-run-softfail into main 2026-06-11 08:10:48 +02:00
Collaborator

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

Canary Context Pack

Product story

Patchwarden client dry-run is supposed to be an operator signal, not a fragile model availability check. The deterministic contract/pr-check artifacts should remain available even when the advisory model lane times out, returns malformed JSON, or a status/comment publication attempt fails technically.

What changed

  • patchwarden contract-pr --execute now retries once without publication if the execute path exits as a technical failure, preserving dry-run artifacts.
  • patchwarden review-run and patchwarden resolve-findings now fail soft: they write patchwarden-client-review-error.json and keep deterministic artifacts authoritative.
  • The workflow uploads the new review-error artifact.
  • Added workflow contract tests covering publication retry and advisory review soft-fail behavior.

Why it changed

Current PRs such as #783, #784, #786, #787, and #788 show red patchwarden-client-dry-run / dry-run despite deterministic gates being fresh/clean or manual-class as expected. Local reproduction showed review-run can fail on Ollama timeout / invalid JSON, and manual-class PRs may lose artifacts if contract publication fails technically.

Files touched

  • .forgejo/workflows/patchwarden-client-dry-run.yml
  • control-plane/platformctl/tests/test_pr_sanity.py

Relevant context

  • .forgejo/workflows/patchwarden-client-dry-run.yml is the external Patchwarden client dogfood lane.
  • Automerge readiness still requires the patchwarden-client-dry-run / dry-run context for narrow W6d candidates.
  • Model review remains advisory; deterministic contract-pr and pr-check remain the merge-safety evidence.

Runtime evidence

No runtime mutation. Local simulations only:

  • contract-pr --execute technical failure retries as dry-run and exits 0 with contract/pr-check artifacts.
  • review-run invalid JSON exits 0 with patchwarden-client-review-error.json.

Known constraints

This does not make unsafe classes eligible for automerge. control-plane/, modules/, workflow, runtime, and governance paths remain manual/security-sensitive per existing policy.

Explicit out-of-scope

  • No Patchwarden source change.
  • No provider credential or secret change.
  • No branch protection change.
  • No runtime deployment or runner mutation.

Requested decision

Approve the workflow resilience change so existing PRs can be retriggered against deterministic artifacts instead of being blocked by advisory reviewer transport/format failures.

Merge blockers

  • Any path where this causes unsafe PR classes to become automerge-eligible.
  • Any loss of deterministic contract-pr or pr-check artifacts.
  • Any secret/token exposure in logs or artifacts.

Spec sources read

  • .forgejo/workflows/patchwarden-client-dry-run.yml — failing workflow path.
  • control-plane/platformctl/tests/test_pr_sanity.py — workflow contract tests.
  • control-plane/platformctl/tests/test_automerge_readiness.py — required context behavior for automerge readiness.
  • .forgejo/workflows/automerge-pilot.yml — confirmed dry-run context remains required.
  • /Users/pd/Developer/patchwarden/src/patchwarden/contract_pipeline.py — local diagnostic only, to understand contract-pr exit behavior.
  • /Users/pd/Developer/patchwarden/src/patchwarden/contract_publish.py — local diagnostic only, to understand publication exit behavior.

No closing keyword; this is a queue-unblocking CI hardening fix.

Canary status: missing — fire canary 3+3 manually before merge ## Canary Context Pack ### Product story Patchwarden client dry-run is supposed to be an operator signal, not a fragile model availability check. The deterministic contract/pr-check artifacts should remain available even when the advisory model lane times out, returns malformed JSON, or a status/comment publication attempt fails technically. ### What changed - `patchwarden contract-pr --execute` now retries once without publication if the execute path exits as a technical failure, preserving dry-run artifacts. - `patchwarden review-run` and `patchwarden resolve-findings` now fail soft: they write `patchwarden-client-review-error.json` and keep deterministic artifacts authoritative. - The workflow uploads the new review-error artifact. - Added workflow contract tests covering publication retry and advisory review soft-fail behavior. ### Why it changed Current PRs such as #783, #784, #786, #787, and #788 show red `patchwarden-client-dry-run / dry-run` despite deterministic gates being fresh/clean or manual-class as expected. Local reproduction showed `review-run` can fail on Ollama timeout / invalid JSON, and manual-class PRs may lose artifacts if contract publication fails technically. ### Files touched - `.forgejo/workflows/patchwarden-client-dry-run.yml` - `control-plane/platformctl/tests/test_pr_sanity.py` ### Relevant context - `.forgejo/workflows/patchwarden-client-dry-run.yml` is the external Patchwarden client dogfood lane. - Automerge readiness still requires the `patchwarden-client-dry-run / dry-run` context for narrow W6d candidates. - Model review remains advisory; deterministic `contract-pr` and `pr-check` remain the merge-safety evidence. ### Runtime evidence No runtime mutation. Local simulations only: - `contract-pr --execute` technical failure retries as dry-run and exits `0` with contract/pr-check artifacts. - `review-run` invalid JSON exits `0` with `patchwarden-client-review-error.json`. ### Known constraints This does not make unsafe classes eligible for automerge. `control-plane/`, `modules/`, workflow, runtime, and governance paths remain manual/security-sensitive per existing policy. ### Explicit out-of-scope - No Patchwarden source change. - No provider credential or secret change. - No branch protection change. - No runtime deployment or runner mutation. ### Requested decision Approve the workflow resilience change so existing PRs can be retriggered against deterministic artifacts instead of being blocked by advisory reviewer transport/format failures. ### Merge blockers - Any path where this causes unsafe PR classes to become automerge-eligible. - Any loss of deterministic `contract-pr` or `pr-check` artifacts. - Any secret/token exposure in logs or artifacts. ## Spec sources read - `.forgejo/workflows/patchwarden-client-dry-run.yml` — failing workflow path. - `control-plane/platformctl/tests/test_pr_sanity.py` — workflow contract tests. - `control-plane/platformctl/tests/test_automerge_readiness.py` — required context behavior for automerge readiness. - `.forgejo/workflows/automerge-pilot.yml` — confirmed dry-run context remains required. - `/Users/pd/Developer/patchwarden/src/patchwarden/contract_pipeline.py` — local diagnostic only, to understand `contract-pr` exit behavior. - `/Users/pd/Developer/patchwarden/src/patchwarden/contract_publish.py` — local diagnostic only, to understand publication exit behavior. No closing keyword; this is a queue-unblocking CI hardening fix.
ci(patchwarden): retry contract publication as dry-run
All checks were successful
canary-required / collect-diff (pull_request) Successful in 4s
infra-docs-drift / docs-drift (pull_request) Successful in 5s
platformctl plan / auto-apply scope (pull_request) Successful in 21s
pyfallow / Pyfallow gate (control-plane) (pull_request) Successful in 18s
python-ci / Python 3.11 (pull_request) Successful in 40s
python-ci / Python 3.12 (pull_request) Successful in 42s
python-ci / Python 3.13 (pull_request) Successful in 43s
workflow-lint / lint (pull_request) Successful in 5s
base-is-main / guard (pull_request) Successful in 2s
canary-required / canary (pull_request) Successful in 13s
patchwarden-pr-sanity / collect-diff (pull_request) Successful in 4s
patchwarden-client-dry-run / dry-run (pull_request) Successful in 17s
patchwarden-client-dry-run / collect-diff (pull_request) Successful in 4s
patchwarden-pr-sanity / sanity (pull_request) Successful in 4m29s
01e31e1a5a
First-time contributor

Patchwarden PR sanity

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

Deterministic findings

  • info sensitive-path-touched Sensitive path touched — .forgejo/workflows/patchwarden-client-dry-run.yml
    • Evidence: .forgejo/workflows/patchwarden-client-dry-run.yml
    • Next: Route through the existing 3+3/risk-tier process; model review remains advisory.

Model reviewers

global-glm / glm-5.1:cloud

  • Status: ok

  • Verdict: OK

  • medium Token environment variable persists during contract-pr retry, may undermine dry-run intent

    • Evidence: In .forgejo/workflows/patchwarden-client-dry-run.yml lines 199-205: export "$PATCHWARDEN_TOKEN_ENV=$PATCHWARDEN_COMMENT_TOKEN"is set before the first run, but the retry at line 207 callsrun_patchwarden_contract_pr without unsetting th
    • Next: Unset the token environment variable before the retry to guarantee dry-run mode: add unset "$PATCHWARDEN_TOKEN_ENV" before the run_patchwarden_contract_pr retry call at line 208.

global-deepseek / deepseek-v4-pro:cloud

  • Status: ok
  • Verdict: OK
  • Findings: none

redteam / kimi-k2.6:cloud

  • Status: ok

  • Verdict: NOT_OK

  • high Ambient secret export enlarges token exposure surface

    • Evidence: .forgejo/workflows/patchwarden-client-dry-run.yml changed from scoped env "$PATCHWARDEN_TOKEN_ENV=$PATCHWARDEN_COMMENT_TOKEN" patchwarden contract-pr ...toexport "$PATCHWARDEN_TOKEN_ENV=$PATCHWARDEN_COMMENT_TOKEN" followed by multiple
    • Next: Wrap the export and contract-pr call in a subshell ( export "$PATCHWARDEN_TOKEN_ENV=$PATCHWARDEN_COMMENT_TOKEN"; run_patchwarden_contract_pr "$@" ) so the token is scoped only to the invocation that needs it, or revert to the env pattern by calling patchwarden directly rather than via a shell fu

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-789 --> # Patchwarden PR sanity - Status: `advisory_findings` - PR: `789` - Commit: `3db949dd304bd274d27a838c6a7daf0e420895e0` - Security-sensitive label: `present` - Authority: advisory model review plus deterministic blockers only - 3+3 canary: still alive; this does not replace it ## Deterministic findings - **`info` `sensitive-path-touched`** Sensitive path touched — `.forgejo/workflows/patchwarden-client-dry-run.yml` - Evidence: `.forgejo/workflows/patchwarden-client-dry-run.yml` - Next: Route through the existing 3+3/risk-tier process; model review remains advisory. ## Model reviewers ### `global-glm` / `glm-5.1:cloud` - Status: `ok` - Verdict: `OK` - **`medium`** Token environment variable persists during contract-pr retry, may undermine dry-run intent - Evidence: `In .forgejo/workflows/patchwarden-client-dry-run.yml lines 199-205: `export "$PATCHWARDEN_TOKEN_ENV=$PATCHWARDEN_COMMENT_TOKEN"` is set before the first run, but the retry at line 207 calls `run_patchwarden_contract_pr` without unsetting th` - Next: Unset the token environment variable before the retry to guarantee dry-run mode: add `unset "$PATCHWARDEN_TOKEN_ENV"` before the `run_patchwarden_contract_pr` retry call at line 208. ### `global-deepseek` / `deepseek-v4-pro:cloud` - Status: `ok` - Verdict: `OK` - Findings: none ### `redteam` / `kimi-k2.6:cloud` - Status: `ok` - Verdict: `NOT_OK` - **`high`** Ambient secret export enlarges token exposure surface - Evidence: `.forgejo/workflows/patchwarden-client-dry-run.yml changed from scoped `env "$PATCHWARDEN_TOKEN_ENV=$PATCHWARDEN_COMMENT_TOKEN" patchwarden contract-pr ...` to `export "$PATCHWARDEN_TOKEN_ENV=$PATCHWARDEN_COMMENT_TOKEN"` followed by multiple` - Next: Wrap the export and contract-pr call in a subshell `( export "$PATCHWARDEN_TOKEN_ENV=$PATCHWARDEN_COMMENT_TOKEN"; run_patchwarden_contract_pr "$@" )` so the token is scoped only to the invocation that needs it, or revert to the `env` pattern by calling patchwarden directly rather than via a shell fu ## 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.
chore(ci): trigger patchwarden contract-pr smoke
Some checks failed
base-is-main / guard (pull_request) Successful in 1s
canary-required / collect-diff (pull_request) Successful in 4s
infra-docs-drift / docs-drift (pull_request) Successful in 5s
patchwarden-client-dry-run / collect-diff (pull_request) Successful in 4s
patchwarden-pr-sanity / collect-diff (pull_request) Successful in 4s
platformctl plan / auto-apply scope (pull_request) Successful in 20s
python-ci / Python 3.11 (pull_request) Successful in 43s
python-ci / Python 3.12 (pull_request) Successful in 45s
workflow-lint / lint (pull_request) Successful in 5s
canary-required / canary (pull_request) Successful in 13s
patchwarden-client-dry-run / dry-run (pull_request) Successful in 18s
pyfallow / Pyfallow gate (control-plane) (pull_request) Successful in 17s
python-ci / Python 3.13 (pull_request) Successful in 44s
patchwarden-pr-sanity / sanity (pull_request) Successful in 4m16s
patchwarden/contract Patchwarden: blocked (governance_tier_path)
63edc7332d
Author
Collaborator

Patchwarden verdict: BLOCKED
Action: do not merge this head until the blocking module is fixed.
Head 63edc7332d0e61fe7f7343fe9fd0e30dc0944dba.
Run 4c5e3d43-dc2f-4612-8dfa-a695cc933bf7.
Core stack patchwarden-core@v0 sha256:66bb609e751b2a66ddd4dfb58facff5eed54fa33e1c3c93dfbbc5f3cf0d8c3d4; modules: patchwarden.pr.path_classification, patchwarden.pr.required_checks.
Module outcomes: patchwarden.pr.path_classification fail (governance_tier_path); patchwarden.pr.required_checks pass.
Classification is not passable by local contract run: workflow. Path .forgejo/workflows/patchwarden-client-dry-run.yml.

<!-- patchwarden-contract target="forgejo://git.pdurlej.com/pdurlej/platform/pulls/789" --> **Patchwarden verdict: BLOCKED** Action: do not merge this head until the blocking module is fixed. Head `63edc7332d0e61fe7f7343fe9fd0e30dc0944dba`. Run `4c5e3d43-dc2f-4612-8dfa-a695cc933bf7`. Core stack `patchwarden-core@v0` `sha256:66bb609e751b2a66ddd4dfb58facff5eed54fa33e1c3c93dfbbc5f3cf0d8c3d4`; modules: `patchwarden.pr.path_classification`, `patchwarden.pr.required_checks`. Module outcomes: `patchwarden.pr.path_classification` fail (governance_tier_path); `patchwarden.pr.required_checks` pass. Classification is not passable by local contract run: workflow. Path `.forgejo/workflows/patchwarden-client-dry-run.yml`.
ci(patchwarden): upload contract inspection artifact
All checks were successful
base-is-main / guard (pull_request) Successful in 1s
canary-required / collect-diff (pull_request) Successful in 4s
infra-docs-drift / docs-drift (pull_request) Successful in 5s
patchwarden-client-dry-run / collect-diff (pull_request) Successful in 4s
patchwarden-pr-sanity / collect-diff (pull_request) Successful in 4s
platformctl plan / auto-apply scope (pull_request) Successful in 21s
pyfallow / Pyfallow gate (control-plane) (pull_request) Successful in 18s
python-ci / Python 3.11 (pull_request) Successful in 42s
python-ci / Python 3.12 (pull_request) Successful in 44s
python-ci / Python 3.13 (pull_request) Successful in 43s
workflow-lint / lint (pull_request) Successful in 5s
canary-required / canary (pull_request) Successful in 13s
patchwarden-client-dry-run / dry-run (pull_request) Successful in 21s
patchwarden-pr-sanity / sanity (pull_request) Successful in 4m32s
3db949dd30
pdurlej deleted branch codex/orders/patchwarden-review-run-softfail 2026-06-11 08:10:48 +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 milestone
No project
No assignees
3 participants
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!789
No description provided.