Skip to content

Raise bug-fix workflow credit cap and run pytest from PATH - #4508

Merged
mnriem merged 5 commits into
github:mainfrom
gyanu2507:fix/4472-bug-fix-credits-pytest
Sep 17, 2026
Merged

mnriem merged 5 commits into
github:mainfrom
gyanu2507:fix/4472-bug-fix-credits-pytest

Conversation

@gyanu2507

@gyanu2507 gyanu2507 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Description

The agentic bug-fix workflow hit the 1K default credit cap, then failed trying ./.venv/bin/python -m pytest with Permission denied.

This sets max-ai-credits: 2000, allows python as well as python3 on the bash allowlist, and tells the agent to use python3 -m pytest / pytest from PATH instead of a project venv interpreter.

Testing

  • Ran existing tests with uv sync && uv run pytest
  • uv run pytest tests/test_bug_fix_workflow.py

AI Disclosure

  • I did use AI assistance (describe below)

LLM-assisted implementation. I chose the issue, wrote the tests, and ran them locally.

Fixes #4472

The agentic bug-fix run hit the 1K default and then failed trying
./.venv/bin/python (Permission denied). Give the workflow 2K credits,
allow `python` as well as python3, and tell the agent to use PATH.

Fixes github#4472
@gyanu2507
gyanu2507 requested a review from mnriem as a code owner September 10, 2026 11:50
@mnriem

mnriem commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Thanks @gyanu2507 — a process heads-up. This one turns out to touch security-sensitive agentic-workflow infra: the change only takes effect once the compiled bug-fix.lock.yml is regenerated, and that has to be done with the exact pinned gh-aw compiler (v0.79.8), plus a careful diff to confirm nothing else in the lock (container digests, action SHAs, firewall allow-domains) shifts. Rather than ask you to chase our pinned toolchain, I'm going to pick up the recompile and finish this directly on your branch — you'll stay the author and I'll keep your commits.

Two small design tweaks I'm weighing as I do, with rationale to follow in the finishing commit: keeping the credit cap as-is if the pytest-from-PATH fix removes the retry loop that caused the ~8-credit overrun, and keeping the bash allowlist at python3 only unless bare python is genuinely needed. Appreciate you surfacing #4472 and getting it most of the way there.

@mnriem mnriem added the triage-must-have Verdict: high-value, important work for Spec Kit — do first label Sep 10, 2026
…fect

