Skip to content

Add direct CLI attach#168

Merged
almogdepaz merged 4 commits into
mainfrom
pr/herdr-e-direct-cli-attach
Jul 11, 2026
Merged

Add direct CLI attach#168
almogdepaz merged 4 commits into
mainfrom
pr/herdr-e-direct-cli-attach

Conversation

@almogdepaz

Copy link
Copy Markdown
Owner

Summary

  • add direct CLI attach support
  • add attach helpers and unit/integration coverage

Source plan: .plans/e-direct-cli-attach.md

Verification

  • cherry-picked Ralph task commit onto fresh branch from main
  • removed accidentally committed .plans task artifact from the PR branch

@almogdepaz almogdepaz left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ran review passes against main...pr/herdr-e-direct-cli-attach:

  • edc-review security/adversarial
  • edc-delivery-review goal + architecture fit
  • edc-audit quality/maintainability
  • antipattern-scan changed-region smell pass

verdict

  • security: no new bypass found; attach stays through authenticated server /ws/pty rather than direct broker access.
  • delivery/architecture: delivered.
  • quality/antipatterns: no blocking findings.

notes

  • raw mode is gated behind interactive stdin/stdout checks and has an idempotent restorer.
  • viewer conflict defaults to non-destructive exit and requires --take-control to displace.
  • prefill/live bytes come from the existing WS path, so this does not create a second terminal-history source.
  • tests cover parsing, target resolution, close-code mapping, ws url jwt construction, handshake, resize, input, prefill, and default conflict refusal.

recommendation

approve from review standpoint. manual TTY smoke against a real broker is still worth doing before release, as the plan notes.

@almogdepaz
almogdepaz merged commit 28654f1 into main Jul 11, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant