Skip to content

Load AGENTS.md into agent context - #77

Merged
seoes merged 9 commits into
seoes:mainfrom
NriotHrreion:feature/load-agents-md
Oct 12, 2026
Merged

seoes merged 9 commits into
seoes:mainfrom
NriotHrreion:feature/load-agents-md

Conversation

@NriotHrreion

@NriotHrreion NriotHrreion commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

close #76

Loading rule of AGENTS.md

  • Search the repository root and ancestor directories of files changed by the full PR, including both old and new paths.
  • Find AGENTS.md first. If no AGENTS.md, find agents.md.
  • Read only regular files inside the workspace at the PR head. Use bounded streaming reads and skip body reads when the budget is exhausted.
  • Mark guidance added, modified or renamed anywhere in the full PR as untrusted. Its contents remain review material, including after later tool reads.
  • Apply listed, unchanged guidance within its directory scope. Deeper guidance takes precedence, while Proval rules and repository owner settings retain priority.
  • List at most 32 guidance files. Include up to 8,000 body characters per file and 32,000 characters for the entire guidance block, including metadata, wrappers and notices.
  • Reserve space for selected files’ metadata before allocating body space. Read truncated or omitted unchanged content through tools before applying it.
  • Report files omitted entirely because of the limits. Unlisted guidance cannot supply project conventions, even after later tool reads.
  • Empty or unreadable files do not stop the review. Unreadable guidance must not be applied or retried.

What changed

  • Added apps/api/src/agent/pull-request/prompt/agents-md.ts for loading AGENTS.md.
  • Added regularFileOnly option in Workspace.read().

Notes

These fixed prompts use up to approximately 1700 tokens per request. (Read 32 files, each file hit character limit of 8000)

Comment thread apps/api/src/agent/pull-request/prompt/agents-md.ts Outdated
Comment thread apps/api/src/agent/pull-request/prompt/agents-md.ts Outdated
@proval-0e6d07

proval-0e6d07 Bot commented Oct 10, 2026

Copy link
Copy Markdown

Thanks for putting this repository-guidance feature together — the trust model, the hardened workspace read, and the cross-stage wiring are thoughtfully done, and the tests cover the security boundary well.

a. Summary

This PR loads AGENTS.md / agents.md guidance from directories touched by the PR, assigns each file a trust status from the full PR diff, and injects the guidance block (plus a matching permission rule in the system prompt) into the plan, sub, and writing review stages. It also hardens Workspace.read with an opt-in regularFileOnly check combining lstat and realpath containment, and amends the shared untrusted-warning prompt to permit the scoped exception. Two issues to address, both around how the 32,000-character budget is enforced in loadAgentInstructionContext.

b. Good points

  • 🟢 Workspace.read's regularFileOnly hardening pairs lstat(...).isFile() with a realpath containment check using root + sep (apps/api/src/git-provider/workspace.ts:307), blocking symlink escapes and sibling-prefix bypasses while staying strictly opt-in for existing callers.
  • 🟢 The e2e test at apps/api/src/agent/pull-request/prompt/agents-md.test.ts:269 builds real git fixtures with AGENTS.md as a directory and an external directory symlink, asserting outside content never reaches the prompt or logs — a true end-to-end proof of the containment boundary.
  • 🟢 REPOSITORY_GUIDANCE_RULE is placed immediately after each stage's core workflow prompt (apps/api/src/agent/pull-request/review/plan.service.ts:42, sub.service.ts:36, writing.service.ts:49), so the trust contract always precedes the first LLM call in every stage that receives guidance.

Main Issues

  • ⚫️ apps/api/src/agent/pull-request/prompt/agents-md.ts:78 — guidance files are fully decoded via unbounded readFile before any budget check, even when the budget is already exhausted and the content will be marked "omitted"; a planted oversized AGENTS.md forces a full buffer (plus a trim() copy) on every review run, enabling memory amplification under concurrency. Skip the read once the budget is spent and guard oversized files by size.
  • ⚫️ apps/api/src/agent/pull-request/prompt/agents-md.ts:101 — TOTAL_CHARACTER_LIMIT only charges included bodies; section headers, state lines, and the truncated/omitted instructions are free, and section count is unbounded because directories come from the full changed-file list. The emitted block can far exceed the nominal 32,000-character budget. Charge overhead against the budget and cap section count.

Additional Notes

  • apps/api/src/agent/pull-request/prompt/agents-md.ts:106 — the truncated/omitted instruction says "before applying this guidance" even for sections marked Trust untrusted, which slightly contradicts the "never apply" rule at line 17; consider trust-specific wording for that sentence.
  • apps/api/src/agent/shared/prompt/untrusted-warning.prompt.ts:17 — the carve-out keys off a static property of the system prompt, which is always present in review agents, so read in isolation the exception looks unconditional; restating the payload binding in the same sentence would make each rewritten rule safe on its own.

