Repository navigation
[AI-7080] Add snapshot flow inputs - #25180
luisorofino wants to merge 4 commits into
Conversation
evalya-impact-summaryevalya impact analysis |
🎉 All green!🧪 All tests passed 🔄 Datadog retried 1 test - 1 passed on retry 🎯 Code Coverage (details) 🔗 Commit SHA: e891d43 | Docs | View more details | Give us feedback! |
luisorofino
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ce4daff1b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 844d4578e5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
d5e5a66 to
ff30482
Compare
Flow inputs can now declare `snapshot: true`. Such an input is validated at launch like an `as_content` one but reaches the flow as a path rather than as inlined text, and the file it names is copied into the run directory before any phase starts. Every phase, task, and goal reviewer in a run resolves the same read-only copy, so the input cannot change underneath a run. Resuming reuses the copy the run started with instead of re-reading the original, and `inputs/manifest.yaml` records the source path, digest, and size so a run can be traced back to what it was launched with. Flow resolution rejects a phase that consumes a snapshot input when its agent does not declare the `read_file` tool, which would otherwise leave the agent holding a path it cannot open.
ff30482 to
cafaeda
Compare
cafaeda to
61f4305
Compare
AAraKKe
left a comment
AAraKKe
left a comment
There was a problem hiding this comment.
note: Reviewed the snapshot capture, launch/resume wiring, and reader-tool validation. The focused existing tests passed (7 in the wiring selection, then all 11 snapshot-helper tests). Two blocking requests remain: the private-file permissions issue, with a reproduction added to the existing thread, and the case-insensitive filename collision below. Both reproduced at this head.
| path = captures_dir / f"{name}{source.suffix}" | ||
| path.unlink(missing_ok=True) | ||
| path.write_bytes(content) |
There was a problem hiding this comment.
request: Snapshot inputs named Spec and spec are both accepted, but with .md sources their destinations are the same file on a case-insensitive macOS or Windows volume. The second capture unlinks and replaces the first, so both runtime paths read the second input. I reproduced this on macOS with source contents A and B: first.samefile(second) returned True and both copies read B, while the manifest retained distinct source hashes. Please reject colliding names before capture or use filenames that remain distinct on these filesystems so one input cannot silently replace another.
|
This pull request has been automatically marked as stale because it has not had activity in the past 15 days. It will be closed in 30 days if no further activity occurs. If this pull request is still relevant, adding a comment or pushing new commits will keep it open. Also, you can always reopen the pull request if you missed the window. Thank you for your contributions! |
Validation ReportAll 22 validations passed. Show details
|
What does this PR do?
Adds a
snapshotflag to flow inputs, alongside the existingas_content. A snapshot input is validated at launch exactly as anas_contentone is — it must exist, be a regular file, and decode as UTF-8 — but it converts to a canonical absolute path instead of the file's text. It is rejected on a non-path input, on a multi input, on an object field, and in combination withas_content.Before any phase runs, the orchestrator copies every snapshot input into the run directory at
inputs/files/<name><suffix>, marks the copy read-only, and repoints the input's runtime variable at it. Each run also gets aninputs/manifest.yamlrecording the original path, sha256, size, and capture time.Resuming reuses the copy the run started with rather than re-reading the original. The resume dialog shows an already-captured input read-only — its recorded source and capture time — instead of collecting the path again, so a run restarts even if that file has since been edited, moved, or deleted. Divergence between the copy and the recorded source is detected and logged; PR 3 surfaces it in the TUI.
Flow resolution now rejects a phase that consumes a snapshot input when its agent does not declare the
read_filetool. Without that check, such an agent would silently receive a path it cannot open — no error, just worse output.Nothing declares a snapshot input yet, so this PR changes no behavior on its own.
Motivation
AI-7080.
Togo inlines the full PRD text into every worker prompt and goal-review prompt that references it, so a large PRD repeatedly consumes context across a run. Handing agents a path instead would fix that, but pointing them at the user's original file would make a resumed run depend on something that may have changed since it started — different phases could then work from different versions of the same requirements.
Capturing the file once per run gives the token saving without that cost: every phase and reviewer resolves the same immutable copy, and the run keeps enough provenance to identify what it was launched with.
The mechanism is a general input mode rather than a special case for the PRD, so any flow can declare a path input it wants pinned for the lifetime of a run.
Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged