Repository navigation
fix(flows): shorter approval TTL for copilot live-run parks - #5112
Conversation
flows_build's streaming (copilot pane) path already scopes APPROVAL_CHAT_CONTEXT + AgentTurnOrigin::WebChat so run_flow / resume_flow_run park for a real approval instead of auto-allowing (PR tinyhumansai#5090). Those parks were using the gate's full 10-minute DEFAULT_APPROVAL_TTL, which is a long time to leave a live Slack/Gmail/HTTP node waiting on a copilot pane the user may have already navigated away from. Add a new APPROVAL_COPILOT_STREAM_CONTEXT task-local, scoped by flows_build's streaming branch alongside the existing chat context, and a COPILOT_APPROVAL_TTL (180s) constant. The gate reads the new task-local the same way it already reads APPROVAL_IN_CALL_CONTEXT for live meetings, and clamps the park window through a new resolve_park_ttl helper (extracted from the existing in-call min/clamp logic so it's unit testable without parking + waiting out a real TTL). Only flows_build's streaming branch scopes the new context, so main-chat and every other approval path keep the unmodified 10-minute default.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesCopilot approval TTL
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant FlowCanvasCopilot
participant flows_build
participant ApprovalGate
FlowCanvasCopilot->>flows_build: Start streaming run
flows_build->>ApprovalGate: Scope copilot streaming context
ApprovalGate->>ApprovalGate: Resolve clamped park TTL
ApprovalGate->>ApprovalGate: Persist matching expiry
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![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: 48f0c780fa
ℹ️ 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".
| let effective_ttl = | ||
| Self::resolve_park_ttl(self.effective_ttl(), in_call_ctx.is_some(), copilot_stream); |
There was a problem hiding this comment.
When a copilot approval is parked, the wait is clamped here, but the PendingApproval inserted above still uses expires_at = now + self.effective_ttl() (the old 10-minute window). Since approval_decide/expire_stale trust the persisted expires_at, a core restart while the copilot request is pending, or a timeout/decide race at the 3-minute boundary, can leave the row actionable for the old window instead of the intended copilot TTL. Please compute the resolved TTL before building the pending row and use the same value for both the DB expiry and the timeout wait.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/approval/gate.rs`:
- Around line 2510-2584: Add an integration regression test alongside the
existing approval-gate tests that scopes APPROVAL_COPILOT_STREAM_CONTEXT while
exercising intercept_audited_inner, then inspect the persisted pending request’s
expires_at to verify the copilot TTL is applied and resolve the request to
complete the flow. Reuse the existing test fixtures, persistence accessors, and
request-resolution helpers rather than testing resolve_park_ttl directly.
- Around line 973-987: Update the approval flow around PendingApproval
construction and Self::resolve_park_ttl so effective_ttl is computed before the
pending row is created, use that clamped duration when setting expires_at, and
reuse the same value for the subsequent timeout instead of calling
self.effective_ttl() earlier.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eadbbdf8-8456-47b4-818e-2b98d24628a1
📒 Files selected for processing (3)
src/openhuman/approval/gate.rssrc/openhuman/approval/mod.rssrc/openhuman/flows/ops.rs
|
Review feedback addressed in |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git show --stat --oneline 1f1487ac8
git show --format=fuller --find-renames --find-copies -- \
1f1487ac8 -- \
src/openhuman/approval/gate.rs \
src/openhuman/approval/mod.rs \
src/openhuman/flows/ops.rs
ast-grep outline src/openhuman/approval/gate.rs --items all --match 'resolve_park_ttl|intercept_audited_inner|APPROVAL_COPILOT_STREAM_CONTEXT'
ast-grep outline src/openhuman/flows/ops.rs --items all --match 'flows_build'
rg -n -C 5 \
'APPROVAL_COPILOT_STREAM_CONTEXT|resolve_park_ttl|DEFAULT_APPROVAL_TTL|COPILOT_APPROVAL_TTL' \
src/openhuman/approval/gate.rs src/openhuman/approval/mod.rs src/openhuman/flows/ops.rsLength of output: 34193
🐇 ✅ |
|
Addressed in
Verified: |
|
To use Codex here, create a Codex account and connect to github. |
|
Tip For best results, initiate chat on the files or code changes.
Using the single resolved TTL for both 🐇 ✅ ✏️ Learnings added
|
coderabbitai
Bot
added
the
rust-core
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f1487ac83
ℹ️ 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".
| APPROVAL_CHAT_CONTEXT.scope( | ||
| chat_ctx, | ||
| APPROVAL_COPILOT_STREAM_CONTEXT.scope((), agent.run_single(&prompt)), |
There was a problem hiding this comment.
In a Flow Canvas copilot turn where the builder calls composio_connect, this wraps the entire agent.run_single in APPROVAL_COPILOT_STREAM_CONTEXT, not just run_flow/resume_flow_run. composio_connect is on the workflow_builder belt and manually calls ApprovalGate::intercept_audited_bounded (src/openhuman/composio/tools.rs:923-943); after its 120s bound it deliberately returns while leaving the pending row open so a later Connect-card click can resolve it (src/openhuman/composio/tools.rs:927-964). With this broad scope, that row's expires_at is clamped to 180s, so OAuth/card completion after three minutes is expired/denied instead of remaining actionable for the normal approval TTL. Please scope the copilot task-local only around live-run tool approvals or exempt bounded inline-connect approvals.
Useful? React with 👍 / 👎.
1 participant
Summary
flows_build's streaming (copilot pane) path already scopesAPPROVAL_CHAT_CONTEXT+AgentTurnOrigin::WebChat(PR #5090) sorun_flow/resume_flow_runpark for a real human approval instead of auto-allowing. Those parks were using the gate's full 10-minuteDEFAULT_APPROVAL_TTL(src/openhuman/approval/gate.rs:65) — a long time to leave a live Slack/Gmail/HTTP node waiting on a copilot pane the user may have already navigated away from.This reuses the existing "shorten a specific park" mechanism already established for live meetings (
IN_CALL_APPROVAL_TTL/APPROVAL_IN_CALL_CONTEXT, issue #3513) rather than adding a parallel path:COPILOT_APPROVAL_TTLconstant (180s / 3 min) insrc/openhuman/approval/gate.rs.APPROVAL_COPILOT_STREAM_CONTEXTtask-local, scoped byflows::ops::flows_build's streaming branch alongside the existingAPPROVAL_CHAT_CONTEXTscope (src/openhuman/flows/ops.rs).APPROVAL_IN_CALL_CONTEXT, and clamps the park window through a newApprovalGate::resolve_park_ttlhelper — extracted from the existing in-call min/clamp logic so the clamp-selection is unit testable without actually parking a future and waiting out a multi-minute TTL.DEFAULT_APPROVAL_TTLitself is untouched, and no new global constant replaces it.Only
flows_build's streaming branch scopes the new context, so main-chat and every other approval path (external channel, in-call meetings, workflow automation, CLI, composio's ownpark_bound) keep their existing behavior unmodified.Test plan
GGML_NATIVE=OFF cargo check --manifest-path Cargo.toml— passes.GGML_NATIVE=OFF cargo test --lib --manifest-path Cargo.toml approval::gate— 47 passed (including 5 newresolve_park_ttl_testscovering: default park unclamped, copilot clamp shortens a 10-minute default, in-call clamp unaffected by the new flag, a clamp never extends an already-shorter boot-time TTL, and the tighter-of-two-clamps case).GGML_NATIVE=OFF cargo test --lib --manifest-path Cargo.toml openhuman::flows::ops— 183 passed, no regressions.cargo fmt --checkon changed files — clean.Summary by CodeRabbit
expires_at) is now aligned with the actual clamped wait duration to reduce stale waits.