Skip to content

fix(reviewer): split agy prompt into trusted AGENTS.md, real argv, and a diff file - #175

Merged
asavs merged 1 commit into
mainfrom
fix/agy-review-task-untrusted-dir
Jul 23, 2026
Merged

asavs merged 1 commit into
mainfrom
fix/agy-review-task-untrusted-dir

Conversation

@asavs

@asavs asavs commented Jul 23, 2026

Copy link
Copy Markdown
Owner

Supersedes #174

#174 fixed the MAX_ARG_STRLEN argv crash (staging the entire untrusted payload behind a generic --print pointer) but traded it for two new problems, both found during review of that fix:

  1. It buried the angry personality's narrative "prefill" inside a file read via a tool call instead of the model's actual literal prompt — weakening a device that only ever worked because the payoff line was the last thing the model read before generating.
  2. append_ci_coverage_context was still inlining ~39–96KB of raw workflow YAML / package.json content into every review regardless of PR size — itself a quarter of what made PR M0: Add launch validation before enabling cron/live daemon #38's prompt oversized in the first place.

This PR redesigns the split three ways instead of patching #174 further:

  • AGENTS.md holds only genuinely trusted, daemon-authored content. The angry personality's simulated dialogue (append_angry_agents_prefill) is removed entirely — a Rules file is static instructions, not a conversation. Confirmed via agy's own bundled docs that AGENTS.md is a real, documented Rules-loading feature (auto-discovered by exact filename, walking up from cwd), not a workaround.
  • The literal --print argv value is a real, PR-specific prompt again. PR metadata, commit subjects, prior review/threads, and the angry interruption/prefix/tail are reunified into one continuous string assembled in agy.sh — small and bounded by construction (new REVIEWER_MAX_ARGV_PROMPT_BYTES, default 100000, plus a hard ~130000-byte backstop against the real kernel limit right before exec). Only the diff — the one section that can legitimately run large, and the one thing the model can't self-serve since the PR-head snapshot has no .git history — goes to a separately staged file (task_dir/REVIEW_DIFF.md).
  • CI coverage context is no longer inlined at all. append_ci_status (GitHub API pass/fail facts) stays; the raw YAML/JSON dump is replaced by one sentence in append_source_snapshot_hint pointing the model at .github/workflows/ and package.json to read itself if a finding needs it — the same explore-yourself idiom already proven for the PR-head snapshot generally.

Verification

  • Full fixture suite: 633/633 (up from 617) under WSL, the project's real GNU/Linux test runtime.
  • Live-VM dry run against asavs/mog-template#38 — the diff-heavy PR that's been silently failing to get reviewed since it opened: AGENTS.md dropped from what would have been ~45KB to 6.5KB, the reunified --print argv came in at 1268 bytes for a 112KB diff, and the model produced its first real review verdict on that PR (APPROVE), citing specific files by path from the separately-staged diff.

Test plan

  • bash -n on every touched shell file
  • bash scripts/reviewer/tests/run-fixtures.sh — 633/633
  • git diff --check
  • Throwaway sanity check: every argv-bounded section pushed to its configured maximum simultaneously stays at ~11.9KB, far under both the 100000-byte argv budget and the 130000-byte hard kernel-limit check
  • Live dry run against a real diff-heavy PR, inspecting the dry-run artifact's AGENTS.md/prompt/diff sections directly

…d a diff file

The interim file-delivery fix for the MAX_ARG_STRLEN argv crash (staging
the entire untrusted payload behind a generic --print pointer) traded one
problem for two others: it buried the angry personality's narrative
"prefill" inside a file read via a tool call instead of the model's
actual literal prompt, and it left `append_ci_coverage_context` inlining
~39-96KB of raw workflow YAML / package.json into every review regardless
of PR size -- itself a quarter of what made PR #38's prompt oversized in
the first place.

Redesign the split three ways instead:

- AGENTS.md holds only genuinely trusted, daemon-authored content. The
  angry personality's simulated dialogue (`append_angry_agents_prefill`)
  is removed from it entirely -- a Rules file is static instructions, not
  a conversation, and agy's own documented Rules system (auto-loading
  exactly GEMINI.md/AGENTS.md/.agents/rules/*.md by walking up from cwd)
  confirms AGENTS.md was always the real trusted channel here, not a
  workaround.
- The literal --print argv value is a real, PR-specific prompt again: PR
  metadata, commit subjects, prior review/threads, and the angry
  interruption/prefix/tail are reunified into one continuous string
  assembled in agy.sh, small and bounded by construction (new
  REVIEWER_MAX_ARGV_PROMPT_BYTES budget, default 100000, plus a hard
  ~130000-byte backstop against the actual kernel limit right before
  exec). Only the diff -- the one section that can legitimately run large
  and the one thing the model cannot self-serve, since the PR-head
  snapshot has no .git history -- goes to a separately staged file
  (task_dir/REVIEW_DIFF.md, --add-dir'd alongside the trusted workspace).
- CI coverage context is no longer inlined at all. append_ci_status
  (GitHub-API pass/fail facts) stays; the raw YAML/JSON dump is replaced
  by one sentence in the existing append_source_snapshot_hint, pointing
  the model at .github/workflows/ and package.json to read itself if a
  finding needs it -- the same explore-yourself idiom already proven for
  the PR-head snapshot generally, not new machinery.

Verified live against PR asavs/mog-template#38 (the diff-heavy PR that
had been failing since it opened): AGENTS.md dropped from what would
have been ~45KB to 6.5KB, the reunified --print argv came in at 1268
bytes for a 112KB diff, and the model produced its first real review
verdict on that PR. Full fixture suite passes (633 assertions, up from
617; new coverage for the argv/diff split, the CI self-serve pointer,
and the reunified angry narrative in the actual --print value rather
than build_review_prompt's own output).
@asavs
asavs merged commit 7a908a1 into main Jul 23, 2026
1 of 2 checks passed
@asavs
asavs deleted the fix/agy-review-task-untrusted-dir branch July 23, 2026 21:54
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