chore: sync workflow templates - #835
Conversation
Automated sync from stranske/Workflows Template hash: 536a715df8b0 Changes synced from sync-manifest.yml
stranske
added
sync
GitHub Actions
Failure
📝 WalkthroughWalkthroughAdds ChangesDeliberate-Break AC Verification Pipeline
Runtime AC Merge Guard
Sequence Diagram(s)sequenceDiagram
rect rgba(100, 149, 237, 0.5)
Note over pr-00-gate,check_deliberate_break.py: test-quality job
end
participant pr-00-gate
participant check_deliberate_break.py
participant git
participant pytest
participant GitHubAPI
pr-00-gate->>check_deliberate_break.py: run --base --head --pr-body-env
check_deliberate_break.py->>check_deliberate_break.py: parse_deliberate_break_spec(markdown)
alt marker present
check_deliberate_break.py->>git: diff base...head (tamper check)
check_deliberate_break.py->>pytest: run test at HEAD
check_deliberate_break.py->>git: git archive base ref
check_deliberate_break.py->>pytest: run test on base snapshot
check_deliberate_break.py-->>pr-00-gate: verdict JSON + GITHUB_OUTPUT
pr-00-gate->>GitHubAPI: add acceptance-criteria label
else marker absent
check_deliberate_break.py-->>pr-00-gate: SKIPPED
end
pr-00-gate-->>gate_summary.py: TEST_QUALITY_RESULT
gate_summary.py->>gate_summary.py: update state/description + render test-quality row
sequenceDiagram
rect rgba(144, 238, 144, 0.5)
Note over agents-73/agents-81,GitHubAPI: pre-merge runtime AC guard
end
participant agents-73/agents-81
participant assertRuntimeAcMergeAllowed
participant fetchPullRequestLabels
participant GitHubAPI
agents-73/agents-81->>assertRuntimeAcMergeAllowed: github, core, owner, repo, prNumber
assertRuntimeAcMergeAllowed->>fetchPullRequestLabels: issues.listLabelsOnIssue
fetchPullRequestLabels->>GitHubAPI: GET /repos/{owner}/{repo}/issues/{prNumber}/labels
GitHubAPI-->>fetchPullRequestLabels: label list
fetchPullRequestLabels-->>assertRuntimeAcMergeAllowed: normalized labels
assertRuntimeAcMergeAllowed->>assertRuntimeAcMergeAllowed: runtimeAcRequirement(labels)
alt runtime AC label found
assertRuntimeAcMergeAllowed-->>agents-73/agents-81: throw Error(code=runtime_ac_merge_blocked)
else no requirement
assertRuntimeAcMergeAllowed-->>agents-73/agents-81: allowed — proceed to merge
agents-73/agents-81->>GitHubAPI: pulls.merge (squash)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Automated Status SummaryHead SHA: d00ff88
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-00-gate.yml:
- Around line 326-330: The actions/checkout action at Line 326 keeps credentials
by default, which creates a token exfiltration risk when executing PR-controlled
test commands. Add persist-credentials: false to the with section of the
actions/checkout action to disable credential persistence and prevent sensitive
tokens from being available to potentially malicious PR code.
- Around line 346-362: The "Apply runtime acceptance-criteria label" step
currently relies on the default success() condition, which prevents it from
running if the "Check deliberate-break acceptance criterion" step fails. This
breaks the label contract that runtime merge guard logic depends on. Add an
explicit condition to the "Apply runtime acceptance-criteria label" step that
allows it to execute regardless of whether the deliberate-break check succeeds
or fails, ensuring the label is always applied as required by the runtime merge
guard logic.
In `@scripts/check_deliberate_break.py`:
- Around line 327-333: The call to parse_deliberate_break_spec(body) can raise a
ValueError when encountering malformed markers, causing the script to crash
before emitting consistent outputs and verdict JSON that downstream workflows
depend on. Wrap the parse_deliberate_break_spec(body) call in a try-except block
to catch ValueError exceptions, and when caught, emit the same consistent output
by calling _write_github_output() and _json_result() with an appropriate verdict
and reason message indicating the marker was malformed, then return the
appropriate exit code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 46f24a87-f5d2-4d29-aeb2-29e48830eefd
📒 Files selected for processing (9)
.github/scripts/gate_summary.py.github/scripts/runtime_ac_merge_guard.js.github/workflows/agents-73-codex-belt-conveyor.yml.github/workflows/agents-81-gate-followups.yml.github/workflows/agents-guard.yml.github/workflows/pr-00-gate.ymlAGENTS.mdCLAUDE.mdscripts/check_deliberate_break.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
stranske/Workflows(auto-detected)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
{AGENTS.md,CLAUDE.md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep AGENTS.md materially aligned with CLAUDE.md. Differences between the two should only be agent-specific execution notes, not different repository rules.
Files:
AGENTS.mdCLAUDE.md
.github/workflows/**/*.{yml,yaml}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
.github/workflows/**/*.{yml,yaml}: Ensure coverage threshold settings in GitHub Actions workflow files forcoverage-minmatch the[tool.coverage.report] fail_undersetting inpyproject.toml, as the lower value will be the effective threshold
Forstartup_failurein GitHub Actions workflows with zero jobs, check for invalid YAML syntax, top-levelpermissions:blocks inworkflow_callreusable workflows (which conflicts with caller permissions), invalid permission scopes, or circular workflow references
Files:
.github/workflows/agents-guard.yml.github/workflows/agents-73-codex-belt-conveyor.yml.github/workflows/agents-81-gate-followups.yml.github/workflows/pr-00-gate.yml
**/.github/workflows/*.yml
📄 CodeRabbit inference engine (AGENTS.md)
Reference reusable workflows with
@mainunless intentionally pinning to an exact commit SHA for a controlled reason.
Files:
.github/workflows/agents-guard.yml.github/workflows/agents-73-codex-belt-conveyor.yml.github/workflows/agents-81-gate-followups.yml.github/workflows/pr-00-gate.yml
{.github/workflows/agents-*.yml,.github/workflows/autofix.yml,.github/codex/**}
📄 CodeRabbit inference engine (AGENTS.md)
Agent-related workflow files (
agents-*.yml), autofix workflows (autofix.yml), and codex prompts (.github/codex/) should be fixed in Workflows, not in the consumer repo.
Files:
.github/workflows/agents-guard.yml.github/workflows/agents-73-codex-belt-conveyor.yml.github/workflows/agents-81-gate-followups.yml
.github/workflows/agents-*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
agents-*.yml workflow files should be edited in stranske/Workflows repository first, not locally, as they are synced from the source repository
Files:
.github/workflows/agents-guard.yml.github/workflows/agents-73-codex-belt-conveyor.yml.github/workflows/agents-81-gate-followups.yml
.github/workflows/pr-00-gate.yml
📄 CodeRabbit inference engine (AGENTS.md)
pr-00-gate.ymlis a create-only standard file; keep it aligned with the standard gate in Workflows unless this repo has a documented reason to diverge.pr-00-gate.yml is a create-only standard file that should match the standard gate in Workflows by default unless this repo has a documented reason to diverge
Files:
.github/workflows/pr-00-gate.yml
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In Manager-Database repository using Prefect 2.x, import schedules from
prefect.client.schemas.schedulesrather than other locations
Files:
scripts/check_deliberate_break.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: stranske/Template
Timestamp: 2026-06-20T01:47:24.245Z
Learning: Evaluate claims, designs, and instructions on merit before agreeing, including those from the orchestrator and user. When something is wrong or weaker than an alternative, state it plainly with the strongest objection first.
Learnt from: CR
Repo: stranske/Template
Timestamp: 2026-06-20T01:47:24.245Z
Learning: State confidence levels and what would change your mind when evaluating proposals. Flag uncertainties explicitly.
Learnt from: CR
Repo: stranske/Template
Timestamp: 2026-06-20T01:47:24.245Z
Learning: Do not soften real problems to be agreeable; do not manufacture disagreement to seem rigorous. Practice calibrated dissent, not maximal disagreement.
Learnt from: CR
Repo: stranske/Template
Timestamp: 2026-06-20T01:47:24.245Z
Learning: Keep workflow logic in `stranske/Workflows` repository. Consumer repositories should only carry repo-specific configuration unless explicitly documented as an exception.
Learnt from: CR
Repo: stranske/Template
Timestamp: 2026-06-20T01:47:24.245Z
Learning: For infrastructure work, follow the source-of-truth hierarchy: (1) `stranske/Workflows` root docs, (2) INTEGRATION_GUIDE and CONSUMER_REPO_MAINTENANCE, (3) consumer sync source templates, (4) local repo-specific files.
Learnt from: CR
Repo: stranske/Template
Timestamp: 2026-06-20T01:47:24.245Z
Learning: If a file is synced from Workflows, fix it in Workflows first rather than locally in the consumer repo.
Learnt from: CR
Repo: stranske/Template
Timestamp: 2026-06-20T01:47:24.245Z
Learning: Before editing local workflow infrastructure, determine whether the change belongs in `stranske/Workflows` instead. Changes affecting reusable workflows, agent prompts, routing, keepalive/autofix/verifier behavior, synced files, or synced scripts should be made in Workflows.
Learnt from: CR
Repo: stranske/Template
Timestamp: 2026-06-20T01:47:24.245Z
Learning: The current consumer default automation surface includes: `agents-issue-intake.yml`, `agents-80-pr-event-hub.yml`, `agents-81-gate-followups.yml`, `agents-verifier.yml`, `autofix.yml`, `ci.yml`, and `pr-00-gate.yml`.
Learnt from: CR
Repo: stranske/Template
Timestamp: 2026-06-20T01:47:36.015Z
Learning: Before editing local workflow infrastructure, determine whether the work belongs in stranske/Workflows instead by asking: Does this change affect reusable workflows, agent prompts or routing, keepalive/autofix/verifier behavior, synced workflow files, or synced scripts or docs? If yes, make the change in the source repository first.
Learnt from: CR
Repo: stranske/Template
Timestamp: 2026-06-20T01:47:36.015Z
Learning: When a file is synced from stranske/Workflows, fix it in Workflows first, not in the consumer repository
Learnt from: CR
Repo: stranske/Template
Timestamp: 2026-06-20T01:47:36.015Z
Learning: For infrastructure work, follow the source of truth hierarchy: (1) stranske/Workflows root docs (README.md, docs/WORKFLOW_GUIDE.md, docs/ci/WORKFLOWS.md), (2) INTEGRATION_GUIDE.md and CONSUMER_REPO_MAINTENANCE.md, (3) consumer sync source in stranske/Workflows/templates/consumer-repo/, (4) this repo's local repo-specific files
Learnt from: CR
Repo: stranske/Template
Timestamp: 2026-06-20T01:47:36.015Z
Learning: Your role is to evaluate claims, designs, and instructions on the merits before agreeing, including from orchestrators and users. When something is wrong or weaker than alternatives, state it plainly and lead with the strongest objection. Separate 'this is correct' from 'I'll do as asked,' state confidence and what would change your mind, and flag uncertainty without softening real problems.
🪛 ast-grep (0.43.0)
scripts/check_deliberate_break.py
[error] 143-150: Use of unsanitized data to create processes
Context: subprocess.run(
list(command),
cwd=cwd,
text=True,
capture_output=True,
env=env,
timeout=timeout,
)
Note: [CWE-78].
(os-system-unsanitized-data)
[error] 143-150: Command coming from incoming request
Context: subprocess.run(
list(command),
cwd=cwd,
text=True,
capture_output=True,
env=env,
timeout=timeout,
)
Note: [CWE-20].
(subprocess-from-request)
[error] 159-166: Command coming from incoming request
Context: subprocess.run(
["git", *args],
cwd=cwd,
check=True,
text=True,
capture_output=True,
timeout=timeout,
)
Note: [CWE-20].
(subprocess-from-request)
[error] 190-196: Command coming from incoming request
Context: subprocess.run(
["git", "archive", "--format=tar", base],
cwd=cwd,
check=True,
capture_output=True,
timeout=DEFAULT_TIMEOUT_SECONDS,
)
Note: [CWE-20].
(subprocess-from-request)
[info] 329-329: use jsonify instead of json.dumps for JSON output
Context: json.dumps(_json_result(VERDICT_SKIPPED, reason="no deliberate-break marker"))
Note: Security best practice.
(use-jsonify)
[info] 341-341: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result, sort_keys=True)
Note: Security best practice.
(use-jsonify)
🪛 zizmor (1.25.2)
.github/workflows/pr-00-gate.yml
[warning] 326-330: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 334-334: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 335-335: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 351-351: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 323-323: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
🔀 Multi-repo context stranske/Workflows
Based on my exploration of the stranske/Workflows repository, I can now provide a comprehensive cross-repository analysis.
Linked repositories findings
[::stranske/Workflows::]
Integration Points for New Scripts
1. runtime_ac_merge_guard.js — New merge guard script being synced
-
Module exports (.github/scripts/runtime_ac_merge_guard.js:131-137):
RUNTIME_AC_REQUIRED_LABELS(Set of 7 required labels:runtime-ac,runtime-verification,acceptance-criteria,verification-spec,verification-plan,ac-checks,runtime-checks)assertRuntimeAcMergeAllowed— async function that blocks PRs with runtime AC labelshasRuntimeAcRequirement,normalizeLabelName,runtimeAcRequirement— helper functions
-
Consumer usage (templates/consumer-repo/.github/workflows/agents-81-gate-followups.yml:1555, 1567):
- Sparse-checked in
guarded-mergejob - Called before merge:
await assertRuntimeAcMergeAllowed({github, core, owner, repo, prNumber, withRetry, source: 'agents-81-gate-followups guarded merge'}) - Blocks merge with error code
runtime_ac_merge_blockedif matching labels present
- Sparse-checked in
-
Tested contracts (.github/scripts/tests/runtime-ac-merge-guard.test.js:lines 8-10, 43-59):
- Tests validate: label normalization (case-insensitive, whitespace-trimmed), suffix matching (e.g.,
something:runtime-ac), API fallback behavior - Test confirms: "allows PRs without runtime AC labels" (lines 43-51) and "blocks exact runtime AC labels" (lines 53+)
- Tests validate: label normalization (case-insensitive, whitespace-trimmed), suffix matching (e.g.,
-
Consumer template test assertions (tests/workflows/test_workflow_agents_consolidation.py):
- Confirms
agents-81-gate-followupsincludesruntime_ac_merge_guard.jsin sparse checkout - Confirms
agents-73-codex-belt-conveyoralso usesassertRuntimeAcMergeAllowedin merge step - Tests that merge workflows calling
pulls.mergehave the guard assertion
- Confirms
2. check_deliberate_break.py — New execution-based verification script
-
Exports (scripts/check_deliberate_break.py):
DeliberateBreakSpecdataclass (test_id, test_file, break_file, command tuple)parse_deliberate_break_spec(markdown)→ returns spec or Noneverify_spec(spec, *, base, head, cwd, enforce_tamper)→ returns dict with verdict (PASS/FAIL_HOLLOW/FAIL_BROKEN/SKIPPED)main(argv)→ CLI entry point returning non-zero exit on non-PASS verdict
-
Workflow integration (.github/workflows/pr-00-gate.yml:303-342):
- New
test-qualityjob runs:python scripts/check_deliberate_break.pywith--baseand--headargs - Passes result to
summaryjob vianeeds.test-quality.result(line 642:TEST_QUALITY_RESULT: ${{ needs.test-quality.result || 'skipped' }})
- New
-
Test coverage (tests/scripts/test_check_deliberate_break.py):
- Tests parse deliberate-break markers from PR markdown
- Validates base-run vs head-run assertion logic (FAIL_HOLLOW when test passes on both)
3. gate_summary.py — Updated summary renderer
- Changes (.github/scripts/gate_summary.py):
- Line 24: Added field
test_quality_result: str = "skipped"toSummaryContextdataclass - Lines 247-257: Extended markdown generation to include
test-qualityrow in gate table - Lines 388-393: State transition logic —
cancelled→ pending, other non-success/skipped values → failure with description - Line 428: Reads
TEST_QUALITY_RESULTenvironment variable - Lines 349-400: Normalizes and applies test_quality_result to final summary state
- Line 24: Added field
Sync Manifest Configuration
[::stranske/Workflows::]
Declared sync entries (.github/sync-manifest.yml):
scripts/check_deliberate_break.py— marked as requirement for pr-00-gate.yml.github/scripts/runtime_ac_merge_guard.js— no template_sync directive (standard sync).github/scripts/gate_summary.py— standard sync for pr-00-gate.yml summary step- Workflows
pr-00-gate.yml,agents-73-codex-belt-conveyor.yml,agents-81-gate-followups.yml— all marked for sync withoverwrite_repos: [stranske/Template]
Cross-Workflow Dependencies
All three workflows that call these scripts have documented contracts:
- pr-00-gate.yml → calls check_deliberate_break.py, expects JSON output and exit code
- agents-73-codex-belt-conveyor.yml (template) → requires runtime_ac_merge_guard.js for merge gating
- agents-81-gate-followups.yml (template) → requires runtime_ac_merge_guard.js for guarded-merge job
The scripts are tested in stranske/Workflows before sync and sync manifest confirms all files are properly declared for distribution to consumer repos.
🔇 Additional comments (12)
.github/scripts/runtime_ac_merge_guard.js (3)
1-61: LGTM!
62-84: LGTM!
86-137: LGTM!.github/workflows/agents-73-codex-belt-conveyor.yml (2)
195-195: LGTM!
444-444: LGTM!Also applies to: 456-464
.github/workflows/agents-81-gate-followups.yml (3)
1555-1555: LGTM!
1567-1567: LGTM!
1736-1744: LGTM!.github/workflows/agents-guard.yml (1)
114-114: LGTM!Also applies to: 183-183
.github/scripts/gate_summary.py (1)
24-24: LGTM!Also applies to: 247-247, 257-257, 270-273, 331-342, 387-400, 428-443
AGENTS.md (1)
5-8: LGTM!CLAUDE.md (1)
5-8: LGTM!
| - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }} | ||
| ref: ${{ github.event.pull_request.head.sha || github.sha }} | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
Disable credential persistence before running PR-controlled commands.
At Line 326, checkout keeps credentials by default. This job executes PR-controlled test commands, so leaving repo credentials configured increases token exfiltration risk.
🔒 Proposed fix
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }}
ref: ${{ github.event.pull_request.head.sha || github.sha }}
fetch-depth: 0
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | |
| with: | |
| repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }} | |
| ref: ${{ github.event.pull_request.head.sha || github.sha }} | |
| fetch-depth: 0 | |
| - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | |
| with: | |
| repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }} | |
| ref: ${{ github.event.pull_request.head.sha || github.sha }} | |
| fetch-depth: 0 | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 326-330: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pr-00-gate.yml around lines 326 - 330, The
actions/checkout action at Line 326 keeps credentials by default, which creates
a token exfiltration risk when executing PR-controlled test commands. Add
persist-credentials: false to the with section of the actions/checkout action to
disable credential persistence and prevent sensitive tokens from being available
to potentially malicious PR code.
Source: Linters/SAST tools
| - name: Check deliberate-break acceptance criterion | ||
| id: deliberate_break | ||
| if: ${{ hashFiles('scripts/check_deliberate_break.py') != '' }} | ||
| run: >- | ||
| python scripts/check_deliberate_break.py | ||
| --base "refs/remotes/upstream/${{ github.event.pull_request.base.ref }}" | ||
| --head HEAD | ||
| env: | ||
| PR_BODY: ${{ github.event.pull_request.body || '' }} | ||
| - name: Skip deliberate-break check | ||
| if: ${{ hashFiles('scripts/check_deliberate_break.py') == '' }} | ||
| run: >- | ||
| echo "scripts/check_deliberate_break.py not present; | ||
| skipping opt-in deliberate-break check." | ||
| - name: Apply runtime acceptance-criteria label | ||
| if: ${{ steps.deliberate_break.outputs.has_marker == 'true' }} | ||
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9 |
There was a problem hiding this comment.
Ensure runtime labeling runs even when the deliberate-break check fails.
Apply runtime acceptance-criteria label currently uses default success() gating, so it will not run if the deliberate-break step exits non-zero. That breaks the label contract consumed by runtime merge guard logic.
💡 Proposed fix
- - name: Apply runtime acceptance-criteria label
- if: ${{ steps.deliberate_break.outputs.has_marker == 'true' }}
+ - name: Apply runtime acceptance-criteria label
+ if: ${{ always() && steps.deliberate_break.outputs.has_marker == 'true' }}🧰 Tools
🪛 zizmor (1.25.2)
[error] 351-351: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pr-00-gate.yml around lines 346 - 362, The "Apply runtime
acceptance-criteria label" step currently relies on the default success()
condition, which prevents it from running if the "Check deliberate-break
acceptance criterion" step fails. This breaks the label contract that runtime
merge guard logic depends on. Add an explicit condition to the "Apply runtime
acceptance-criteria label" step that allows it to execute regardless of whether
the deliberate-break check succeeds or fails, ensuring the label is always
applied as required by the runtime merge guard logic.
Source: Linked repositories
| spec = parse_deliberate_break_spec(body) | ||
| if spec is None: | ||
| _write_github_output(has_marker="false", verdict=VERDICT_SKIPPED) | ||
| print(json.dumps(_json_result(VERDICT_SKIPPED, reason="no deliberate-break marker"))) | ||
| print("skipped: no deliberate-break marker") | ||
| return 0 | ||
|
|
There was a problem hiding this comment.
Handle malformed deliberate-break markers without uncaught exceptions.
At Line 327, a malformed marker can raise ValueError from parsing and crash the script before emitting consistent outputs/verdict JSON. That makes downstream workflow branching brittle.
💡 Proposed fix
- spec = parse_deliberate_break_spec(body)
+ try:
+ spec = parse_deliberate_break_spec(body)
+ except ValueError as exc:
+ _write_github_output(has_marker="true", verdict=VERDICT_BROKEN)
+ print(json.dumps(_json_result(
+ VERDICT_BROKEN,
+ reason="marker-parse-error",
+ detail=str(exc),
+ )))
+ return 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| spec = parse_deliberate_break_spec(body) | |
| if spec is None: | |
| _write_github_output(has_marker="false", verdict=VERDICT_SKIPPED) | |
| print(json.dumps(_json_result(VERDICT_SKIPPED, reason="no deliberate-break marker"))) | |
| print("skipped: no deliberate-break marker") | |
| return 0 | |
| try: | |
| spec = parse_deliberate_break_spec(body) | |
| except ValueError as exc: | |
| _write_github_output(has_marker="true", verdict=VERDICT_BROKEN) | |
| print(json.dumps(_json_result( | |
| VERDICT_BROKEN, | |
| reason="marker-parse-error", | |
| detail=str(exc), | |
| ))) | |
| return 1 | |
| if spec is None: | |
| _write_github_output(has_marker="false", verdict=VERDICT_SKIPPED) | |
| print(json.dumps(_json_result(VERDICT_SKIPPED, reason="no deliberate-break marker"))) | |
| print("skipped: no deliberate-break marker") | |
| return 0 |
🧰 Tools
🪛 ast-grep (0.43.0)
[info] 329-329: use jsonify instead of json.dumps for JSON output
Context: json.dumps(_json_result(VERDICT_SKIPPED, reason="no deliberate-break marker"))
Note: Security best practice.
(use-jsonify)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/check_deliberate_break.py` around lines 327 - 333, The call to
parse_deliberate_break_spec(body) can raise a ValueError when encountering
malformed markers, causing the script to crash before emitting consistent
outputs and verdict JSON that downstream workflows depend on. Wrap the
parse_deliberate_break_spec(body) call in a try-except block to catch ValueError
exceptions, and when caught, emit the same consistent output by calling
_write_github_output() and _json_result() with an appropriate verdict and reason
message indicating the marker was malformed, then return the appropriate exit
code.
1 participant
Sync Summary
Files Updated
Files Skipped
Review Checklist
Source: stranske/Workflows
Source SHA:
deacb8ee2852a7c22fe229645468776f35921628Template hash:
536a715df8b0Sync branch:
sync/workflows-536a715df8b0Consumer repo:
stranske/TemplateManifest:
.github/sync-manifest.ymlSummary by CodeRabbit
New Features
Documentation