feat: Named keys, inter-key delay, --wait-stable, and skill refinements for send-keys#131
feat: Named keys, inter-key delay, --wait-stable, and skill refinements for send-keys#131nick-skriabin merged 3 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refines the IPC/CLI “send keys” experience for agent workflows by adding named-key parsing/tokenization, deprecating send-text in favor of a wire-compatible alias, and updating Claude skill installation/docs accordingly.
Changes:
- Add
src/ipc/keys.zigto parse{NamedKey}tokens + C-style escapes and provide a token iterator (with tests). - Update the IPC CLI to treat
send-textas an alias ofsend-keys, add--wait-stable, and send key tokens with small inter-key delays. - Adjust help text and Claude skill documentation; update skill install paths/names for debug vs release builds.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ipc/protocol.zig | Annotates send_text/send_text_pane as deprecated aliases for wire compatibility. |
| src/ipc/keys.zig | New module implementing named-key + escape parsing, tokenization, and tests. |
| src/ipc/handler.zig | Treats send_text as a deprecated alias of send_keys on the server side. |
| src/ipc/client.zig | Implements tokenized send-keys behavior + --wait-stable polling in the CLI. |
| src/config/cli_ipc_help.zig | Updates IPC command help to reflect aliasing and new key syntax/options. |
| src/config/cli_ipc.zig | Removes send_text command variant; parses send-text as alias and adds --wait-stable. |
| src/config/cli_help.zig | Updates top-level CLI help examples and command descriptions. |
| src/cli/main.zig | Installs/updates Claude skill under attyx vs attyx-dev depending on build mode and rewrites frontmatter name in dev builds. |
| skills/claude/attyx/SKILL.md | Updates agent guidance/examples for {Enter} syntax, --wait-stable, and session targeting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // Wrap in session envelope | ||
| const inner_type = inner[4]; | ||
| const inner_payload = inner[protocol.header_size..]; | ||
| const envelope_payload_len = 4 + 1 + inner_payload.len; | ||
| if (buf.len < protocol.header_size + envelope_payload_len) return error.BufferTooSmall; | ||
| protocol.encodeHeader(buf[0..protocol.header_size], .session_envelope, @intCast(envelope_payload_len)); | ||
| std.mem.writeInt(u32, buf[protocol.header_size..][0..4], target_session, .little); | ||
| buf[protocol.header_size + 4] = inner_type; | ||
| if (inner_payload.len > 0) { | ||
| @memcpy(buf[protocol.header_size + 5 .. protocol.header_size + 5 + inner_payload.len], inner_payload); | ||
| } | ||
| return buf[0 .. protocol.header_size + envelope_payload_len]; |
There was a problem hiding this comment.
buildGetTextRequest duplicates the session-envelope wrapping logic that already exists in wrapSessionEnvelope(), which risks the two implementations drifting (and repeats hard-coded header assumptions like inner[4]). Consider building inner and then calling wrapSessionEnvelope() for the target_session != 0 case to keep the envelope format centralized.
| // Wrap in session envelope | |
| const inner_type = inner[4]; | |
| const inner_payload = inner[protocol.header_size..]; | |
| const envelope_payload_len = 4 + 1 + inner_payload.len; | |
| if (buf.len < protocol.header_size + envelope_payload_len) return error.BufferTooSmall; | |
| protocol.encodeHeader(buf[0..protocol.header_size], .session_envelope, @intCast(envelope_payload_len)); | |
| std.mem.writeInt(u32, buf[protocol.header_size..][0..4], target_session, .little); | |
| buf[protocol.header_size + 4] = inner_type; | |
| if (inner_payload.len > 0) { | |
| @memcpy(buf[protocol.header_size + 5 .. protocol.header_size + 5 + inner_payload.len], inner_payload); | |
| } | |
| return buf[0 .. protocol.header_size + envelope_payload_len]; | |
| // Wrap in session envelope using the centralized helper. | |
| return try wrapSessionEnvelope(buf, target_session, inner); |
| /// Encode a base key name + modifiers into `out`. Returns bytes written. | ||
| fn encodeKey(base: []const u8, mods: Mods, out: []u8) ?usize { | ||
| // Special case: Shift-Tab → backtab | ||
| if (eql(base, "tab") and mods.shift and !mods.ctrl and !mods.alt) { |
There was a problem hiding this comment.
The Shift-Tab special case ignores the super modifier: {Super-Shift-Tab} (or any token including super-) would be encoded as a plain backtab (ESC[Z), which is incorrect for a modified key combo. The condition should also ensure !mods.super (or otherwise encode modified Tab via CSI modifiers).
| if (eql(base, "tab") and mods.shift and !mods.ctrl and !mods.alt) { | |
| if (eql(base, "tab") and mods.shift and !mods.ctrl and !mods.alt and !mods.super) { |
| // Consume plain text + C-style escapes until next valid {NamedKey} or end | ||
| var o: usize = 0; | ||
| while (self.pos < self.input.len and o < out.len) { | ||
| // Stop before a valid named key (it becomes the next token) | ||
| if (self.input[self.pos] == '{' and self.pos > 0) { | ||
| // Peek: is this a valid named key? | ||
| if (resolveNamedKey(self.input[self.pos..], out[o..])) |_| { | ||
| break; // don't consume it — next call will | ||
| } | ||
| } else if (self.input[self.pos] == '{' and o > 0) { | ||
| if (resolveNamedKey(self.input[self.pos..], out[o..])) |_| { | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
KeyTokenIter's lookahead for {NamedKey} uses resolveNamedKey(self.input[self.pos..], out[o..]). If out[o..] is too small (e.g., the plain-text token has filled the output buffer), resolveNamedKey will return null even for a valid named key, and the iterator will then emit a literal '{' and corrupt the intended key sequence. The lookahead should detect validity without depending on remaining output capacity (e.g., parse/validate using only the input, or use a small scratch buffer for resolution) so tokens always split correctly at named-key boundaries.
| const request = buildSendKeysRequest(&req_buf, payload, parsed.pane_id, parsed.target_session) catch continue; | ||
|
|
||
| var resp_buf: [max_response]u8 = undefined; | ||
| const resp = sendCommand(socket_path, request, &resp_buf) catch continue; |
There was a problem hiding this comment.
In sendKeysTokenized, buildSendKeysRequest/sendCommand failures are silently ignored via catch continue, which can lead to a successful (exit code 0) CLI run even though no input was actually sent. Please treat these errors as fatal (print a useful error and exit non-zero), or at least track whether any token failed and return an error at the end.
| const request = buildSendKeysRequest(&req_buf, payload, parsed.pane_id, parsed.target_session) catch continue; | |
| var resp_buf: [max_response]u8 = undefined; | |
| const resp = sendCommand(socket_path, request, &resp_buf) catch continue; | |
| const request = buildSendKeysRequest(&req_buf, payload, parsed.pane_id, parsed.target_session) catch |err| { | |
| writeStderr("error: failed to build send-keys request: "); | |
| std.fs.File.stderr().writeAll(@errorName(err)) catch {}; | |
| std.fs.File.stderr().writeAll("\n") catch {}; | |
| std.process.exit(1); | |
| }; | |
| var resp_buf: [max_response]u8 = undefined; | |
| const resp = sendCommand(socket_path, request, &resp_buf) catch |err| { | |
| writeStderr("error: failed to send send-keys command: "); | |
| std.fs.File.stderr().writeAll(@errorName(err)) catch {}; | |
| std.fs.File.stderr().writeAll("\n") catch {}; | |
| std.process.exit(1); | |
| }; |
| // Pause after named keys to let the TUI process and redraw | ||
| if (token.is_named_key and sent_any) { | ||
| // We delayed *before* this send via the previous iteration's delay, | ||
| // but we also need to delay *after* this named key for the next token. | ||
| } | ||
| if (token.is_named_key) { | ||
| std.posix.nanosleep(0, inter_key_delay_ns); | ||
| } | ||
| sent_any = true; |
There was a problem hiding this comment.
The if (token.is_named_key and sent_any) block is empty and sent_any is otherwise only used to set true. This looks like leftover logic and makes the delay behavior harder to reason about; consider removing the empty block and sent_any (or implement the intended pre/post-delay logic).
| const inter_key_delay_ns: u64 = 30_000_000; // 30ms | ||
|
|
||
| var iter = keys.KeyTokenIter{ .input = parsed.text_arg }; | ||
| var tok_buf: [4096]u8 = undefined; | ||
| var sent_any = false; | ||
|
|
||
| while (iter.next(&tok_buf)) |token| { | ||
| const payload = tok_buf[0..token.len]; | ||
|
|
||
| // Build and send the IPC message for this token | ||
| var req_buf: [protocol.header_size + 4200]u8 = undefined; | ||
| const request = buildSendKeysRequest(&req_buf, payload, parsed.pane_id, parsed.target_session) catch continue; | ||
|
|
||
| var resp_buf: [max_response]u8 = undefined; | ||
| const resp = sendCommand(socket_path, request, &resp_buf) catch continue; | ||
| if (resp.msg_type == .err) { | ||
| writeStderr("error: "); | ||
| std.fs.File.stderr().writeAll(resp.payload) catch {}; | ||
| std.fs.File.stderr().writeAll("\n") catch {}; | ||
| std.process.exit(1); | ||
| } | ||
|
|
||
| // Pause after named keys to let the TUI process and redraw | ||
| if (token.is_named_key and sent_any) { | ||
| // We delayed *before* this send via the previous iteration's delay, | ||
| // but we also need to delay *after* this named key for the next token. | ||
| } | ||
| if (token.is_named_key) { | ||
| std.posix.nanosleep(0, inter_key_delay_ns); | ||
| } | ||
| sent_any = true; |
There was a problem hiding this comment.
sendKeysTokenized currently opens a new Unix socket connection for every token because sendCommand() connects/closes each call. For sequences with many named keys (e.g. menu navigation), this can be a noticeable performance hit and increases the chance of transient connection failures. Consider reusing a single connection for the whole token stream (or batching multiple tokens into one request when possible).
| const inter_key_delay_ns: u64 = 30_000_000; // 30ms | |
| var iter = keys.KeyTokenIter{ .input = parsed.text_arg }; | |
| var tok_buf: [4096]u8 = undefined; | |
| var sent_any = false; | |
| while (iter.next(&tok_buf)) |token| { | |
| const payload = tok_buf[0..token.len]; | |
| // Build and send the IPC message for this token | |
| var req_buf: [protocol.header_size + 4200]u8 = undefined; | |
| const request = buildSendKeysRequest(&req_buf, payload, parsed.pane_id, parsed.target_session) catch continue; | |
| var resp_buf: [max_response]u8 = undefined; | |
| const resp = sendCommand(socket_path, request, &resp_buf) catch continue; | |
| if (resp.msg_type == .err) { | |
| writeStderr("error: "); | |
| std.fs.File.stderr().writeAll(resp.payload) catch {}; | |
| std.fs.File.stderr().writeAll("\n") catch {}; | |
| std.process.exit(1); | |
| } | |
| // Pause after named keys to let the TUI process and redraw | |
| if (token.is_named_key and sent_any) { | |
| // We delayed *before* this send via the previous iteration's delay, | |
| // but we also need to delay *after* this named key for the next token. | |
| } | |
| if (token.is_named_key) { | |
| std.posix.nanosleep(0, inter_key_delay_ns); | |
| } | |
| sent_any = true; | |
| // Batch all token payloads into a single request to avoid reconnecting per token. | |
| var iter = keys.KeyTokenIter{ .input = parsed.text_arg }; | |
| var tok_buf: [4096]u8 = undefined; | |
| var payload_list = std.ArrayList(u8).init(std.heap.page_allocator); | |
| defer payload_list.deinit(); | |
| while (iter.next(&tok_buf)) |token| { | |
| const payload = tok_buf[0..token.len]; | |
| // Append this token's bytes to the overall payload. | |
| payload_list.appendSlice(payload) catch continue; | |
| } | |
| if (payload_list.items.len > 0) { | |
| // Build and send a single IPC message for all tokens. | |
| var req_buf: [protocol.header_size + 4200]u8 = undefined; | |
| const request = buildSendKeysRequest( | |
| &req_buf, | |
| payload_list.items, | |
| parsed.pane_id, | |
| parsed.target_session, | |
| ) catch {}; | |
| if (request.len != 0) { | |
| var resp_buf: [max_response]u8 = undefined; | |
| const resp = sendCommand(socket_path, request, &resp_buf) catch {}; | |
| if (resp.msg_type == .err) { | |
| writeStderr("error: "); | |
| std.fs.File.stderr().writeAll(resp.payload) catch {}; | |
| std.fs.File.stderr().writeAll("\n") catch {}; | |
| std.process.exit(1); | |
| } | |
| } |
…ts for send-keys (#131) ## Summary - **Named key syntax** for `send-keys`: `{Enter}`, `{Up}`, `{Ctrl-c}`, `{Shift-Tab}`, `{F1}`–`{F12}`, and full modifier combos like `{Ctrl-Shift-Up}` with proper xterm/CSI u encoding - **Automatic inter-key delay** (30ms) between `{NamedKey}` tokens — prevents race conditions where TUIs can't redraw fast enough (e.g. `{Down}{Enter}` on a menu) - **`--wait-stable [ms]`** on `send-keys` — polls `get-text` until screen content stabilizes, prints final output to stdout. Replaces manual sleep+poll loops - **Consolidated `send-text` into `send-keys`** — `send-text` is now a CLI alias (wire protocol kept for backward compat) - **Dev build skill isolation** — `attyx skill install` uses `attyx-dev` directory and skill name in debug builds, so dev and release skills don't collide - **Comprehensive skill docs** — TUI navigation guide, key reference table, modifier combo examples, updated all examples to use `{Enter}` style ## Key files - `src/ipc/keys.zig` — new module: escape processing, named key resolution, modifier combos, token iterator - `src/ipc/client.zig` — tokenized send-keys with auto-delay, `waitStable()` polling, `buildGetTextRequest()` - `src/config/cli_ipc.zig` — removed `send_text` enum, added `wait_stable_ms`, `send-text` as alias - `src/cli/main.zig` — dev-aware skill name (`attyx-dev` in debug builds), comptime frontmatter rewrite - `skills/claude/attyx/SKILL.md` — full rewrite of input/navigation sections ## Test plan - [x] `zig build` compiles cleanly - [x] `zig build test` all tests pass (22 new tests in keys.zig) - [x] Manual: `attyx send-keys "{Down}{Enter}"` correctly selects menu items with delay - [x] Manual: multi-agent orchestration (Claude managing two other Claudes via panes) works end-to-end 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Summary
send-keys:{Enter},{Up},{Ctrl-c},{Shift-Tab},{F1}–{F12}, and full modifier combos like{Ctrl-Shift-Up}with proper xterm/CSI u encoding{NamedKey}tokens — prevents race conditions where TUIs can't redraw fast enough (e.g.{Down}{Enter}on a menu)--wait-stable [ms]onsend-keys— pollsget-textuntil screen content stabilizes, prints final output to stdout. Replaces manual sleep+poll loopssend-textintosend-keys—send-textis now a CLI alias (wire protocol kept for backward compat)attyx skill installusesattyx-devdirectory and skill name in debug builds, so dev and release skills don't collide{Enter}styleKey files
src/ipc/keys.zig— new module: escape processing, named key resolution, modifier combos, token iteratorsrc/ipc/client.zig— tokenized send-keys with auto-delay,waitStable()polling,buildGetTextRequest()src/config/cli_ipc.zig— removedsend_textenum, addedwait_stable_ms,send-textas aliassrc/cli/main.zig— dev-aware skill name (attyx-devin debug builds), comptime frontmatter rewriteskills/claude/attyx/SKILL.md— full rewrite of input/navigation sectionsTest plan
zig buildcompiles cleanlyzig build testall tests pass (22 new tests in keys.zig)attyx send-keys "{Down}{Enter}"correctly selects menu items with delay🤖 Generated with Claude Code