PR github#4508 raised max-ai-credits to 2000 and added `python` to the bash
allowlist in bug-fix.md, but never regenerated the compiled
bug-fix.lock.yml — the artifact GitHub Actions actually runs. Both
frontmatter-derived settings were therefore inert: the lock still pinned
maxAiCredits to the 1000 default and allowed shell(python3) but not
shell(python). (The prompt guidance was already live via {{#runtime-import}}.)

Recompile with gh-aw v0.79.8 (the pinned compiler) to apply exactly:
- agent + summary jobs' effective maxAiCredits: 1000 -> 2000
- harness allowlist: + shell(python)

Dependabot's action SHA pins in the committed lock (actions/checkout
v7.0.1, actions/setup-node v7.0.0) are preserved rather than reverted to
the compiler's older baked-in defaults. No container digest, firewall
domain, permission, or detection-job (400) change.

Rework tests/test_bug_fix_workflow.py to assert the compiled lock (the real
running artifact) for the cap and allowlist, and the runtime-imported
Markdown for the pytest-PATH guidance, replacing the prior .md-only string
checks that passed even while the running workflow was unchanged.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@gyanu2507

Copy link
Copy Markdown
Contributor Author

Sounds good. I'll leave the lockfile recompile and the credit / python allowlist tweaks to you.

@mnriem

mnriem commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Maintainer note: I (@mnriem) pushed one agent-assisted commit (5c52420) to complete this PR. Disclosure: it was generated autonomously by GitHub Copilot (model: Claude Opus 4.8) and reviewed by me before pushing.

The core gap was that .github/workflows/bug-fix.lock.yml — the compiled artifact GitHub Actions actually runs — was never regenerated, so two of the three changes were inert as merged: the effective maxAiCredits stayed at the compiled 1000 default, and the harness still allowed shell(python3) but not shell(python). (The pytest-PATH guidance was already live, since the lock imports the Markdown body at runtime via {{#runtime-import}}.)

I recompiled with the pinned gh-aw v0.79.8 and verified the lock diff is only the intended deltas: agent+summary maxAiCredits 1000 -> 2000 and + shell(python). Container image digests, action SHAs, firewall allow-domains, permissions, min-integrity, and the detection job's 400 cap are all byte-identical to main. One wrinkle worth flagging: a plain recompile wanted to revert Dependabot's committed action bumps (actions/checkout v7.0.1->v6.0.3, actions/setup-node v7.0.0->v6.4.0), so I preserved the v7 pins to avoid a downgrade.

I also reworked tests/test_bug_fix_workflow.py, which previously only asserted strings in bug-fix.md and stayed green even though the running workflow was unchanged; it now asserts the compiled lock for the cap and allowlist, plus the runtime-imported guidance.

One judgment call for reviewers: keeping the cap at 2000 is generous given the original overrun was ~0.84% and the real root cause was the permission-denied .venv interpreter (addressed by the prose). It stays bounded by the unchanged daily 5000 and detection 400 caps, and is a one-line revert if you prefer 1000.

CI has not run on this push and will need a maintainer trigger.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The source, generated workflow, and focused regression coverage consistently address both reported failures.

Pull request overview

Fixes #4472 by raising the bug-fix workflow’s credit budget and avoiding disallowed project-local Python executables.

Changes:

  • Raises AI credit limit to 2000.
  • Allows PATH-based python, python3, and pytest.
  • Adds regression tests covering source and compiled workflow consistency.
File summaries
File Description
.github/workflows/bug-fix.md Updates limits, allowlist, and test guidance.
.github/workflows/bug-fix.lock.yml Regenerates the executable workflow.
tests/test_bug_fix_workflow.py Guards the workflow configuration.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@mnriem

mnriem commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Thanks @gyanu2507 — the completed change now has passing CI and a review recommending approval on the current head. The compiled workflow includes the intended settings.

One disclosure detail remains before merge: please identify the agent/tool, model(s), and settings/mode used for your original contribution. My maintainer comment covers the later Copilot-authored completion commit, but your original work is currently described only as “LLM-assisted.”

No further code changes requested from this pass.

Drafted for @mnriem by GitHub Copilot (model: GPT-6 Astra).

@gyanu2507

Copy link
Copy Markdown
Contributor Author

Original commit is ac992b4.

That was an interactive coding-agent session. I picked #4472, went through the diff, and ran uv sync && uv run pytest tests/test_bug_fix_workflow.py locally before I pushed.

I don't have a reliable record of the exact model slug or sampling settings from that session. I steered which issue to take, the tests, and the commit.

5c52420 is the lockfile recompile I left to you.

@mnriem

mnriem commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Thanks, @gyanu2507. Please identify the coding-agent tool/product used for ac992b4; that is the remaining detail from the earlier disclosure request. For the exact model and sampling settings, explicitly stating "not recorded" is preferable to guessing. Please fold that clarification into the PR’s AI Disclosure section so the contribution record is self-contained. The separate disclosure for 5c52420 already covers the maintainer completion commit. No further code changes are requested.

Posted on behalf of @mnriem by GitHub Copilot (model: GPT-6 Astra).

@mnriem mnriem added author-needs-disclosure AI use, or the agent/model/settings behind it, not disclosed per CONTRIBUTING author-awaiting Waiting on author response labels Sep 16, 2026
mnriem and others added 2 commits September 16, 2026 16:03
…its-pytest

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
main advanced past this PR: github#4604 upgraded the bug-fix workflow to gh-aw
v0.88.7 (new container digests, setup-action pin, engine copilot 1.0.80),
which conflicted with the v0.79.8 lock this branch had generated. Merged
origin/main and regenerated bug-fix.lock.yml from source with the now-current
pinned compiler v0.88.7 so the artifact GitHub Actions runs reflects
@gyanu2507's frontmatter.

Net delta vs main is only the intended change: agent maxAiCredits 1000->2000
and the harness allowlist gains shell(python) alongside shell(python3). The
manifest (action SHAs, container digests), firewall allowDomains, permissions,
isolation, models, detection cap (400) and daily cap (5000) are byte-identical
to main. The .gitattributes churn the compiler emits is reverted to match main.

Focused tests (tests/test_bug_fix_workflow.py) assert the compiled lock, not
just the Markdown, and pass; the workflow-validation suite passes.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@mnriem
mnriem force-pushed the fix/4472-bug-fix-credits-pytest branch from 0ff3230 to e8c80aa Compare September 16, 2026 21:13
@mnriem
mnriem requested a balanced review from Copilot September 16, 2026 21:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new regression assertions can pass without verifying the executable allowlist or prohibition wording.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread tests/test_bug_fix_workflow.py Outdated
Comment thread tests/test_bug_fix_workflow.py Outdated
…rsed prohibition

Addresses two regression-test gaps from review of github#4508:

1. The shell-allowlist test matched bare shell(python) substrings, which also
   appear in the lock's commented tool inventory, so it passed even if
   compilation dropped the real --allow-tool arguments. It now isolates the
   executable harness command line (the sole copilot_harness.cjs reference) and
   asserts the full escaped --allow-tool 'shell(python)'/'shell(python3)'/
   'shell(pytest)' arguments.

2. The pytest-PATH test checked three independent substrings, so wording that
   recommended .venv/bin/python would still pass. It now asserts the normalized
   prohibition ("Do not invoke ... Permission denied.") as one contiguous unit,
   so reversing the guidance or splitting the permission-denied explanation away
   fails the test.

Source and compiled lock are unchanged; this is test-only strengthening.
Mutation checks confirm the old assertions passed while the new ones fail when
the executable allow-tool args are dropped, the prohibition is reversed, or the
permission-denied explanation is separated.

Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The source, generated workflow, and focused regression tests consistently implement the reported fix.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@mnriem

mnriem commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Maintainer completion is pushed to gyanu2507/spec-kit:fix/4472-bug-fix-credits-pytest at 1c495c8. Merged upstream main d743a69 to resolve the conflict introduced by #4604, then regenerated .github/workflows/bug-fix.lock.yml with this workflow’s current pinned compiler, gh-aw v0.88.7 (gh aw compile bug-fix). The earlier v0.79.8-generated lock was no longer current. Repeating compilation produces a byte-identical lock.

Against that upstream baseline, the effective configuration changes remain the intended credit cap of 2000 and the additional executable shell(python) permission alongside python3 and pytest. Action SHA pins, container digests, firewall domains, permissions, isolation/integrity settings, model settings, detection cap (400), and daily cap (5000) retain the upstream baseline. No unrelated generated changes were retained.

The final test-only commit closes both review gaps: assertions now inspect the executable harness arguments rather than its commented inventory, and the PATH guidance check requires the complete prohibition and permission-denied explanation together. Mutation checks demonstrated that dropping executable permissions, reversing the prohibition, or separating its explanation passed the old assertions but fail the new ones. All 29 focused workflow tests pass. Fresh-head CI has been approved, and the new Copilot review recommends approval with no new comments.

The original ac992b4 and 5c52420 commits are preserved. With explicit permission from @mnriem, only the two new maintainer commits were recreated using a lease-protected push to repair omitted merge-commit authorship trailers; file contents and merge parents were unchanged. Their replacements are f7c5825 and e8c80aa, followed by the normal test-only commit 1c495c8. All three carry both required AI-authorship trailers. The remaining author follow-up is the tool/product name requested in the earlier disclosure comment; no additional code work is requested from @gyanu2507.

Posted on behalf of @mnriem by GitHub Copilot (model: GPT-6 Astra). The implementation, regeneration, and test commits were authored by GitHub Copilot (model: Claude Opus 4.8, autonomous); this summary does not assert human line-by-line review.

@mnriem mnriem removed author-needs-disclosure AI use, or the agent/model/settings behind it, not disclosed per CONTRIBUTING author-awaiting Waiting on author response labels Sep 17, 2026
@mnriem
mnriem merged commit 7466e2a into github:main Sep 17, 2026
15 checks passed
@mnriem

mnriem commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-must-have Verdict: high-value, important work for Spec Kit — do first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[aw] Fix Bug from Labeled Issue exceeded max AI credits

3 participants