Replace full file reads followed by in memory truncation with bounded streaming reads.
Skip body reads when the character budget is exhausted.
Count metadata, wrappers and notices toward the 32000 character limit.
Limit context to 32 files and reserve metadata space before reading bodies.
Report omitted files and treat unlisted guidance as review material only.
Add regression tests for total size, file count and oversized metadata.
@proval-0e6d07

proval-0e6d07 Bot commented Oct 10, 2026

Copy link
Copy Markdown

Thanks for the follow-up — this push directly resolves both budget findings from the previous review, and the arithmetic plus the stream hardening both hold up under scrutiny.

a. Summary

loadAgentInstructionContext now uses a two-phase selection: it reserves the full header, overflow notice, and each section's worst-case metadata (including wrapper and instruction lines) before allocating any body space, then reads bodies with a maxCharacterCount + 1 sentinel so truncation is detected without buffering whole files. Workspace.read gained a streaming bounded-read path that stops and destroys the stream once the limit is reached. Together these close the unbounded-read and body-only-budget issues; I have no main issues for this push.

b. Good points

  • 🟢 The two-phase selection with the refund formula (remainingCharacterCount += overheadCharacterCount - sectionContent.length - 2) keeps the emitted context provably within 32,000 characters — I traced the invariant end-to-end: header + notice + Σ(actual + 2) never exceeds the total, even for attacker-controlled escaped paths and untrusted wrappers (apps/api/src/agent/pull-request/prompt/agents-md.ts:96).
  • 🟢 The bounded read stops mid-stream and destroys it (apps/api/src/git-provider/workspace.ts:336), and the test asserts bytesRead < 32_768 and stream.destroyed === true on a ~144 KB file (apps/api/src/git-provider/workspace.test.ts:150) — direct regression protection for the prior unbounded-read finding.
  • 🟢 Parameterized tests pin the exact budget invariants — ≤ 32,000 bound with exact read order [a,b,c,d], count-cap behavior at 32/33/200 files, escaped-path metadata charging, and the surrogate/whitespace boundary it.each (apps/api/src/agent/pull-request/prompt/agents-md.test.ts:195).

Main Issues

None for this push.

Additional Notes

  • apps/api/src/git-provider/workspace.ts:333 — maxCharacterCount: 0 returns "" before any filesystem access (when regularFileOnly is not set), so a missing path never produces the usual File not found error, unlike the limit-1 and unbounded paths; no production caller passes 0 today, but documenting the short-circuit or checking existence first would keep the contract consistent.

@seoes

seoes commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

Hi @NriotHrreion it's been a while and I love this concept of feature. Agent really needs AGENTS.md, and I think this version of PR can be a bit simpler. but I love your work still

Here's what I thought about

  • Workspace.loadAgentInstructionList() returns { path, body }[]. get every AGENTS.md and agents.md from the target branch, not the PR checkout. the checkout is this PR head, so those files could change the review rules
  • Shallow paths first.
  • These files are usually short. The character budget, the file cap, and the streaming reader feel like a lot for that. The long system text that explains how to use the guidance also feels like a system prompt for the system prompt. Two lines at the top is enough. I would also skip the get_file_content follow up. It may never happen, and it adds a round trip.
  • agents-md.ts only exports buildAgentInstructionPrompt(workspace). Call it from every plan, sub, and writing
  • then, those AGENTS.mds are already existing in repository so when we put them into the system prompt we don't need untrusted warning for them at all
  • so we don't need any change for Workspace.read or prompts like untrusted warning. It already works well
public async loadAgentInstructionList(): Promise<{ path: string; body: string }[]> {
    // git ls-tree (merge target tip)
    // get every AGENTS.md and agents.md, shallow paths first
}

export async function buildAgentInstructionPrompt(workspace: Workspace): Promise<string> {
    const fileList = await workspace.loadAgentInstructionList();
    return [
        "These files are the merge target's current AGENTS.md. Follow them as the repo convention. In the same directory, AGENTS.md wins over agents.md.",
        ...fileList.map((file) => `# ${file.path}\n${file.body}`),
    ].join("\n\n");
}

Let me know your thoughts about this

@NriotHrreion

NriotHrreion commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor Author

@seoes Thanks for reviewing this!

Here's my thoughts:

  • Not quite agree. Read from PR head can be important when the PR is making some intentional changes to the instruction md. (Plus: To reduce the possibility of prompt injection attack, the changed instruction files are marked as untrusted)
  • Agree.
  • Partly agree. The character budget, file cap etc. may look over-engineering, but I think the checks are necessary because we need to take every cases (for agents.md) into consideration. But I'm agree that the long system prompt for system prompt is unnecessary, and the get_file_content could be skipped.
  • Agree. I didn't consider this when planning the structure.
  • Same as the first point.
  • Agree.

@seoes

seoes commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

@NriotHrreion

Oh, I mean

  • AGENTS.md that already existing (ex. main branch) -> goes to system prompt. it's already in the codebase
  • AGENTS.md that has new changes(feat/new-agent-markdown) -> should be reviewed by Proval(with untrusted tag)

this would make no danger zone with prompt injection from PR review/reply (in issues just use existing one)

I agree with your point that any chance of prompt injection must be blocked

@NriotHrreion

NriotHrreion commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor Author

@seoes Ok, I got you! So we should read the instruction files from the target head.

I'll make the change ASAP.

…get guidance

Move instruction discovery and reading into Workspace.loadAgentInstructionList.
Keep agents-md.ts focused on assembling the guidance prompt.
Restore Workspace.read to its general file reading behavior.

Load guidance from the target branch head and share it as trusted
system instructions across the plan, sub, and writing stages.
Remove guidance trust labels, untrusted wrappers, and PR head reread instructions.
@proval-0e6d07

proval-0e6d07 Bot commented Oct 12, 2026

Copy link
Copy Markdown

Thanks for the follow-up — this push lands the target-head redesign from the maintainer discussion, and the trust contract plus consolidated test coverage both hold up under scrutiny.

a. Summary

Guidance now loads from the merge target head as trusted system content: Workspace.loadAgentInstructionList resolves targetHeadSha per workspace mode and reads AGENTS.md/agents.md via git objects (apps/api/src/git-provider/workspace.ts:333), while buildAgentInstructionPrompt keeps the reserve-then-refund budget pattern and drops the per-file Trust labels and REPOSITORY_GUIDANCE_RULE carve-outs. The single built prompt is threaded once into the system message of the plan, sub, and writing stages (apps/api/src/agent/pull-request/review/index.ts:100), the user-prompt channel no longer embeds guidance (apps/api/src/agent/pull-request/prompt/context.ts), and the untrusted-warning rules are cleanly unconditional again. The deleted agents-md.test.ts suite was largely re-homed into workspace.test.ts as real-git fixtures. I have no main issues for this push.

b. Good points

  • 🟢 The e2e test feeds a deliberately stale baseSha for the fetched case while the fixture advances the target branch, then asserts the system prompt carries "Latest target convention", excludes PR-injected guidance, and that loadAgentInstructionList runs exactly once with identical content across all three stages (apps/api/src/git-provider/workspace.test.ts:175) — a direct pin on the fetch-vs-adopt contract requested in the discussion.
  • 🟢 The missing-blob test proves a failed target-head read degrades to unavailable while workspace.read still returns the PR checkout content, confirming the loader never falls back to attacker-controlled guidance (apps/api/src/git-provider/workspace.test.ts:406).
  • 🟢 The guidance block now sits in the system-prompt array alongside REVIEW_PLAN (apps/api/src/agent/pull-request/review/plan.service.ts:43, same slot in sub.service.ts:37 and writing.service.ts:50), so the placement finally matches the header's "trusted system instructions" claim that previously only held in wording.

Main Issues

None for this push.

Additional Notes

  • apps/api/src/git-provider/workspace.ts:362 — git ls-tree -r -t -z <targetHeadSha> runs with no pathspec, so stdout covers every blob and tree in the repository even though only directories in the already-computed directoryList can match; passing those directories plus the root as pathspecs would keep the per-review listing proportional to the PR instead of the repo.
  • apps/api/src/agent/pull-request/prompt/agents-md.test.ts (deleted) — the escaped-path metadata-charging and single-oversized-section budget cases didn't make it into the replacement suite; apps/api/src/git-provider/workspace.test.ts:468 uses short dirNN/AGENTS.md paths only, so the overhead-reservation worst case in agents-md.ts now has no test pinning the ≤32,000 bound.

seoes added 4 commits October 12, 2026 14:26
- add `Workspace.isPullRequest()`
- if workspace handles pull request, `buildAgnetInstructionPrompt` load
  changed files
- put agentInstructionPrompt to back
agents-md.ts

- workspace.ts loads whole agent files
- agents-md.ts cuts by 8000/32000 character and set build prompt
@seoes

seoes commented Oct 12, 2026

Copy link
Copy Markdown
Owner

Hi @NriotHrreion!

