Skip to content

Store guardian transcript boundary on review session - #15261

Open
charley-oai wants to merge 5 commits into
mainfrom
guardian-parent-history-boundary
Open

Store guardian transcript boundary on review session#15261
charley-oai wants to merge 5 commits into
mainfrom
guardian-parent-history-boundary

Conversation

@charley-oai

@charley-oai charley-oai commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • store the parent transcript checkpoint on the cached guardian review session instead of rollout reconstruction or parent session state
  • slice guardian transcript evidence from that checkpoint so follow-up reviews only include transcript since the last terminal guardian review
  • preserve the checkpoint when the cached guardian trunk is recreated for a config change and add coverage for approved and denied follow-up reviews

@charley-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8d9ff1943f

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/core/src/guardian/prompt.rs Outdated
Comment thread codex-rs/core/src/guardian/review_session.rs
@charley-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown
Contributor

Codex Review: Didn't find any major issues. 🎉

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@charley-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown
Contributor

Codex Review: Didn't find any major issues. 🎉

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

charley-oai and others added 5 commits March 24, 2026 10:26
Keep the parent transcript checkpoint on the cached guardian review session so follow-up guardian prompts only inject transcript evidence since the last terminal guardian review.

Also preserve that checkpoint across guardian trunk replacement and add tests for approved and denied follow-up reviews.

Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
@charley-oai
charley-oai force-pushed the guardian-parent-history-boundary branch from ece1018 to be7bc5c Compare March 24, 2026 17:26
@charley-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: be7bc5c9ae

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/core/src/guardian/review_session.rs

@Hardanish-Singh Hardanish-Singh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@Raymondriter

Copy link
Copy Markdown

I'd close this as superseded rather than request a rebase.

The follow-up transcript slice this PR wanted is already on main as GuardianTranscriptCursor + GuardianPromptMode::{Full, Delta}, stored on the review session (last_reviewed_transcript_cursor) and copied through fork snapshots. Main also version-checks against parent history (so compaction/rewrite falls back to a full prompt), keeps root authorization out of the delta, and after #41215 resets the cursor when reviewer compaction drops the original transcript.

This branch is from March (be7bc5c, last code Mar 24) and is mergeable_state: dirty against current main. The design here is a raw raw_items().len() index on the session manager, which is weaker than what landed and has its own holes even on that snapshot:

  • Sequential same-turn reviews (two tools, no history append) slice [len..] and send <no retained transcript entries>. Tests only cover follow-up after a new user turn.
  • Abort / timeout / fail-closed still call mark_terminal_guardian_review_boundary, so a review that never assessed the current history still hides it from the next one.
  • Only replace_history clears the checkpoint; replace_compacted_history is untouched.
  • Config-change trunk recreate keeps the parent slice while spawning with initial_history: None, so the new reviewer loses both prior assessments and original user authorization. Main starts a new trunk in Full mode instead.

CI was green on Mar 24 against then-main; that does not apply now. No maintainer review (the Aug 1 LGTM is unaffiliated). Please reopen from current main only if some delta-cursor case is still missing. I don't think this patch is the vehicle.

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.

3 participants