Repository navigation
docs(ssh): state the SSH execution boundary and pin the liveness vocabulary - #14971
Merged
Merged
Conversation
…bulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughAdded an SSH execution-boundary reference that defines execution ownership, disconnect behavior, verdict terminology, observability limits, verification artifacts, implementation gaps, and execution-model selection. Updated SSH guidance to require execution-host ownership and use 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bf233ef6-5eb5-4281-af93-012fe8eea9cb
📒 Files selected for processing (3)
.gitignoreAGENTS.mddocs/reference/ssh-execution-boundary.md
Included review availability: Your plan includes up to 10 reviews per rolling hour; 9 remain after this review.
The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/reference/ssh-execution-boundary.md (1)
72-72: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not label open PRs as fixed.
Line 72 calls these issues fixed while stating that the PRs are open. An open PR does not change the behavior in the referenced baseline.
Use wording that describes proposed fixes until the changes have merged.
Proposed wording
-Fixed (PRs open): +Fixes proposed in open PRs:
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f291c3ef-e76e-4ab0-b1f2-18fa47fb0509
📒 Files selected for processing (1)
docs/reference/ssh-execution-boundary.md
Included review availability: Your plan includes up to 10 reviews per rolling hour; 1 remains after this review.
Comment on lines
+34
to
+38
| By default, remote work survives your machine going away. The relay is a detached daemon (`nohup … </dev/null &`), its handler in `src/relay/relay.ts` ignores `SIGHUP`, the PTY is its child rather than the ssh channel's, and quitting Orca is a **detach, not a dispose** (`src/main/ssh/ssh-relay-session.ts:901-915`). Sleep additionally pushes `graceTimeSeconds: 0` to un-bound any running grace window. | ||
|
|
||
| Two ways remote work _can_ actually stop: | ||
|
|
||
| - **A bounded grace period.** The shipped default is `0` = keep alive until reset. If "keep terminals alive until reset" is unchecked, the configurable range is **60s–7d** and the form defaults to **24h**. The countdown starts when the client disconnects, after which the relay SIGKILLs every PTY. Note the asymmetry: sleep protects you, but ordinary disconnect and app quit do not. There is currently **no command that reports which setting is in effect for a target** — see Known gaps. |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Correct the grace-period asymmetry.
Line 34 says quitting Orca detaches without disposing remote work. Line 38 says the shipped graceTimeSeconds: 0 keeps terminals alive until reset. The final sentence says ordinary disconnect and app quit do not protect remote work, which contradicts those statements.
State that sleep overrides a finite grace window. State that ordinary disconnect and app quit use the configured grace period.
Proposed wording
-Note the asymmetry: sleep protects you, but ordinary disconnect and app quit do not.
+Sleep overrides a finite grace window by setting `graceTimeSeconds: 0`.
+Ordinary disconnect and app quit use the configured grace period.
+The shipped default `0` keeps terminals alive until reset.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| By default, remote work survives your machine going away. The relay is a detached daemon (`nohup … </dev/null &`), its handler in `src/relay/relay.ts` ignores `SIGHUP`, the PTY is its child rather than the ssh channel's, and quitting Orca is a **detach, not a dispose** (`src/main/ssh/ssh-relay-session.ts:901-915`). Sleep additionally pushes `graceTimeSeconds: 0` to un-bound any running grace window. | |
| Two ways remote work _can_ actually stop: | |
| - **A bounded grace period.** The shipped default is `0` = keep alive until reset. If "keep terminals alive until reset" is unchecked, the configurable range is **60s–7d** and the form defaults to **24h**. The countdown starts when the client disconnects, after which the relay SIGKILLs every PTY. Note the asymmetry: sleep protects you, but ordinary disconnect and app quit do not. There is currently **no command that reports which setting is in effect for a target** — see Known gaps. | |
| By default, remote work survives your machine going away. The relay is a detached daemon (`nohup … </dev/null &`), its handler in `src/relay/relay.ts` ignores `SIGHUP`, the PTY is its child rather than the ssh channel's, and quitting Orca is a **detach, not a dispose** (`src/main/ssh/ssh-relay-session.ts:901-915`). Sleep additionally pushes `graceTimeSeconds: 0` to un-bound any running grace window. | |
| Two ways remote work _can_ actually stop: | |
| - **A bounded grace period.** The shipped default is `0` = keep alive until reset. If "keep terminals alive until reset" is unchecked, the configurable range is **60s–7d** and the form defaults to **24h**. The countdown starts when the client disconnects, after which the relay SIGKILLs every PTY. | |
| Sleep overrides a finite grace window by setting `graceTimeSeconds: 0`. | |
| Ordinary disconnect and app quit use the configured grace period. | |
| The shipped default `0` keeps terminals alive until reset. There is currently **no command that reports which setting is in effect for a target** — see Known gaps. |
|
|
||
| > When the client disconnects, every `orca …` command run on the SSH host fails with `No owning Orca client is connected to the relay`. The PTY stays `live`; its control plane does not. | ||
|
|
||
| Orchestration state (Runs, Tasks, Dispatches, mailboxes) is client-resident for the same reason. An agent on an SSH host should not depend on `orca` for anything it must finish while you are away. **Commit and push early** — unpushed work on a remote box is unavailable to the client until it reconnects. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Qualify the push recommendation.
Line 49 tells agents to commit and push early. Lines 81-83 state that SSH hosts may lack git identity, SSH agent forwarding, provider API keys, and gh authentication. A push may therefore fail even when remote execution succeeds.
Change the guidance to make host credentials a condition.
Proposed wording
-**Commit and push early** — unpushed work on a remote box is unavailable to the client until it reconnects.
+**Commit early. Push when host credentials are available** — unpushed work on a remote box is unavailable to the client until it reconnects.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Orchestration state (Runs, Tasks, Dispatches, mailboxes) is client-resident for the same reason. An agent on an SSH host should not depend on `orca` for anything it must finish while you are away. **Commit and push early** — unpushed work on a remote box is unavailable to the client until it reconnects. | |
| Orchestration state (Runs, Tasks, Dispatches, mailboxes) is client-resident for the same reason. An agent on an SSH host should not depend on `orca` for anything it must finish while you are away. **Commit early. Push when host credentials are available** — unpushed work on a remote box is unavailable to the client until it reconnects. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 798715b8-597d-47c6-960d-7b188133b79d
📒 Files selected for processing (1)
docs/reference/ssh-execution-boundary.md
Included review availability: Your plan includes up to 10 reviews per rolling hour; 2 remain after this review.
| - **A bounded grace period.** The shipped default is `0` = keep alive until reset. If "keep terminals alive until reset" is unchecked, the configurable range is **60s–7d** and the form defaults to **24h**. The countdown starts when the client disconnects, after which the relay SIGKILLs every PTY. Note the asymmetry: sleep protects you, but ordinary disconnect and app quit do not. There is currently **no command that reports which setting is in effect for a target** — see Known gaps. | ||
| - **Host-acknowledged explicit user action** — End Remote Terminals, Reset Relay, removing the target, or closing the tab. When the host cannot acknowledge the request, closing a tab or removing a target may clear only client state; the remote verdict remains `unverifiable`. | ||
|
|
||
| Reconnect re-attaches to the same live PTYs and replays a bounded buffer (`REPLAY_BUFFER_MAX`, a 102,400-code-unit tail). Output beyond that while you were away is lost to the client even though the process was never interrupted: **the transcript is truncated; the work stays `live`.** |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make same-PTY reattachment conditional on lease recovery.
Line 41 states that reconnect always re-attaches to the same live PTYs. getSshPtyConsumerRecovery can return null for a sealed lease, and the Known gaps section documents duplicate cold starts while the original PTY may remain live. Qualify this statement so readers do not treat a newly started PTY as the original execution.
Proposed wording
-Reconnect re-attaches to the same live PTYs and replays a bounded buffer (`REPLAY_BUFFER_MAX`, a 102,400-code-unit tail). Output beyond that while you were away is lost to the client even though the process was never interrupted: **the transcript is truncated; the work stays `live`.**
+When lease recovery succeeds, reconnect re-attaches to the same live PTYs and replays a bounded buffer (`REPLAY_BUFFER_MAX`, a 102,400-code-unit tail). Output beyond that while you were away is lost to the client even though the process was never interrupted: **the transcript is truncated; the work stays `live`.**
+If lease recovery fails, the client may cold-start a duplicate agent while the original PTY remains `live`; see Known gaps.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Reconnect re-attaches to the same live PTYs and replays a bounded buffer (`REPLAY_BUFFER_MAX`, a 102,400-code-unit tail). Output beyond that while you were away is lost to the client even though the process was never interrupted: **the transcript is truncated; the work stays `live`.** | |
| When lease recovery succeeds, reconnect re-attaches to the same live PTYs and replays a bounded buffer (`REPLAY_BUFFER_MAX`, a 102,400-code-unit tail). Output beyond that while you were away is lost to the client even though the process was never interrupted: **the transcript is truncated; the work stays `live`.** | |
| If lease recovery fails, the client may cold-start a duplicate agent while the original PTY remains `live`; see Known gaps. |
Contributor
Author
Review verdict: land after fixesI reviewed the full change and pushed accuracy fixes through Validation:
Electron evidence and residual risk:
This screenshot is pre-connect state after restarting the client. After clicking Connect, backend logs showed a successful relay reconnect, but renderer-target CDP commands stalled and further screenshots timed out; therefore the real remote-workspace/PTY disconnect-and-reconnect UI leg was blocked and was not observed as passing. Mobile QA was not applicable because no mobile-facing surface changed. |
The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by #14974, #14977 and #14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots.
Resolve the .gitignore allow-list conflict by keeping all three SSH reference docs: main added ssh-host-key-verification and ssh-reconnect-source-recovery while this branch adds ssh-execution-boundary. Dropping either side would silently untrack a doc.
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
…ecord The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land.
paidaxingyo666
pushed a commit
to paidaxingyo666/Manta
that referenced
this pull request
Aug 21, 2026
…bulary (stablyai#14971) * docs(ssh): state the SSH execution boundary and pin the liveness vocabulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore. * docs(ssh): cite the live restoreRequired site after it moved The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on. * docs(ssh): require host evidence for liveness verdicts * docs(ssh): keep boundary references stable * docs(ssh): fence liveness evidence to its host identity * docs(ssh): state replay and environment boundaries precisely * docs(ssh): correct replay and platform boundary claims * docs(ssh): describe headless runtime continuity accurately * docs(ssh): distinguish authority from client metadata * docs(ssh): describe pending fixes accurately * docs(ssh): date the gap list and name the PR that closes each entry The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by stablyai#14974, stablyai#14977 and stablyai#14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots. * docs(ssh): make the boundary doc a durable ruleset, not an incident record The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land.
dallascrilley
pushed a commit
to dallascrilley/orca
that referenced
this pull request
Aug 27, 2026
…bulary (stablyai#14971) * docs(ssh): state the SSH execution boundary and pin the liveness vocabulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore. * docs(ssh): cite the live restoreRequired site after it moved The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on. * docs(ssh): require host evidence for liveness verdicts * docs(ssh): keep boundary references stable * docs(ssh): fence liveness evidence to its host identity * docs(ssh): state replay and environment boundaries precisely * docs(ssh): correct replay and platform boundary claims * docs(ssh): describe headless runtime continuity accurately * docs(ssh): distinguish authority from client metadata * docs(ssh): describe pending fixes accurately * docs(ssh): date the gap list and name the PR that closes each entry The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by stablyai#14974, stablyai#14977 and stablyai#14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots. * docs(ssh): make the boundary doc a durable ruleset, not an incident record The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land.
enrick-dev
pushed a commit
to enrick-dev/orca
that referenced
this pull request
Oct 15, 2026
…bulary (stablyai#14971) * docs(ssh): state the SSH execution boundary and pin the liveness vocabulary Nothing under docs/ described how work splits between the client and an SSH host, so agents and humans inferred it from error strings and got it wrong: loss of contact was repeatedly reported as process death, which orphaned live remote agents and cold-started duplicates over the same worktree. Pins the vocabulary to the incumbent live/unverifiable/exited verdict from unstopped-pty-verification so no synonym is introduced, records the one real discriminator (all of a host's terminals drop together on link loss; one alone means process exit), and lists the outstanding gaps with citations. Tracked via the docs allow-list and linked from AGENTS.md, per the convention in .gitignore. * docs(ssh): cite the live restoreRequired site after it moved The throw now lives in reattachSshPtySessionForSpawn; ssh-pty-provider.ts no longer contains it. Caught by the worker fixing it, against a newer main than the audit ran on. * docs(ssh): require host evidence for liveness verdicts * docs(ssh): keep boundary references stable * docs(ssh): fence liveness evidence to its host identity * docs(ssh): state replay and environment boundaries precisely * docs(ssh): correct replay and platform boundary claims * docs(ssh): describe headless runtime continuity accurately * docs(ssh): distinguish authority from client metadata * docs(ssh): describe pending fixes accurately * docs(ssh): date the gap list and name the PR that closes each entry The Known gaps section was accurate when written and becomes actively misleading as its fixes land: it told a reader to go fix restoreRequired, the missing unverifiable verdict, and the absent terminal-list host field, three things now addressed by stablyai#14974, stablyai#14977 and stablyai#14973. Mark the section as dated, require verification against current code before acting on any entry, name the PR per entry, and move landed items out. Also correct the two body claims that the landed fixes invalidated. The rules above are durable; only this section rots. * docs(ssh): make the boundary doc a durable ruleset, not an incident record The Known gaps section was 18 of 93 lines enumerating specific defects from one investigation, several already fixed by sibling PRs in the same batch. A reference doc that needs a 'this section rots' warning is telling you the section belongs somewhere else; those entries belong in issues. Replace the six-row table of currently-lying signals with the method that outlands any particular bug: ask whether the owning host produced the signal, whether every PTY on the target went quiet together, whether the termination event matches the current incarnation and generation, and whether a returned status is actually a claim. Same for artifacts - state what ls-remote and a PR head each do and do not prove, rather than listing which command is currently wrong. Nothing here goes stale when the open fixes land.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Nothing under
docs/described how work splits between the client and an SSH host. Agents and humans inferred it from error strings and got it wrong in a consistent direction: loss of contact was reported as process death. That orphaned live remote agents and cold-started duplicates over the same worktree.Measured, not assumed: in a blind survey, two independent agents (Claude Opus 5 and a Codex model) were given the real signals from a disconnected SSH worker and both concluded the worker had died and the work was lost — one at 90% confidence, citing five concurring signals. All five are loss-of-contact signals. Both reversed after reading this document.
What this adds
live/unverifiable/exited, adopted from the incumbentUnstoppedPtyVerdict(src/main/runtime/unstopped-pty-verification.ts:12-16) rather than introducing a synonym.exitedrequires positive evidence of absence from the owning host.Notes for review
docs/**allow-list in.gitignoreand linked fromAGENTS.md, which is the convention that file documents. Note!docs/reference/relay-grace-time-reconfiguration.mdis already allow-listed but was never written — that is the dangling target of the citation atsrc/relay/pty-handler.ts:492.finallyinsrc/main/ipc/ssh.ts:1393-1399carries an explicit rationale that the doc now concedes and argues against only in the narrow case it does not cover, and a decaying "enforced ~33 times" count was replaced by naming the guard.Related
Three boundary fixes already open: #14945, #14947, #14948. Three more in flight for the
restoreRequiredmislabel, false-success stop/close reporting, and host-scoped terminal listing.SSH / Electron QA evidence
Validated the runtime claims on the pre-review PR head (
fd993c8a54) through an isolated Linux-arm64 Docker SSH target with a real remote repository. The rendered terminal created a file, showed remote git state, and ran a long-lived process; after an explicit disconnect, host-side file/git/process checks remainedlive, and the rendered Connect action reattached the same PTY and accepted commands.pnpm build:relayand the focused liveness/grace tests passed.The target, app profile, test process, and remote marker were cleaned up. Half-open TCP, app restart, and replay-buffer truncation remain untested; this run covered explicit disconnect/reconnect with a 300-second bounded grace period.