Skip to content

representing guardian review timeouts in protocol types#17381

Merged
won-openai merged 1 commit intomainfrom
guardian_timeout_fix
Apr 11, 2026
Merged

representing guardian review timeouts in protocol types#17381
won-openai merged 1 commit intomainfrom
guardian_timeout_fix

Conversation

@won-openai
Copy link
Copy Markdown
Collaborator

@won-openai won-openai commented Apr 10, 2026

Summary

  • Add TimedOut to Guardian/review carrier types:
    • ReviewDecision::TimedOut
    • GuardianAssessmentStatus::TimedOut
    • app-server v2 GuardianApprovalReviewStatus::TimedOut
  • Regenerate app-server JSON/TypeScript schemas for the new wire shape.
  • Wire the new status through core/app-server/TUI mappings with conservative fail-closed handling.
  • Keep TimedOut non-user-selectable in the approval UI.

Does not change runtime behavior yet; emitting TimeOut and parent-model timeout messaging will come in followup PRs

Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

💡 Codex Review

if ev.status != GuardianAssessmentStatus::Denied {
return;

P1 Badge Render timed-out guardian reviews in the chat timeline

TimedOut is now mapped from app-server notifications, but on_guardian_assessment still exits early for every terminal state except Denied. This drops timed-out reviews from history (the footer is cleared, but no terminal cell is added), and makes the new ReviewDecision::TimedOut history rendering path effectively unreachable.

ℹ️ 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".

Copy link
Copy Markdown
Collaborator

@owenlin0 owenlin0 left a comment

Choose a reason for hiding this comment

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

codex flagged this:

For GuardianAssessmentStatus::TimedOut, this live app-server path completes the command item as Declined, but ThreadHistoryBuilder::handle_guardian_assessment maps the same timed-out event to CommandExecutionStatus::Failed. Users who reload or replay the thread will see a different command status than live clients for the same timeout. Split the timed-out arm or make both mappings use the same status so live and replay agree.

but otherwise lgtm!

@won-openai won-openai force-pushed the guardian_timeout_fix branch 3 times, most recently from 6ec70d8 to 95e1bb6 Compare April 11, 2026 01:25
@won-openai won-openai force-pushed the guardian_timeout_fix branch from 95e1bb6 to 3d99d75 Compare April 11, 2026 01:34
@won-openai won-openai merged commit 37aac89 into main Apr 11, 2026
35 of 38 checks passed
@won-openai won-openai deleted the guardian_timeout_fix branch April 11, 2026 03:02
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants