Repository navigation
Load AGENTS.md into agent context - #77
Conversation
|
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 b. Good points
Main Issues
Additional Notes
|
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.
|
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
b. Good points
Main Issues None for this push. Additional Notes
|
|
Hi @NriotHrreion it's been a while and I love this concept of feature. Agent really needs Here's what I thought about
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 |
|
@seoes Thanks for reviewing this! Here's my thoughts:
|
|
Oh, I mean
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 |
|
@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.
|
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: b. Good points
Main Issues None for this push. Additional Notes
|
- 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
|
Hi @NriotHrreion! Great work. I added small changes (might be look big but it's just some details)
I'll merge it now. thanks for your hard work |
| const decoder = new TextDecoder("utf-8", { ignoreBOM: true }); | ||
| let content = ""; | ||
| try { | ||
| while (true) { |
There was a problem hiding this comment.
⚫️ [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();|
Thanks for the continued work on this — this push simplifies the guidance API and extends it beyond the review pipeline. a. Summary
b. Good points
Main Issues
Additional Notes
|
close #76
Loading rule of AGENTS.md
AGENTS.mdfirst. If noAGENTS.md, findagents.md.untrusted. Its contents remain review material, including after later tool reads.What changed
apps/api/src/agent/pull-request/prompt/agents-md.tsfor loading AGENTS.md.regularFileOnlyoption inWorkspace.read().Notes
These fixed prompts use up to approximately 1700 tokens per request. (Read 32 files, each file hit character limit of 8000)