Skip to content

fix(chore-style): forbid create_issue in autofix mode; agent must open PR #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
norrietaylor wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(chore-style): forbid create_issue in autofix mode; agent must open PR #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 44 additions & 16 deletions .github/workflows/chore-style-go.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 40 additions & 1 deletion .github/workflows/chore-style-go.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ safe-outputs:
private-key: ${{ secrets.APP_PRIVATE_KEY }}
repositories:
- ${{ github.event.repository.name }}
# Note (issue #30): both create-issue and create-pull-request are listed
# so the agent has the right tool available in either mode. The agent
# MUST select the tool that matches inputs.mode — see the prose contract
# below. Gating via templated `max` was attempted but rejected: every
# available gh-aw expression form for a ternary (`cond && '0' || '1'`,
# `fromJSON('{"k":"v"}')[inputs.x]`) renders into the safe-outputs JSON
# env var with characters that actionlint cannot lex (`&&` → `&&`;
# nested `\"` inside `fromJSON()`). The prose contract is the primary
# defense; if the agent calls create_issue in autofix anyway, the
# safe-output allowlist will still create the issue but the chore
# behaviour is wrong and visible from the run log.
Comment on lines +43 to +53

@coderabbitai coderabbitai Bot May 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Add runtime enforcement for mode/tool mismatch, not just prompt prose

This block confirms the system still permits create_issue side effects in autofix when the agent miscalls the tool. That means the #30 failure mode can recur under model drift. Please add an execution-time guard (before safe-output processing) that rejects mismatched tool types by inputs.mode so wrong outputs fail closed instead of being applied.

🤖 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/chore-style-go.md around lines 43 - 53, Add a runtime
guard that validates inputs.mode against the chosen side-effect tool before
safe-output processing: inspect inputs.mode and the parsed tool invocation
(create_issue vs create_pull_request) and if they mismatch, abort processing
with a non-zero exit (or throw) so the workflow fails closed; implement this
check in the code path that handles safe-output parsing/dispatch (the
module/function that currently processes safe-output) and ensure the error
message clearly references inputs.mode and the invoked tool so the mismatch is
visible in logs.

@norrietaylor norrietaylor May 21, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged — the model-drift concern is real. Runtime enforcement was investigated in the original PR work and is blocked upstream in gh-aw today; the rationale is captured both in the Why no max: gating section of the PR body and in the inline YAML comment in safe-outputs: in every chore-style-* source file.

Short version: every supportable gh-aw expression form for a templated max (the natural gating point) renders into the safe-outputs JSON env var with characters that actionlint refuses (&& → &&; nested \" inside fromJSON() breaks the expression lexer). gh-aw's safe_outputs_config_generation.go uses plain json.Marshal (HTML-escaping on) instead of the project's own MarshalCompactNoHTMLEscape. Fixing this is a 1-line gh-aw upstream change and out of scope for this PR.

Tracking the upstream side and a complementary fixture-based CI defense in a new issue rather than blocking this PR. The current prose+forbidden-list contract is the strongest defense available without an upstream change; adding a runtime parse-and-fail step inside the chore lock would require either hand-editing the generated lock files (against repo conventions — see CLAUDE.md) or duplicating the chore logic into a wrapper-side validator, both of which trade more surface area for marginal extra safety.

@coderabbitai coderabbitai Bot May 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@norrietaylor, thanks for the thorough write-up — the constraint is clear and well-documented. The root cause (json.Marshal HTML-escaping in safe_outputs_config_generation.go producing && that actionlint can't lex) is a genuine upstream blocker, and the prose-contract approach is the right call given that hand-editing generated lock files is out per CLAUDE.md. I'll withdraw the original concern for this PR.

Would you like me to open the tracking issue for the two follow-up items you mentioned?

  1. Upstream gh-aw fix — switch safe_outputs_config_generation.go from json.Marshal to the project's MarshalCompactNoHTMLEscape so max: can be gated with a templated expression.
  2. Fixture-based CI defense — a workflow test that asserts create_issue is never invoked when inputs.mode == autofix (and vice versa), catching model-drift regressions without requiring the upstream change.

Just say the word and I'll create it and link it back to this PR and #30.

✏️ Learnings added 
Learnt from: norrietaylor
Repo: norrietaylor/ch-oracles PR: 39
File: .github/workflows/chore-style-go.md:43-53
Timestamp: 2026-05-21T14:40:24.043Z
Learning: In the ch-oracles repo (norrietaylor/ch-oracles), runtime gating of safe-output tool calls by `inputs.mode` (e.g. blocking `create_issue` in autofix mode via a templated `max:` expression in gh-aw safe-outputs config) is currently blocked because gh-aw's `safe_outputs_config_generation.go` uses `json.Marshal` (HTML-escaping enabled) instead of the project's `MarshalCompactNoHTMLEscape`. This causes actionlint to reject the rendered env var due to `&&` (from `&&`) and nested escaped quotes. The fix is a 1-line upstream change to gh-aw; until then, the prose contract + explicit forbidden-list is the strongest available defense. Hand-editing generated lock files is prohibited per CLAUDE.md.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

create-issue:
max: 1
labels:
Expand Down Expand Up @@ -87,6 +98,30 @@ Behavior summary:

You are the Go style agent. Read `inputs.mode` and act accordingly.

## Mode → safe-output contract (READ FIRST)

The safe-output tool you call MUST match `inputs.mode`. Picking the wrong
tool is the defect tracked in issue #30 and is the single most important
rule in this prompt.

- `mode == report`:
- You MUST call `create_issue` (or `update_issue` on dedup hit).
- You MUST NOT call `create_pull_request`. Report mode does not modify
files; opening a PR is a contract violation.
- `mode == autofix`:
- You MUST call `create_pull_request`.
- You MUST NOT call `create_issue` or `update_issue` under any
circumstances — not as a fallback, not to "also notify", not because
the verification gate failed. If the verification gate fails, emit
`report_incomplete` and stop; do not file a new issue.
- Even though `create_issue` appears in the safe-outputs allowlist
(so report mode can use it), calling it from autofix is a contract
violation tracked by issue #30. The wrong-tool behaviour is visible
in the run log and treated as a defect.

If you find yourself about to call `create_issue` while `inputs.mode ==
autofix`, stop and re-read this section.

## Mode: report

1. `gofmt -l .` — list files needing format (empty output means clean).
Expand Down Expand Up @@ -124,7 +159,9 @@ Apply dedup before emitting.
the PR**; staticcheck has no `--fix` mode and surfaces semantic issues.
- `go build ./...` — must exit 0.
- `go test ./...` — must exit 0.
4. Open one PR via `create-pull-request`:
4. Open one PR via `create-pull-request` (safe-output tool
`create_pull_request`). **Do not call `create_issue` in this mode** —
see the contract above.
- Title: `[lint:go] auto-applied gofmt + goimports`.
- Body: summary of files touched, `Closes #<n>` if applicable.
- Labels: `agent:lint:go`, `agent:autofix`.
Expand All @@ -142,3 +179,5 @@ Apply dedup before emitting.
- Do not modify `go.mod` or `go.sum`.
- Do not auto-apply go vet or staticcheck suggestions.
- Do not open more than one PR or issue per run.
- Do not call `create_issue` or `update_issue` when `inputs.mode == autofix`.
- Do not call `create_pull_request` when `inputs.mode == report`.
Loading
Loading