Skip to content

fix: TaskOutput-timeout guard, actor-permission preflight, report deps - #7

Merged
lklimek merged 1 commit into
mainfrom
fix/ci-run-followups
Jul 20, 2026
Merged

fix: TaskOutput-timeout guard, actor-permission preflight, report deps#7
lklimek merged 1 commit into
mainfrom
fix/ci-run-followups

Conversation

@lklimek

@lklimek lklimek commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-ups from auditing dashpay/dash-evo-tool#908's first successful run (29744073162) after the run_in_background fix (#6) landed. The fix worked mechanically, but the audit surfaced a few more things worth hardening:

  • TaskOutput timeout re-block guard. TaskOutput(block=true) can time out and return status: running instead of a final result — that's not completion. The prompt now requires re-blocking on timeout instead of moving on. Made this rule explicitly transitive: the orchestrator must pass it down to any specialist it spawns (e.g. via grumpy-review), since a nested reviewer that fans out its own sub-agents has no way to know it's running headless otherwise. This wasn't theoretical — the QA specialist in that same run hit exactly this at the 10-minute mark and only recovered by model discretion, not enforcement.
  • Actor-permission preflight. The underlying claude-code-action rejects triggering actors without write access from deep inside its own execution (~40s in, an opaque "Action failed with error: User does not have write access on this repository"). We hit this for real on the first re-trigger attempt. Added a fast gh api .../collaborators/{actor}/permission check up front so it fails in a few seconds with a clear message instead.
  • Report-rendering dependencies. generate_review_report.py crashed on import markdown mid-run (the CI runner image doesn't have claudius's documented Python deps) and got silently retried around. Installs them via apt before the review runs.
  • Label-removal error visibility. Was 2>/dev/null || true — a silent failure there leaves the trigger label on the PR, which re-triggers a full review on the next push. Now surfaces a ::warning::.

Test plan

  • python3 -c "import yaml; yaml.safe_load(open('action.yml'))" — YAML parses cleanly (yamllint unavailable locally)
  • Exercise on a live PR via uses: lklimek/claudius-review-action@fix/ci-run-followups — in particular confirm the write-access preflight fails fast and cleanly for a non-write actor, and that report generation no longer hits the markdown import error

🤖 Generated with Claude Code

Follow-ups from auditing dashpay/dash-evo-tool#908's successful run
(29744073162) after the run_in_background fix landed:

- TaskOutput can time out mid-block and return status:running instead of a
  final result; the prompt now requires re-blocking on timeout instead of
  treating that as "collected". Made explicit that this rule is transitive —
  the orchestrator must pass it down to any specialist it spawns (e.g. via
  grumpy-review), since a nested reviewer that itself fans out sub-agents has
  no way to know it's running headless otherwise. Observed live: a QA
  sub-agent hit exactly this and only recovered by model discretion, not
  enforcement.
- Add a fast write-access preflight: the underlying claude-code-action
  rejects non-write-access triggering actors from deep inside its own
  execution (~40s in, opaque "Action failed with error" message). Check via
  `gh api .../collaborators/{actor}/permission` before paying that cost.
- Install claudius's documented report-rendering Python deps (markdown,
  jsonschema, jinja2, etc.) via apt — the runner image doesn't have them,
  which was crashing generate_review_report.py mid-run (`import markdown`)
  and getting silently retried around.
- Surface (rather than swallow) trigger-label removal failures — a silent
  failure there leaves the label on the PR and re-triggers a full review on
  the next push.
@lklimek
lklimek merged commit ecf1ebf into main Jul 20, 2026
1 check passed
@lklimek
lklimek deleted the fix/ci-run-followups branch July 20, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant