Conversation
Let read-only publication and review sessions inspect their captured change without requesting shell access. Restrict listing, chunk reads, and literal search to the verified invocation manifest, with bounded responses and explicit failures for absent or modified evidence. Closes #16 Co-authored-by: Codex <noreply@openai.com> Agent-Workflows-Run: 905089e8-f83b-4b66-919a-71daf7030455
mingchuno
left a comment
mingchuno
left a comment
There was a problem hiding this comment.
Review of 3a8dd2f
The evidence was fully readable and hash-verified. Four actionable issues remain: truncated searches misreport coverage, worst-case search work is insufficiently bounded, the central large-change publication flow lacks an acceptance test, and review-stage registration is not actually tested.
| return this.bounded({ | ||
| term, | ||
| limit, | ||
| searchedChanges: changes.filter((change) => !change.binary).length, |
There was a problem hiding this comment.
searchedChanges always reports the total number of non-binary changes, even when the loop stops after reaching the match limit and later changes were never searched. This can falsely imply complete coverage and undermine honest limitation reporting. Track the number of changes/chunks actually scanned and report the unsearched remainder; add a regression where the first of multiple changes exceeds the limit.
| for (const change of changes) { | ||
| signal?.throwIfAborted(); | ||
| if (change.binary || change.chunks.length === 0) continue; | ||
| const parts = await Promise.all( |
There was a problem hiding this comment.
Search reads every chunk of a change concurrently, concatenates the entire change, and repeatedly copies/splits its prefix for each match. At the supported 32 MiB limit this can fan out roughly 512 reads and copy about 1.6 GiB across 50 late matches, risking resource pressure or the timeout this feature is intended to prevent. Process chunks sequentially with bounded overlap and incremental line tracking.
| assert.ok(messages.includes("captured evidence")); | ||
| }); | ||
|
|
||
| test("Copilot read-only stages expose bounded evidence tools and retain tool failures", async () => { |
There was a problem hiding this comment.
The requested high-level controlled-Copilot publication test is missing. This test invokes handlers directly over a one-chunk hand-built manifest and returns finished; it never demonstrates a session navigating captured multi-chunk evidence and producing schema-valid publication JSON without shell approval. Add an invocation-level large-change fixture that performs list/search/read calls and returns a valid publication response.
| assert.equal(calls, 1); | ||
| }); | ||
|
|
||
| test("only Copilot publication and review receive captured evidence tools", async () => { |
There was a problem hiding this comment.
Despite its name, this test only exercises publication; it never sets the stage name to review. Automatic review-stage evidence registration therefore lacks coverage. Add a review case asserting the evidence object, application-owned prompt instruction, and resulting tool registration while preserving the Codex exclusion.
This branch has not been deployed
1 participant
Summary
Why
Large Copilot publication and review changes previously lacked a bounded way to inspect captured evidence without requesting shell access. These tools expose the evidence already owned and verified by the runner, allowing the agent to navigate it without broadening read-only stage permissions or introducing a second source of truth.
Closes #16.
Validation
pnpm verifypassedLimitations
Refs #16