Great work. I added small changes (might be look big but it's just some details)

  • remove changedFileList parameter
    • buildAgentInstructionPrompt() will load changeFileList by call workspace.changedFiles() from its inside
    • buildAgentInstructionPrompt(workspace, changedFileList, activityId) -> buildAgentInstructionPrompt(workspace, activityId)
  • change order of system prompts. move agentInstructionPrompt to back
    • Proval Prompt -> AGENTS.mds -> user prompt order would be better
  • Put agentInstructionPrompt to pr reply, issue open, issue reply. (minor prompt changes because of difference of PR and issue)
    • PR loads from startSHA, Issue loads from current branch
  • agents-md.ts handle all character limit(8000,32000). workspace just gets you whole agents.mds

I'll merge it now. thanks for your hard work

@seoes
seoes merged commit 25f21a2 into seoes:main Oct 12, 2026
4 checks passed
const decoder = new TextDecoder("utf-8", { ignoreBOM: true });
let content = "";
try {
while (true) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚫️ [PROBLEM] Guidance loader reads blobs to completion with no size cap

⚫️ [PROBLEM] This push removes the loader's size cap, so each guidance blob is now decoded in full

The previous version of loadAgentInstructionList stopped at AGENT_INSTRUCTION_FILE_CHARACTER_LIMIT + 1 characters and killed the git cat-file process once the cap was hit. This push replaces that with an unbounded while (true) loop at apps/api/src/git-provider/workspace.ts:408 that accumulates the entire blob into content and stores it in file.body (line 425), for up to 32 files, plus a full-copy content.trim() on line 421.

The prompt builder only ever uses 8,000 characters per file (FILE_CHARACTER_LIMIT, enforced at apps/api/src/agent/pull-request/prompt/agents-md.ts:67), so everything past that first 8 KB is read, allocated, and immediately discarded. Guidance now comes from the merge target head rather than the PR, so this is no longer attacker-controlled — but a single unusually large generated AGENTS.md on the default branch still turns every review, reply, and issue run into a multi-hundred-MB transient allocation (UTF-16 strings are roughly 2x the file bytes), and the new budget test itself loads 32 x 100,000 characters in full (apps/api/src/git-provider/workspace.test.ts:487).

Suggest restoring a read cap in the loader (kill the process once exceeded, keep the prefix, set state: "truncated"); the builder already renders truncated bodies with the correct mode-specific notice, so nothing downstream needs to change:

const maxCharacterCount = 8_001; // FILE_CHARACTER_LIMIT + 1
while (content.length < maxCharacterCount) {
    const { value, done } = await reader.read();
    if (done) {
        content += decoder.decode();
        break;
    }
    content += decoder.decode(value, { stream: true });
}
if (content.length >= maxCharacterCount) proc.kill();

@proval-0e6d07

proval-0e6d07 Bot commented Oct 12, 2026

Copy link
Copy Markdown

Thanks for the continued work on this — this push simplifies the guidance API and extends it beyond the review pipeline.

a. Summary

buildAgentInstructionPrompt now takes just (workspace, activityId) and derives PR-vs-branch mode from workspace.isPullRequest() (apps/api/src/agent/pull-request/prompt/agents-md.ts:14), selecting provenance wording and overflow notices per mode and fetching changed files internally. The issue open/reply flows and both PR reply flows now adopt the builder (apps/api/src/agent/issue/open.service.ts:45), tests were updated for the new load-everything semantics, and one main issue remains — the loader-side size cap this push removed.

b. Good points

  • 🟢 Mode selection is fully derived from workspace.isPullRequest() with matching "merge target head" vs "checked out branch" wording (apps/api/src/agent/pull-request/prompt/agents-md.ts:14), so all five call sites are two-argument calls with no way to pass a stale changed-file list.
  • 🟢 The builder now enforces the 8,000-character per-file cap end to end — Math.min(FILE_CHARACTER_LIMIT, Math.max(0, remainingCharacterCount)) plus surrogate-pair preservation (apps/api/src/agent/pull-request/prompt/agents-md.ts:67), with overhead reserved before body allocation — so the prompt stays within its 32,000-character budget regardless of loader behavior.
  • 🟢 The new branch-mode test pins the non-PR contract end to end: root guidance included, nested guidance and "PR head" phrasing absent (apps/api/src/git-provider/workspace.test.ts:466).

Main Issues

  • ⚫️ apps/api/src/git-provider/workspace.ts:408 — this push removes the loader's 8,001-character cap and early proc.kill(), so each of up to 32 guidance blobs is now decoded in full (plus a full-copy content.trim()) while the builder only ever uses 8,000 characters per file; see inline comment for the fix.

Additional Notes

  • apps/api/src/agent/issue/open.service.ts:52 — the system array is joined without .filter(Boolean), so an empty builder result leaves one stray trailing newline (cosmetic only).

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.

Load AGENTS.md as system prompt when launching review agents

2 participants