feat(agent): attach to the canonical PTY by ID - #465
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 019f1547d2
ℹ️ 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".
| let runtime_id = { | ||
| let _catalog_lock = st2::CatalogLock::shared(&root) |
There was a problem hiding this comment.
Hold the catalog snapshot through the attach handoff
The shared catalog lock is scoped only around discovery and is released before the readiness wait and pty exec. If a publish or catalog apply changes/removes the selected task during that window—especially while the command waits up to 30 seconds—the old runtime ID can be reassigned and adopted by another agent, causing this exact-ID command to attach to the wrong live terminal. Retain the snapshot fence through the handoff or reacquire it and revalidate the selected task and effective PTY root immediately before exec.
Useful? React with 👍 / 👎.
019f154 to
09031f6
Compare
agent-identity: dev3.direct.omp.2ahzpbs3 agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.1.2 agent-runtime: OMP 18.1.2 tooling-profile: dotfiles@7534055
09031f6 to
ff3bfe5
Compare
|
Closing in favour of the address-first landing: st2#468 → #467, per dotfiles decisions Q23–Q25 (seat dev3.direct.omp.v6c4mkm2) and the two audits (StackMinimal, StackAnatomy): the #453 problem statement is solved by a mutable Posted on behalf of @schickling
|
Problem
Callers that create an Agent currently have to rediscover its PTY runtime ID and reproduce readiness logic before they can open the session. That duplicates internal runtime-ID and PTY-root rules and can attach the terminal to the wrong registry.
Goal
Add one transparent exact-ID handoff:
st2 agent attach --id <AGENT-ID> [--host H] [--wait SECONDS].Decisions
agentPTY task and reuse the runner's shared runtime-ID derivation.--waitonly for advanced direct use.ptythe same effective PTY registry, including an intentional ambientPTY_ROOToverride.pty attach --force; PTY owns terminal input, signals, detach behavior, restart behavior, and exit status.Verification
nix develop --offline -c cargo test --offline --test agent_attach— 11 passed.nix develop --offline -c cargo test --offline --workspace --no-run— passed.nix build .#checks.x86_64-linux.wasm-resolver-feature --no-link --print-build-logs— passed.--waitdeadlines; bounded re-review returned go.Full
nix flake checkbuilt 21 of 22 checks and failed only the existing load-sensitivetests/resync.rs::an_address_change_keeps_the_subscription_and_its_delivery_target. The attach diff does not touch its call graph. The test's unacknowledgedResyncSupervisor::refreshcan seed an already-written carrier as its silent baseline under parallel load. The focused test and its exact Nix check passed immediately in isolation. The durable test defect is recorded in Agent Feedback; this PR does not hide it with a longer timeout or unrelated resync change.Complexity
The command is intentionally a narrow resolve → wait → exec path. The only shared refactor promotes the existing runtime-ID helper so attach, reconciliation, evaluation, and inventory cannot drift.
Concerns
The default wait is a bounded readiness window, not a lifecycle controller. A timeout changes no state and directs the operator to inspect
st2 tasks --jsonor runst2 up.References
Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile