Conversation
![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: c161bc3462
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Adds a controller-side, terminal-only attach path for controlled sessions so external recorders (asciinema 3.x) can capture terminal I/O while the structured broker lifecycle remains intact.
Changes:
- Introduces
reploy controlled-session attach --socket PATH, forwarding stdin/stdout bytes and resize events, and switching TTY input to raw mode with restoration. - Validates and dials only the broker-created private Unix socket (path grammar + filesystem properties + peer credentials).
- Pins an asciinema 3.2.1 Linux amd64 fixture in CI and adds an integration test proving the recorder finalizes its cast while the broker remains available for
complete/ack.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| testdata/controlled-session/asciinema-v3-linux-amd64.json | Declares pinned asciinema asset URL + SHA-256 for CI/test fixture verification. |
| internal/controlledsession/terminal_attachment.go | Core attachment loop: forward input/output, handle resize/events, manage connection lifetime. |
| internal/controlledsession/terminal_attachment_unsupported.go | Non-Linux stub returning a clear unsupported error. |
| internal/controlledsession/terminal_attachment_test.go | Cross-platform unit tests for forwarding semantics, ordering, restore, and disconnect/finalization behavior. |
| internal/controlledsession/terminal_attachment_linux.go | Linux implementation: socket validation, peer identity checks, raw TTY + SIGWINCH resize wiring. |
| internal/controlledsession/terminal_attachment_linux_test.go | Linux tests for broker-socket acceptance and rejection of arbitrary socket paths. |
| internal/controlledsession/controller_terminal_unsupported_test.go | Extends non-Linux coverage to include attachment failure behavior. |
| internal/controlledsession/asciinema_fixture_linux_test.go | Linux integration test validating pinned asciinema binary and recorder/broker lifecycle behavior. |
| internal/cli/controlled_session_test.go | CLI help/usage updates + attach dispatch/error-channel behavior tests. |
| internal/cli/cli.go | Adds controlled-session attach command parsing and dispatch with interrupt cancellation. |
| docs/CONTROLLED_SESSION_DESIGN.md | Updates design doc “implementation status” to reflect attach + CI pinning and test coverage. |
| .github/workflows/ci.yml | Downloads pinned asciinema fixture, verifies checksum, exports env var for integration test. |
| .changes/unreleased/+controlled-session-terminal-attachment.yaml | Adds a Changie “Added” fragment for the new attach command. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Add the Reploy-owned terminal-only attach command for controlled sessions. Validate the broker-created socket, switch attached terminals to raw mode with restoration, and forward exact input, output, and resize events under the frozen exit contract. Pin asciinema 3.2.1 by release asset and checksum in CI. Prove an unmodified recorder closes its cast while the long-lived broker remains available for controller finalization, result delivery, and acknowledgement.
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Add the Reploy-owned terminal-only attach command for controlled sessions. Validate the broker-created socket, switch attached terminals to raw mode with restoration, and forward exact input, output, and resize events under the frozen exit contract.
Pin asciinema 3.2.1 by release asset and checksum in CI. Prove an unmodified recorder closes its cast while the long-lived broker remains available for controller finalization, result delivery, and acknowledgement.