feat: phase 2 — LLM module + statusbar hint/error slots + atty init#5
Merged
Conversation
Reserves a separate slot for short, transient prose that lives one row above the status text (`rows - 1`). Mechanism: - `setHint(text, ttl_ms)` — paint into the hint row, auto-clear after the TTL. - `clearHint()` — erase immediately + force a repaint. The hint row uses the existing blank-padding row that `reserve_rows = 2` (default) already keeps above the status text, so no DECSTBM resizing is required. When `reserve_rows < 2` the hint is silently dropped — there's nowhere to draw it. Style is inherited from the bar's `style` for now; a dedicated hint style can land later if it turns out to be visually confusing. Phase 2 of the LLM module will populate this slot with a one-line explanation of the command being injected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The LLM now returns BOTH a one-sentence summary of what the
command does AND the command itself, in a single round-trip:
<explanation>
```
<command>
```
atty parses the response into the two halves: the command lands
in the user's prompt (as before) and the explanation flashes in
the new statusbar hint row above the status text.
Wiring:
- `Config.with_explanation: bool = true` — opt-out for users who
want the old terse "just a command" response.
- `effective_system_prompt` resolves at comptime: explicit
override → user's prompt; otherwise the canned prompt for the
selected mode.
- `extractResponse(body, cmd_out, exp_out) -> ExtractedResponse`
splits the JSON `"content"` field at the markdown fence,
feeding the inner block to `sanitizeCommand` and the prose
above to the new `sanitizeExplanation` (flattens newlines,
strips control bytes, coalesces whitespace). Falls back to
command-only when no fence is found, so a model that ignores
the format still works.
- Worker carries both halves through `RequestResult` → `Shared`
→ `Runtime`. `pollShellInput` latches the explanation into a
one-shot `hint_buf`; the new `provideHintText` hook surfaces
it on the next tick, returning null thereafter.
- Dispatcher gains a `gatherHintText` walker (same first-non-null
pattern as `gatherGhostText`).
- Proxy calls `gatherHintText` per tick after `pollShellInput`
and pushes the result into `statusbar.setHint(text,
config.statusbar.hint_ttl_ms)` (default 30s; set to 0 to
disable).
`extractCommand` stays for back-compat (refactored to share the
new `decodeContent` helper internally).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`Config.context_env_vars: []const []const u8` lets the user pick
which environment variables the LLM should see alongside the
prompt. Each named var is read at attach time; the set entries
become a single comma-separated `KEY=value, KEY2=value2` line
appended to the user message:
Generate a bash command to: list files
Context: PATH_BASE=/opt/foo, PROJECT=acme
Empty default — no context. The canonical example is `PATH_BASE`,
which most projects use as a "what does 'here' mean for this
user" anchor (build outputs, CI scratch dirs, …).
Implementation:
- `resolveContextEnv(allocator)` builds the blob at attach time;
unset / empty vars are skipped silently.
- `Runtime.context_blob` owns the resolved bytes; freed in detach.
- Worker threads through `doRequest` to `buildRequestBody`, which
splices the blob into the user message only when non-empty (no
empty "Context: " line when nothing's configured).
Tests:
- `buildRequestBody appends a context blob when provided`
- `buildRequestBody omits the context section entirely when blob is empty`
Cwd / git-root context deferred — both need child-PID tracking
(or OSC 7) for accuracy as the shell `cd`s during a session;
landing the env-var path first because it's static and reliable.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Building with `zig build -Doptimize=ReleaseSafe` against the
current Zig 0.16 std lib failed:
src/modules/llm.zig:537:25: error: member function expected
0 argument(s), found 1
`std.http.Client.deinit` reads `io` off the receiver now (see
lib/std/http/Client.zig:1308). The unit tests didn't catch this
because `doRequest` is private and not transitively referenced
by any pure-helper test path.
This is also present on master — fixed here on phase 2 because
that's the active branch; will surface as part of the phase 2 PR.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the first integration-level test for the LLM module's worker
thread + HTTP path. Previously coverage stopped at the pure
helpers (`extractResponse`, `sanitizeCommand`, `buildRequestBody`)
— `doRequest` and `client.fetch` were unexercised, which is how
the `std.http.Client.deinit` signature drift slipped past CI.
The test:
- Stands up a raw libc TCP listener on 127.0.0.1, OS-picked port
(std.Io.net's Server has no portable getLocalAddress accessor
in 0.16, and std.posix dropped socket/bind/listen into Io.net's
vtable — going direct keeps the test stable across further
std API churn).
- Spawns a single-shot mock handler thread that reads the request
headers and replies with a canned OpenAI-shape JSON
(`"Greets you.\n\`\`\`\necho hi\n\`\`\`"`).
- Sets `$ATTY_TEST_LLM_API_BASE` to the bound URL, configures
the module to read from that custom env var, attaches.
- Drives `onInput` with `#: hello` + Enter, asserts the
`.replace = "\\x15"` (Ctrl+U) action fires.
- Polls `pollShellInput` in a 2s-budgeted loop, asserts the
worker delivers `"echo hi"` and that `provideHintText` returns
`"Greets you."`.
- Bypasses `detach`'s `t.detach()` path (which is intentionally
leaky for production exit) and does a sync `shutdown + cv.signal
+ t.join` cleanup so the testing allocator's leak detector
stays happy.
Linux-only by construction (Linux-pinned socket constants); CI
and the dev box are both x86_64-linux-{gnu,musl} per CLAUDE.md.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…son as a hint Previously the module went silent on every failure mode: no endpoint env, connect refused, HTTP non-2xx, unparseable response. The user saw their `#: …` line vanish and… nothing. That looks identical to a working setup that returns a comment. Now each failure path latches a one-line diagnostic into the statusbar's hint slot (the row above the status text, surfaced via `provideHintText`). The user sees *why* the prompt produced no command: - `llm: no endpoint set — export $LLM_API_BASE or $OLLAMA_HOST` Latched synchronously by onInput when api_base is empty. Returns `.forward` instead of `.replace` so the typed prompt reaches the shell as a comment — no harm done, user can edit + retry without retyping. - `llm: request failed (endpoint unreachable?)` Worker's `client.fetch` errored (DNS, connect refused, etc.). - `llm: HTTP <status>` Endpoint returned a non-2xx status. The actual status number is formatted in so 404 (model name wrong) is distinguishable from 500 (ollama crashed mid-request). - `llm: couldn't extract a command from the response` HTTP 200 but `extractResponse` returned cmd_len=0 — model emitted markdown-only or unparseable JSON. Wiring: - `Shared.error_buf: [256]u8, error_len: usize` — worker latches the message alongside the (zero) response. - `RequestResult.err_len` joins `cmd_len` / `exp_len`; `doRequest` writes formatted diagnostics into an `error_out` buffer threaded through from the worker. - `pollShellInput` on the failure path copies the error from shared into the hint slot with an `llm: ` prefix so the user can spot it at a glance. - `latchHint(rt, msg)` — small helper for synchronous hint latching (used by the inert onInput path; reusable for future modules that want to surface their own diagnostics). Tests: - `inert mode (no endpoint env) surfaces a 'no endpoint' hint` — pins the synchronous latch path + `.forward` action. - `HTTP 5xx surfaces a 'HTTP <status>' hint, no command injected` — adds a second mock server that returns 500, asserts the worker latches `HTTP 500` and pollShellInput surfaces it. Test count: 213 → 215. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n slot
The previous transparent-failure commit reused the hint slot for
both explanations and errors, so a successful `qwen3-coder`
response and a `HTTP 500` looked identical. Users couldn't tell
"the model wants you to read this" from "something broke" at a
glance.
Now errors have their own surface:
- New `setError(text, ttl_ms)` / `clearError()` on `StatusBar`,
parallel to the existing `setHint`. Errors paint in `error_style`
(muted red, `dim + fg=1` by default) with a leading ⚠ glyph so
the row reads as a notification rather than informational text.
- Errors take precedence over hints when both are active on the
same row. After the error TTL expires, the suppressed hint
reappears automatically (idempotent repaint logic tracks both
kinds via a `HintKind` enum so transitions repaint without
redundant emit on no-change ticks).
- `config.statusbar.error_style` + `config.statusbar.error_ttl_ms`
expose both knobs; defaults are `.{ .dim = true, .fg = 1 }` and
60s (twice the hint TTL so users have time to read).
Plumbing:
- New module hook `provideErrorText(rt, ctx) !?[]const u8` —
same one-shot first-non-null contract as `provideHintText`,
surfaced via `dispatch.gatherErrorText`. Proxy calls both per
tick after `pollShellInput`.
- LLM module splits its single `hint_buf` latch into separate
`hint_buf` (info, for explanations) and `err_buf` (error, for
diagnostics) with independent `_pending` flags + `latchHint` /
`latchErr` helpers. onInput's inert-mode path now latches via
the error channel, as do the failure paths through pollShellInput.
- Existing inert-mode and HTTP-5xx tests updated to assert the
error channel (not the hint channel) carries the diagnostic.
- New statusbar tests pin: muted-red + ⚠ rendering, error
precedence over hint, hint resurfacing after error expiry.
Test count: 215 → 217.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The env-var lookup pattern (`$LLM_API_BASE` → `$OLLAMA_HOST`) is fragile: atty inherits env at fork time, so a misconfigured `.bashrc` (or running atty from a launcher that strips env) can leave the module silently inert even when the endpoint is reachable. The previous transparent-failure commit makes that state visible, but the user still has to chase env state instead of just declaring the endpoint they want. New `Config.api_base: []const u8 = ""`. When set, wins over both env vars — no shell state in the resolution path. The user's own `src/config.zig` is the right place to anchor this; nothing else on the box can flip it accidentally. Priority order in `resolveApiBase`: 1. `cfg.api_base` (static) — used verbatim 2. `$LLM_API_BASE` (env override) 3. `$OLLAMA_HOST` (Ollama-native fallback, `/v1` appended) Trailing-slash normalisation applies to all three. Tests: - `resolveApiBase priority — static cfg.api_base beats both env vars` - `resolveApiBase priority — env wins when cfg.api_base is empty` - `resolveApiBase trims a single trailing slash on cfg.api_base` Also updates `src/config.zig` (user's local) to use the static form pointing at the local Ollama, since their env vars weren't set and that was the immediate "nothing happens" cause. Test count: 217 → 220. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pattern users already know from atuin / zoxide / direnv:
# .bashrc / .zshrc
eval "$(atty init bash)"
The current shell re-execs under atty with a recursion guard, so
nested invocations no-op (atty's own child shell sees `ATTY=1`
and skips the auto-spawn). Non-interactive shells (scripts, `bash
-c`, ssh non-tty) skip the snippet entirely via `[ -t 0 ] && [ -t
1 ]` so the user's automation isn't surprised.
Generated snippet:
# atty shell integration — drop this in your .bashrc / .zshrc:
# eval "$(atty init bash)"
if [ -z "${ATTY:-}" ] && [ -t 0 ] && [ -t 1 ]; then
export ATTY=1
exec atty
fi
POSIX-compatible — same snippet works for bash and zsh; the shell
argument is currently a no-op accepted for future per-shell
variants (fish, nu).
Parser detail:
- New `ParseOutcome.print_init` variant. `parseArgv` recognises
`init` ONLY when it appears as the first positional with no
prior tokens; everything else (incl. `--` then `init`) is
treated as a shell name to spawn. That preserves the escape
hatch for anyone who genuinely has a shell binary named `init`
(`atty -- init`).
- Usage / `--help` updated to document the subcommand.
Tests:
- `parseArgv: \`init\` is a subcommand that prints integration snippet`
— covers `init`, `init bash`, `init zsh` all yielding .print_init.
- `parseArgv: \`--\` escape lets a user reach a real shell named \`init\``
— pins the escape hatch.
Test count: 220 → 222.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… 133
Two corrections to the integration snippet:
1. `export ATTY=1` removed — atty already injects `ATTY=1` /
`ATTY_PID` / `ATTY_VERSION` into the child shell's env at the
pty.zig:179 setenv calls. Exporting it ourselves in the
pre-exec process is a no-op (we're about to `exec atty`,
which replaces the process anyway) and visually obscures
which side actually owns the marker.
2. Shell argument is now plumbed through: `atty init bash` emits
`exec atty bash` (not just `exec atty`) so the re-exec runs
the same shell the rc file is for, not whatever `$SHELL`
happens to be (which can disagree — e.g. zsh user running a
bash script that sources its bashrc).
3. NEW: OSC 133 prompt markers. The snippet now wires the shell's
prompt to emit `\x1b]133;A/B/C/D` so atty can capture the
user's input region precisely instead of falling back to
keystroke tracking. `src/osc133.zig` already consumes these
markers; the integration was missing the shell-side
emission. Shell-specific output:
- `atty init bash` → PROMPT_COMMAND + PS1 wrap (A/B/D).
- `atty init zsh` → add-zsh-hook precmd + preexec + PS1
suffix (A/B/C/D).
- `atty init` (no shell arg) → dual-branch fallback that
detects $BASH_VERSION / $ZSH_VERSION at runtime.
Parser detail: `ParseOutcome.print_init` becomes a payload
variant carrying the (possibly-empty) shell name; the parser
borrows the caller's argv slice rather than allocating, which is
safe because main.zig keeps the iterated argv alive past the
parser call.
main.zig grows three string-literal snippets plus a small
`emitInitSnippet(shell)` dispatcher that handles the header
formatting and picks the right OSC 133 block.
Tests:
- `parseArgv: \`init\` is a subcommand …` now asserts the
carried shell name (empty / "bash" / "zsh").
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d hint_style
Previously the hint row sat directly above the status text, in
the same style. Visually it read as a second status segment, not
as a distinct annotation. Two changes:
1. Hint paints at the TOP of the reserved region, not adjacent
to the status text. New row math:
hint_row = rows - reserve_rows + 1
With the bumped `reserve_rows = 3` default, that yields:
rows-2: hint / error notification
rows-1: blank padding
rows : status bar
Bumping to 4+ adds more blank padding above the hint;
`reserve_rows = 2` (legacy) keeps the old adjacent layout.
2. New `Config.statusbar.hint_style` knob, defaulting to
`style.presets.muted_italic`. The hint reads in italic + dim
so it visually contrasts with the bar's plain-dim text below.
Override per taste — `.italic = true, .fg = 39` for a soft
cyan, etc. Error notifications keep their own `error_style`
(muted red + ⚠) which already differs from both.
Plumbing:
- `defaults.zig`: `reserve_rows` 2 → 3, `hint_style` added.
- `statusbar.zig`: new `hint_style` field on the struct; new
`initFull(rows, cols, reserve_rows, style, error_style,
hint_style)` constructor (kept `init` / `initWithError` for
back-compat). Render path picks `hint_style` for the .hint
variant (error path is unchanged).
- `proxy.zig`: switches to `initFull` so the user's configured
styles all reach the bar.
New test pins the row math + style precedence:
`hint paints at TOP of the reserved region (gap above status
when reserve_rows >= 3)`.
Test count: 222 → 223.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r) + push prompts to history Three additions that all key off the same observation: the user should know atty recognised their LLM prefix BEFORE hitting Enter, and the prompt itself should be a first-class shell input (history, ghost-suggest, recall) — not vanish into the void. 1. Cursor colour signal (OSC 12 / 112): `Config.prefix_signal_cursor: bool = true` + `Config.prefix_signal_cursor_color: []const u8 = "cyan"`. New module hook `provideTermBytes(rt, ctx) !?[]const u8` — sibling of `pollShellInput` that returns bytes for the user's OUTER terminal (not pty.master). Dispatcher gains `gatherTermBytes`; proxy pushes the result to STDOUT_FILENO each tick. LLM module's `provideTermBytes` emits OSC 12 on the edge-in (prefix first matches) and OSC 112 on edge-out (prefix breaks). Edge-driven — no per-tick OSC spam. 2. Statusbar segment toggle: `Config.prefix_signal_status: bool = true` + `Config.prefix_signal_status_text: []const u8 = "✨ prompt"`. `statusText` returns the signal text while `line.current()` starts with `prefix`. In-flight thinking… spinner still takes precedence. 3. Push `#: …` prompts to history: New Action variant `.replace_commit: []const u8`. Behaves like `.replace` (bytes go to pty.master) but ALSO tells the proxy to fire `dispatchLineCommit` on the pre-replace typed line — even though the replacement doesn't contain Enter. LLM module's onInput now returns `.replace_commit = "\x15"` instead of `.replace = "\x15"`, so atuin / history record the full `#: list files` line. Ghost suggestions surface prior prompts the next time the user starts typing `#: l…` — same recall power as any normal command. Dispatcher detail: the input walker has to PRESERVE `.replace_commit` through later modules' plain `.replace` substitutions (only the bytes get overwritten, the commit decision sticks). Pinned by a new test `dispatchInput preserves .replace_commit across downstream modules` which caught the implementation bug on the first run. Tests added: - `provideTermBytes emits OSC 12 on prefix-match edge, OSC 112 on un-match` — pins edge semantics + colour string baked in. - `statusText flips to prefix_signal_status_text while prefix matches` — pins toggle behaviour + in-flight precedence. - `dispatchInput preserves .replace_commit across downstream modules` — pins the dispatcher's commit-stickiness. Test count: 223 → 226. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ng TOC)
Catches docs up to the phase-2 code. Four files touched:
- docs/llm.md (new) — module-specific guide:
- Quickstart: minimal config tuple + how a `#:` prompt flows
end-to-end (onInput → worker → pollShellInput → review)
- Endpoint resolution (static `api_base` → `$LLM_API_BASE` →
`$OLLAMA_HOST`, with trailing-slash normalisation)
- Context injection via `context_env_vars`
- Transparent failure table (every notification message + the
cause)
- Live signals while typing (cursor colour + status segment)
- History integration (`.replace_commit` so prompts land in
atuin / history → ghost-suggestible)
- Full Config reference table
- Security notes (C0/C1/DEL stripping, CR drop, shell-safe
prefix)
- Shutdown semantics (t.detach + intentional leak)
- docs/modules.md:
- Floating TOC via kramdown's `* TOC\n{:toc}` marker
- Top-of-file hook list updated (pollShellInput,
provideHintText, provideErrorText, provideTermBytes)
- Action semantics: `.replace_commit` row + the dispatcher's
stickiness rule
- New sections:
`provideHintText / provideErrorText` — notifications above
the status bar (hint vs error styles, TTLs, edge-driven
one-shot pattern, example latch helper)
`provideTermBytes` — write to the user's outer terminal
(OSC sequences for cursor / title / palette; example OSC 12
colour transition)
`pollShellInput` — async byte injection into pty.master
(paired with `line_state.applyInput` on the proxy side
so downstream hooks see the injected bytes)
- docs/architecture.md:
- "Status bar + incognito" section: rewrite reserve_rows math
to reflect new default (3 = hint / blank / status), describe
the hint + error notification slots, styling, precedence,
TTL fallback
- "OSC 133 prompt markers" section: shortest path is now
`eval "$(atty init bash)"` which wires the shell-side hooks
- docs/index.md:
- "What's in the box" gains LLM module + Statusbar entries
- Module-framework hook list updated
- "Make it your shell launcher" gets a second path for the
shell-rc-side install via `eval "$(atty init bash)"`
Site layout:
- `docs/_layouts/default.html` adds the `llm` nav link
- `docs/assets/style.css` styles `#markdown-toc` to render
inline by default; on ≥1200px viewports it floats as a sticky
sidebar in the right margin, follows the reader as they
scroll. Auto-generated by kramdown — every doc that emits the
`{:toc}` marker gets the same treatment without per-page CSS.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Phase 2 of the LLM module and supporting UX surfaces: adds statusbar hint/error rows, terminal-side OSC emission, history-commit semantics for replaced input, and a new atty init subcommand + docs updates to support shell integration.
Changes:
- Extend the status bar to support transient hint + error notifications (with TTL, precedence, and styling).
- Expand the module framework + proxy loop with new hooks (
provideHintText,provideErrorText,provideTermBytes) and new.replace_commitaction semantics. - Enhance the LLM module with static endpoint config, context injection, explanation parsing, cursor/status signals, improved failure surfacing, and integration tests; add
atty init [shell]snippet generation + docs refresh.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/statusbar.zig | Adds hint/error buffers, styles, TTL handling, and rendering logic for a notification row above the status text. |
| src/proxy.zig | Wires new dispatcher walkers into the tick loop; supports .replace_commit; forwards terminal-only bytes to stdout. |
| src/modules/llm.zig | Adds static endpoint config, context env injection, explanation parsing + hint/error surfacing, cursor/status signals, and mock HTTP worker tests. |
| src/module.zig | Documents new hooks and introduces .replace_commit action variant. |
| src/main.zig | Adds atty init [shell] CLI surface and emits shell integration snippet with OSC 133 setup. |
| src/dispatch.zig | Preserves .replace_commit through downstream replaces; adds walkers for hint/error/terminal bytes; adds tests. |
| src/defaults.zig | Updates statusbar defaults (reserve rows, hint/error styles + TTLs). |
| src/config.def.zig | Updates example config template with new LLM config fields. |
| src/args.zig | Adds init subcommand parsing and tests. |
| docs/modules.md | Documents new hooks and .replace_commit semantics; adds TOC and updates signatures. |
| docs/llm.md | New end-to-end LLM module guide including signals, failure UX, config reference, and security notes. |
| docs/index.md | Updates homepage feature list + quickstart to mention atty init and new module hooks. |
| docs/assets/style.css | Adds responsive floating right-sidebar TOC styling. |
| docs/architecture.md | Updates architecture docs for OSC 133 integration and new statusbar notification row behavior. |
| docs/_layouts/default.html | Adds LLM page to docs nav. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Six threads, all actionable:
1. Inert-mode error text was hard-coded to `$LLM_API_BASE` /
`$OLLAMA_HOST` literals despite `Config.api_base_env` and
`Config.api_base_fallback_env` being configurable. Now
comptime-built from those field names + a hint about
`Config.api_base` as the static alternative. Test pinned the
message via new assertions against the user-supplied env
names rather than the upstream defaults.
2. `sanitizeExplanation` only stripped C0 / DEL — the same C1
terminal-escape injection the command path was hardened
against (U+009B = CSI via UTF-8 `0xC2 0x9B`) reached the
statusbar's hint row unfiltered. Factored the C1-aware UTF-8
walker out of `sanitizeCommand` into a shared
`stripControlBytes` helper and threaded `sanitizeExplanation`
through it after the whitespace-collapsing pass. New
regression test `sanitizeExplanation strips C1 codepoints —
terminal-escape injection (security)` exercises U+009B in
both raw and UTF-8 forms and pins that legitimate UTF-8
sequences with continuation bytes in the C1 range (e.g.
"ƒ" = 0xC6 0x92) survive intact.
3. `gatherTermBytes` results were written to STDOUT
unconditionally. Non-TTY invocations (CI, piped/redirected
stdout, capture-the-binary integration tests) could leak
OSC sequences into the captured stream. Gated on `args.is_tty`
to match the existing kitty-keyboard push/pop pattern at
proxy startup.
4. `atty init <shell>` pasted the shell name into the emitted
snippet unquoted, making a `eval "$(atty init '; rm -rf ~; echo bash')"`
attack possible (shell-injection primitive). Replaced the
bare paste with an allowlist validator
`args.isSafeShellName(s)` that rejects anything outside
`[A-Za-z0-9_-]{1,32}`. Path-laden names (`/usr/bin/zsh`),
names with spaces, `$`, backticks, semicolons, quotes —
all flunk and fall back to the bare `exec atty` form. New
tests `isSafeShellName accepts the well-known shells` and
`isSafeShellName rejects shell-injection primitives (security)`.
5. `render()` was not idempotent when `reserve_rows < 2`: the
paint was correctly skipped, but `last_hint_valid` was never
updated in the skipped path, so `hint_unchanged` stayed false
forever and every tick re-emitted the save/restore-cursor
pair pointlessly. Moved the tracking-state update outside the
`reserve_rows >= 2` branch so it runs whether or not the
paint happened. New test pins zero new bytes on the
second-render call.
6. Stale doc comments — the original `setHint` / hint-buf prose
said "rows - 1" but the renderer paints at
`rows - reserve_rows + 1` (rows - 2 with the default
reserve_rows = 3). Updated both docstrings to describe the
actual row math.
Test count: 226 → 230.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four actionable + one refuted as false-positive (replied in the
thread):
1. (actionable) Underflow risk on `rows - reserve_rows + 1` when
`rows < reserve_rows` (tiny terminals, misconfigured
reserve). The u16 subtraction wrapped to ~65000 and emitted a
CUP escape for a nonsense row, terminal-dependent mojibake.
Added `self.rows >= self.reserve_rows` to the existing
reserve_rows >= 2 guard. New test
`render guards against u16 underflow when rows < reserve_rows`
pins zero wraparound row values in the output.
2. (false-positive — refuted in the thread) `resolveContextEnv`
does respect "non-empty only" — `envValue` returns null when
`sliceTo(v, 0).len == 0`, and `resolveContextEnv` does
`orelse continue` on that. No code change needed.
3. (actionable) Stray double `)` in the architecture.md status-bar
section. Both wrapping-parenthesis link constructs reworded to
use em-dashes / inline phrasing — the markdown's now valid
under stricter renderers and reads more cleanly.
4. (actionable) `setHint paints text into rows - 1 with the bar's
style` was misleading — the renderer uses `hint_style` (dim
italic by default), not the bar's plain `style`. Renamed to
`setHint paints into the hint row with hint_style (dim by
default)` so failure output is unambiguous.
5. (actionable) `isSafeShellName` doc comment said "Lower-case
ASCII … only" but the predicate accepts A-Z too. Fixed the
comment to match the code ("ASCII letters (both cases) …").
The case-insensitive allowlist is the right call — shell
names are case-sensitive on Linux but realistic binaries
(`bash`, `zsh`, `sh`, `dash`, `fish`, `nu`) are all
lower-case, while staying tolerant covers the corner cases.
Test count: 230 → 231.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four threads, all actionable: 1. `isSafeShellName` accepted leading `-` (e.g. "-h"), which would emit `exec atty -h` in the snippet — atty's argv parser interprets that as --help, prints usage, exits, and breaks the interactive shell that ran the eval. Added an explicit `if (s[0] == '-') return false;` check; mid-name dashes like `bash-fork` still pass. New test `isSafeShellName rejects leading dash — would parse as an atty flag` pins -h / -V / --help / -bash and keeps `bash-fork` as the positive case. 2. `src/module.zig`'s top-of-file hook list still claimed `attach(allocator)` / `detach(rt)`. The dispatcher actually calls `M.attach(allocator, io)` and `M.detach(rt, io)` (and has for a while — the docs/modules.md version was already correct). Fixed the inline signatures to match. 3. `Shared.error_buf`'s field comment said diagnostics surface via `provideHintText`, but the round-1 refactor moved them to the dedicated `provideErrorText` channel (muted red + ⚠). Updated the comment so future maintenance routes errors through the right hook. 4. `detach()`'s thread-detach comment listed the deliberately leaked allocations as shared / api_base / api_key / shell, missing `context_blob` (added in the env-var context commit). The worker captures it too, so it has to leak for the same reason. Updated both the code comment and the `docs/llm.md` shutdown-semantics section to include it. Test count: 231 → 232. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… musl `make test` (and `make itest` / `make e2e`) didn't forward `-Dtarget=$(TARGET)` like `make build` and `make debug` do. Bare `make test` on this dev box hit the gcc-16 SFrame relocation linker issue documented in CLAUDE.md (`R_X86_64_PC64` in /usr/lib/gcc/.../crt1.o) — failing to build the test executable before any tests could run. Two changes: 1. Forward `-Dtarget=$(TARGET)` to `test`, `itest`, `e2e`, and `e2e-update`. Per CLAUDE.md these are supposed to wrap the underlying `zig build … -Dtarget=…` invocations; they regressed at some point and dropped the arg. 2. Default `TARGET` is now Linux-aware: on Linux it's `x86_64-linux-musl` (matches CI, sidesteps the system-libc linker issue), otherwise `native` (macOS / BSD, where the crt1.o problem doesn't apply). `uname -s` does the dispatch at parse time. Override with `make TARGET=… <goal>` for cross-compilation (e.g. `aarch64-linux-musl`). The end-user effect: bare `make test` / `make build` now "just works" on Linux without anyone having to remember the TARGET=… escape hatch. The produced binary is statically linked against musl, which runs on any x86_64-linux box regardless of the host's libc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…usbar constructors Two comment-only fixes: 1. `error_style` field doc said defaults are populated via `init()`, but `init()` doesn't take/assign `error_style` — only `initWithError` / `initFull` do. Rewritten to describe the actual defaulting story: struct default unless the caller threads the config through one of the richer constructors. Same treatment applied to the new `hint_style` field doc, which had the same shape. 2. `initWithError`'s docstring still claimed "Used by proxy.zig…", but proxy switched to `initFull` when phase 2 added `hint_style`. Reworded as a general-purpose variant for callers that want error styling without committing to a hint override (and for tests that isolate error rendering). No behaviour change. Test count unchanged (232). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two threads, both actionable: 1. `resolveContextEnv` appended env var values verbatim despite the docstring claiming a "single-line `KEY=value, …` blob". Env values can legally contain newlines, carriage returns, tabs, NUL bytes — a `PATH_BASE=$'/opt/foo\ntext-after'` value would have split the context line across multiple lines in the JSON-encoded user message, confusing the model and risking prompt-injection-shaped behaviours. Fix: new `writeSanitizedEnvValue` helper collapses any whitespace / control byte (< 0x20 or 0x7F) to a single space, coalesces runs, and caps each value at 256 bytes. Docstring matches actual behaviour again. 2. `parseArgv`'s `.print_init` outcome carried a `shell_name` slice borrowed directly from the caller's argv, while every other variant returned fully-owned (alloc-duped) strings. Inconsistent ownership; the in-code lifetime comment also didn't match main.zig's behaviour (main.zig dupes argv into `collected.items`). Fix: `.print_init` payload is now allocator-owned, duped from the matched argv slot. New `freePrintInit(allocator, shell)` helper for the explicit cleanup path. main.zig still skips the free (process exits immediately after emitting the snippet); the parseArgv tests now `defer freePrintInit` to keep the leak detector happy. Doc comment on the variant explicitly states the ownership contract. No test count change (233) — both are correctness / contract fixes rather than new surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…arlier rounds Three comment-only fixes, all consequences of tightening behaviour in earlier rounds without updating prose: 1. `statusbar.zig`'s hint-row field doc + `setHint` docstring still claimed `rows - reserve_rows + 1`. Round 5 moved the actual math to `effectiveRows() + 1` so notifications keep working when `reserve_rows >= rows`. Rewrote both docs to describe the `effectiveRows() + 1` rule + note that in the common case (`rows > reserve_rows`) it equals the older form. 2. `proxy.zig`'s "explanations render in the bar's regular style" comment was the round-1 wording. Phase-2 hints render in `hint_style` (dim italic) and errors in `error_style` (muted red + ⚠) — both distinct from the bar's plain `style`. Reworded to name the actual fields. 3. `docs/modules.md`'s `.replace_commit` row + accompanying prose read like the action unconditionally forces an `onLineCommit`. The round-5 tightening keyed it on `containsEnter(input)` (the original pre-replace bytes), so a misbehaving module returning `.replace_commit` on a non-Enter keystroke is a no-op for history. Added a "Important caveat" paragraph spelling out the precondition and noting the only behavioural difference vs. plain `.replace` is which slice's Enter test fires. No behaviour change, test count unchanged (233). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
Copilot review loop complete — 7 rounds. Round-by-round comment counts: 6, 5, 4, 2, 2, 2, 3. 24 threads addressed total; 23 fixed, 1 refuted, 0 deferred. PR ready for human review. |
4 tasks
Merged
2 tasks
fentas
added a commit
that referenced
this pull request
May 18, 2026
) `BufReader::lines()` had no length bound — a buggy or hostile local client (still inside the 0600 trust boundary, but inside is still inside, e.g. a browser sandbox running as the same user) could stream an unbounded line and OOM the daemon before serde_json's recursion limit ever kicked in. Switch to `BufReader::take(MAX_LINE_BYTES).read_line(...)` per request and drop the connection on overflow. 64 KiB is well beyond any plausible typed command + context blob. Tests still pass (16/16). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fentas
added a commit
that referenced
this pull request
May 18, 2026
…eat map (#105) * feat(atty-guard): V2-A sidecar daemon — UDS + Tier-1 classifier + threat map Lands the V2-A slice from docs/security-guard-design.md — a separate Rust crate that ships as its own binary alongside atty. Splits the PTY proxy (Zig) from the security daemon (Rust) along the privilege + classifier-deps boundary the design doc lays out: atty - Zig, no privileges, owns the typed-line intent. atty-guard - Rust, will hold CAP_BPF in V2-B + the ONNX SLM in V2-C; this PR ships only the UDS surface + Tier-1 regex classifier so atty has something to talk to. Crate layout (`atty-guard/`): - src/main.rs — argparse, default socket path, listener bring-up. - src/protocol.rs — JSON-line RPC types: Request / ResponseBody / ThreatLevel / Verdict / Category / ClassifyResult. - src/classifier.rs — Tier-1 regex classifier mirroring atty's in-proc patterns (curl|sh, npm install <flagged>, bash -c <long b64>). Tier-2 SLM is stubbed — always returns Safe with 0.0 confidence — until V2-C wires the real model. - src/threat_map.rs — In-memory PID → ThreatLevel map. V2-B will replace the HashMap backing with a libbpf-rs BPF_MAP_TYPE_HASH so the kernel LSM hook reads the same record atty wrote over UDS. - src/server.rs — UDS accept loop + thread-per-connection JSON-line dispatcher. Socket created mode 0600 so co-tenant users can't connect. classify with `context.pid` set escalates to Warn / Block when the PID is in the threat map at High / Critical. - README.md — Build / run / systemd-user unit / protocol spec / security model / roadmap. - .gitignore — target/, Cargo.lock. Deps: serde, serde_json, regex, clap. Release build is ~2 MiB. Tests (cargo test): 16 pass. - 7 classifier unit tests (Tier-1 hits and misses). - 3 threat_map unit tests. - 6 integration tests over real UDS sockets (health, classify-warn, classify-safe, set+get threat level, PID-escalation, malformed JSON). Follow-ups in the V2 series: - V2-B: libbpf-rs LSM hook + ringbuf consumer + real BPF map. - V2-C: ONNX SLM in `classifier::tier2`. - V2-D: atty-side UDS client (separate PR on atty's side). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(atty-guard): cap request line size at 64 KiB (subagent finding #1+#5) `BufReader::lines()` had no length bound — a buggy or hostile local client (still inside the 0600 trust boundary, but inside is still inside, e.g. a browser sandbox running as the same user) could stream an unbounded line and OOM the daemon before serde_json's recursion limit ever kicked in. Switch to `BufReader::take(MAX_LINE_BYTES).read_line(...)` per request and drop the connection on overflow. 64 KiB is well beyond any plausible typed command + context blob. Tests still pass (16/16). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fentas
added a commit
that referenced
this pull request
May 20, 2026
Copilot round 1 finding #5: docs / comments referenced \`atty-guard trust list\` but the CLI subcommand didn't exist. The TrustList RPC was shipped + the atty-side lazy seed was wired in the previous round-1 fix; this commit closes the loop on the user-facing surface. New subcommands: - \`atty-guard trust list\` — prints the caller's persistent trust hashes (sorted). No sudo. SUDO_UID forwarding applies same as other read-only ops. - \`atty-guard trust add <hash>\` — direct insert (mostly for testing / scripting; the typical path is banner [t]). main.rs gains a Trust subcommand variant + TrustOp inner enum; cli_client.rs gains handle_trust_list + maps TrustOp::Add to the existing Request::TrustAdd dispatch. The other 4 Copilot findings (stale docstrings claiming classify-time consultation, dead-code TrustList RPC, atty-side seeding gap, docs mentioning unimplemented surface) were already addressed in a16e237 — that commit's docstring rewrites + lazy seed wiring + uds_client::trustList method covered the overlap.
fentas
added a commit
that referenced
this pull request
May 20, 2026
* feat(security_guard): migrate trust cache to daemon-side Completes the post-#142 trust-state architecture. `[t]rust permanently` keystrokes now mirror into a per-UID commands.trusted.txt under /var/lib/atty-guard/users/<uid>/ in addition to the legacy ~/.cache/atty/security_trust.txt atty-side write. Closes the last asymmetry where atoms / URL decisions / session trust all lived daemon-side but command trust didn't. Daemon (atty-guard): - PerUserState.persistent_trust: HashSet<String> for hex SHA-256 hashes loaded from commands.trusted.txt. - load_persistent now reads commands.trusted.txt too. - persistent_add_trust: validates hash shape (delegates to validate_trust_hash), enforces PERSISTENT_TRUST_CAP=16384 (decades of routine use before any operator hits it), atomic tmp+rename write, reload. - list_persistent_trust(uid): sorted snapshot for atty's startup fetch. - session_write now also drains session_trust into commands.trusted.txt — banner [a]llow always → sudo atty-guard session write → persistent trust. Trust hashes with valid shape go in; invalid ones stay in session for inspection (same semantics as atoms / urls). - SessionWriteReport gains trust_added counter. Protocol: - New TrustAdd { hash, target_uid } Request (NO sudo — banner [t] has always been a non-sudo action since the module's introduction; the per-UID cap is the DOS guard). - New TrustList { target_uid } Request — read-only fetch of persistent trust hashes for atty's startup seed. - New TrustList response variant. atty side: - handleArmedResponse case 't': continues to write ~/.cache/atty/security_trust.txt (backward compat for non-daemon installs), ALSO fires trustAdd UDS call so the hash lands in commands.trusted.txt. - uds_client.zig: new trustAdd(hash) method. - Daemon's classify hot path does NOT consult trust at classify time — atty's in-proc trust set is authoritative for the runtime check (cheap HashSet lookup, no UDS round-trip). The daemon-side file is for cross-shell sharing + visibility. Test coverage: - trust_store/tests.rs: persistent_trust_add_then_list_roundtrips, persistent_trust_rejects_bad_shape (length + uppercase), persistent_trust_round_trip_via_disk (write → fresh store → load → assert), persistent_trust_idempotent_re_add, session_write_persists_trust_into_permanent_file (banner [a] → session write → persistent). - server tests: trust_add_then_list_roundtrip (real UDS pair), trust_add_rejects_invalid_hash_shape. Test count: atty-guard 140 → 147. Docs: - operator-workflow.md updated to clarify the dual-write model (atty-side file for back-compat + daemon mirror for cross- shell consistency). Mentions [a]llow always / [B]lock host forever in the verification section's banner output too. * fix(security_guard): address PR #147 review round 1 Subagent: fix-and-ship → 9 findings, applying all. 1. Stale docstrings claimed daemon classify-time consultation of persistent_trust + session_trust. Daemon does NOT consult trust at classify time — atty's in-proc check is authoritative. Rewrote PerUserState.persistent_trust + TrustAdd docs to match. 2. TrustList was dead code (RPC shipped but no atty-side caller). Wired lazy seed in queryDaemon: after the first successful daemon classify per session, atty fires trustList and merges the daemon's commands.trusted.txt into its in-proc rt.trust. New `daemon_trust_seeded: bool` runtime flag guards against re-firing. Picks up trust hashes set on a different atty proxy under the same UID without atty restart. New `uds_client::trustList(allocator, target)` method — minimal JSON parser (scans `"trust":[...]` then extracts 64-char hex needles) so we avoid pulling a full JSON crate into atty for one known-shape response. 3. Backward-compat gap documented inline. PerUserState comment makes clear: legacy ~/.cache/atty/security_trust.txt entries are NOT migrated; new entries flow to BOTH sides; the lazy seed unifies them on first reconnect for any new atty session. 4. TrustAdd doc gains an explicit DOS / write-amplification note: spamming random hashes disables NO detection but does trigger ~1.3 MB atomic rewrites at the cap. Per-UID scope contains the blast radius. 5. session_write trust drain re-ordered: duplicate-of-already- persisted is now a silent no-op (matches persistent_add_trust semantics). Previously a duplicate AT cap incorrectly reported "trust file full" — wrong UX since the operator has already trusted the hash. 6. JSON injection assumption documented inline at uds_client::trustAdd — hash is provably [0-9a-f]{64} by construction via trust_cache.zig::hashCategoryMatch's "0123456789abcdef" lookup. Raw interpolation safe; if the hash format ever changes the JSON escaper from sessionAddUrlBlock should be reused. 7. New test: persistent_trust_silently_skips_malformed_legacy_lines. Locks the silent-skip-uppercase behaviour. A hand-edited commands.trusted.txt with uppercase / truncated lines now has regression coverage. 8. PR body test count claim is wrong (says 140 → 147 but actual was 165 across all build configs). Will fix in PR description on next push. 9. CLAUDE.md compliance + scope sanity: OK. Test count: atty-guard 147 → 148 (+1 mixed-case test); atty 711 unchanged (uds_client trustList is parser-only, integration is exercised through the lazy seed path). * fix(security_guard): wire \`atty-guard trust\` CLI subcommand Copilot round 1 finding #5: docs / comments referenced \`atty-guard trust list\` but the CLI subcommand didn't exist. The TrustList RPC was shipped + the atty-side lazy seed was wired in the previous round-1 fix; this commit closes the loop on the user-facing surface. New subcommands: - \`atty-guard trust list\` — prints the caller's persistent trust hashes (sorted). No sudo. SUDO_UID forwarding applies same as other read-only ops. - \`atty-guard trust add <hash>\` — direct insert (mostly for testing / scripting; the typical path is banner [t]). main.rs gains a Trust subcommand variant + TrustOp inner enum; cli_client.rs gains handle_trust_list + maps TrustOp::Add to the existing Request::TrustAdd dispatch. The other 4 Copilot findings (stale docstrings claiming classify-time consultation, dead-code TrustList RPC, atty-side seeding gap, docs mentioning unimplemented surface) were already addressed in a16e237 — that commit's docstring rewrites + lazy seed wiring + uds_client::trustList method covered the overlap.
fentas
added a commit
that referenced
this pull request
May 20, 2026
- Watchdog spawn failure now propagates an error instead of silently disabling enforcement (subagent finding #3). The user's config explicitly asked for "kill after Nms" — a silent unbounded subprocess on a Thread.spawn OOM/limit was the worst possible default. - Test margin bumped from 200 ms budget / 170 ms lower bound to 500 ms / 400 ms — more robust against jitter on shared CI runners (finding #4). - Trim the WHY-only-violating comments per CLAUDE.md (finding #5). The race-safety + `posix.kill` rationale stays; the restatement-of-code prose goes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fentas
added a commit
that referenced
this pull request
May 20, 2026
* feat(llm): subprocess wall-clock timeout enforcement Closes #159. `Config.provider.subprocess.timeout_ms` is now load- bearing: a watchdog thread spawns alongside the child and sends SIGKILL via `std.posix.kill` if `timeout_ms` elapses before the main thread signals completion. The dying child unblocks the main thread's stdout read; `wait()` reaps the corpse cleanly; the post-wait check on the `timed_out` atomic surfaces the right error. Implementation: * Two `std.atomic.Value(bool)` flags — `completed` (set by main after stdout EOF) and `timed_out` (set by watchdog on expiry). Watchdog polls `completed` in 50 ms slices via `std.c.nanosleep`. Zig 0.16's `std.Thread` has no cancel primitive — polling is the only way to bail early when the main path finishes quickly. * `std.posix.kill(pid, .KILL)` bypasses `std.process.Child.kill`'s bookkeeping (which null-s `child.id` and races with the main thread's read/wait). The SIGKILL is a one-shot syscall; no shared state mutation. * `timeout_ms = 0` disables the watchdog entirely. Useful for debugging hung CLIs without atty stepping in. Tests: * `/bin/sleep 5` with `timeout_ms = 200` — verifies the watchdog fires within a 3 s upper bound, the error message contains "timed out" and "200ms", and the lower bound is honored (no spurious early kill). * `/bin/echo "hello"` with `timeout_ms = 0` — verifies the happy path still works when the watchdog is disabled. * All prior subprocess tests pass — none of them hit the default 30s budget on fast `/bin/echo` / `/bin/cat` runs. 728/728 unit + integration + e2e. fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(llm): subprocess timeout — review round 1 - Watchdog spawn failure now propagates an error instead of silently disabling enforcement (subagent finding #3). The user's config explicitly asked for "kill after Nms" — a silent unbounded subprocess on a Thread.spawn OOM/limit was the worst possible default. - Test margin bumped from 200 ms budget / 170 ms lower bound to 500 ms / 400 ms — more robust against jitter on shared CI runners (finding #4). - Trim the WHY-only-violating comments per CLAUDE.md (finding #5). The race-safety + `posix.kill` rationale stays; the restatement-of-code prose goes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(llm): subprocess timeout — copilot review round Three substantive Copilot findings addressed: * **EINTR drift** (#1): the slice-counted loop incremented `elapsed_ms` by the full slice even when `nanosleep` returned early due to a signal, so a busy parent could make the watchdog fire ahead of the configured budget. Switch to wall- clock via `nowMs()` — EINTR shortenings stop mattering. * **Race window** (#2): re-check `done` immediately before the SIGTERM closes the (already safe but theoretical) window between the last in-loop check and the kill syscall. Belt- and-suspenders. * **SIGTERM grace** (#4): graceful escalation per #159's spec — SIGTERM first, 200 ms grace, SIGKILL fallback. Lets well- behaved CLIs flush stderr / close files instead of being decapitated mid-write. Copilot's finding #3 (silent watchdog-spawn fallback) was already fixed in 874687e — it's an error now. 726/726 tests + integration + e2e green. fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 21, 2026
fentas
added a commit
that referenced
this pull request
May 21, 2026
Four findings: #2 providerLabel duplication: the circular-import rationale was wrong — both files already share types.zig and worker.zig imports cleanly. Moved the canonical helper to worker.zig (next to resolveProviderForMode where it pairs naturally), paint.zig + hooks.zig now reference \`worker_mod_ns.providerLabel\`. #3 Long provider labels overflow the divider. Clamp to \`max(8, cols/3)\` with U+2026 ellipsis so the trailing \`Alt+C close · Enter send\` shortcut stays on row even with a 40-char provider name on an 80-col terminal. #4 Gate missed the mid-dialog-without-chat case. Alt+M now also fires when \`dialog_persistent_mode != .off\` OR \`dialog_state != .idle\` — the user can be deep in an Alt+S flow with neither chat surface open and ai_mode_active toggled false post-commit. #5 Two new tests: overlay-open path (was uncovered) and the dialog-persistent-mode-without-ai_mode_active edge from #4. 761/761 tests + integration + e2e green. fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fentas
added a commit
that referenced
this pull request
May 21, 2026
* feat(llm): chat panel model indicator + Alt+M gate (#173 #6 #7) Two small chat-UX fixes from #173: #7 Alt+M cycle now fires inside the chat panel. Previously gated on \`ai_mode_active\` (the \`#: \` prompt-prefix mode); the chat panel doesn't set that flag, so a multi-provider config couldn't be cycled from inside the surface the user was chatting in. Gate widened to fire when chat panel OR overlay is open OR ai mode is active. When none of those hold, Alt+M still returns .forward so a stray Alt+M at the shell prompt doesn't get eaten. #6 Inline chat panel divider now surfaces the active provider name (\`✨ atty chat · <provider> ─\`). Resolved via \`resolveProviderForMode(.chat, …)\` so a chat-only entry from \`providers[]\` reads correctly even when current_provider_idx sits on a single-only entry that wouldn't actually serve this request. Trail-clearance math updated to subtract the dynamic label width. Two new tests pinning Alt+M behavior: fires inside chat without ai_mode_active; forwards when no LLM context is active. 759/759 tests + integration + e2e green. fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(llm): #174 subagent review — DRY label + ellipsis + gate widening Four findings: #2 providerLabel duplication: the circular-import rationale was wrong — both files already share types.zig and worker.zig imports cleanly. Moved the canonical helper to worker.zig (next to resolveProviderForMode where it pairs naturally), paint.zig + hooks.zig now reference \`worker_mod_ns.providerLabel\`. #3 Long provider labels overflow the divider. Clamp to \`max(8, cols/3)\` with U+2026 ellipsis so the trailing \`Alt+C close · Enter send\` shortcut stays on row even with a 40-char provider name on an 80-col terminal. #4 Gate missed the mid-dialog-without-chat case. Alt+M now also fires when \`dialog_persistent_mode != .off\` OR \`dialog_state != .idle\` — the user can be deep in an Alt+S flow with neither chat surface open and ai_mode_active toggled false post-commit. #5 Two new tests: overlay-open path (was uncovered) and the dialog-persistent-mode-without-ai_mode_active edge from #4. 761/761 tests + integration + e2e green. fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(llm): move resolver into types.zig — paint loses worker dep (#174) Copilot finding: paint.zig pulled in worker.zig only to call resolveProviderForMode + providerLabel, but worker.zig also contains the full HTTP worker implementation and its deps. Real concern: paint's compile surface widened for no reason. Move ResolvedProvider, providerLabel, resolveProviderForMode, and resolveProvider into types.zig (next to Mode/ModeMask/ ProviderEntry — their natural pairing). worker.zig keeps re-exports so existing call sites + tests don't shift. paint.zig drops the worker import and uses types.resolveProviderForMode directly. The other two Copilot comments were on the pre-43cf46d revision: - providerLabelFallback doc/impl mismatch: file already deleted in 43cf46d (function gone, replaced with the canonical providerLabel reference). - Long-name overflow: 43cf46d added the max(8, cols/3) clamp + U+2026 ellipsis. Already fixed. 761/761 tests + integration + e2e green. fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8 tasks
fentas
added a commit
that referenced
this pull request
May 21, 2026
Copilot review round 1 — six findings, five addressed: - **#1 cursor cell mid-byte slice** — `paintInputLine` rendered the cell under the cursor as `line[c_abs..c_abs+1]`, which cut multi- byte codepoints (`•`, emoji) mid-sequence and the terminal drew `�`. Now walks the UTF-8 codepoint starting at `c_abs` via `pw.utf8Iter` and slices the full byte range. Tail slice (`c_abs + cell_advance .. win_end`) follows. - **#2 label_buf re-cut** — the chat divider's provider-label truncation used `min(truncated.len, label_buf.len - 3)` to bound the @memcpy, which could re-cut a codepoint-aligned slice mid- sequence on a wide terminal. Capped `label_cap` at 32 cols (provider labels are model names — never longer in practice), bumped `label_buf` to 256 bytes, dropped the inner min. - **#5 UTF-8 paste through parseChatKey** — printable-insert arm only accepted 0x20..0x7E, so pasting `•` (0xE2 0x80 0xA2) hit `.none` for every byte. Extended to 0x80..0xFF — the terminal reassembles the codepoint on render. Also handles a bare CSI-u ASCII codepoint (`\x1b[N;1u` with N in 0x20..0x7E) as an insert fallback for terminals at higher progressive-enhancement levels. - **#4 CSI scanner overflow guard** — `p1 = p1 * 10 + (x - '0')` had no overflow check; malicious input with a long digit run could trap in safety builds. Cap accumulation at 1M. - **#6 doc nit** — `\x1b[1;7A` is xterm modifier 7 = Ctrl+Alt (not Shift+Alt+Ctrl, which is 8). Comment fixed. Deferred: #3 (continuation-row indent under role prefix — UX preference, not a bug). +1 test pinning `•` paste through onInput. 794/794 unit + fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fentas
added a commit
that referenced
this pull request
May 21, 2026
…resize (#175) * feat(llm): UTF-8 codepoint walker + display-width helper New private `paint_width.zig` module: ASCII fast-path, coarse East- Asian-Width / emoji range lookup, codepoint-boundary truncation. Replaces byte-slice truncation in subsequent commits so glyphs like `•` (U+2022 = 3 bytes) stop rendering as `�` when the cut lands mid-sequence. Scope is deliberately minimal — Ambiguous defaults to 1 col (Western terminals); the emoji-default-presentation subset of 0x2600..0x27BF is enumerated rather than blanket-billed; Wide covers CJK / Hangul / fullwidth / regional indicators / pictographs / supplemental symbols. 13 tests covering ascii / bullet / ellipsis / CJK / emoji / combining marks / truncation / invalid-sequence recovery. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(llm): chat paint truncations use display-col widths Routes every `slice = text[0..cap]` byte-slice through `paint_width.truncateToCols`, and every `text.len > cap` overflow check through `paint_width.measureCols`. Fixes the `•` (U+2022) and similar multi-byte chars rendering as `�` when the byte-prefix cut landed in the middle of a UTF-8 sequence. Wide glyphs (CJK, emoji like ✨) now bill 2 columns so trailing chrome stops getting pushed off the row. Sites touched: chat-overlay raw-fallback + per-action render (exec/question/done/choices), inline-panel renderTurnContent (envelope + raw), inline-panel divider label_visible math + clamp. 778/778 unit tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(llm): word-wrap long chat turns across multiple panel rows Inline chat panel turns previously truncated at one row with a dim `[…]` marker — fine for envelope summaries, useless for raw LLM prose where the actual reply lived past the cut. Raw turns now wrap at the last space inside the row (hard-break on codepoint boundary when a token alone exceeds cols), capped at 3 rows per turn so one long reply can't push the rest of the scrollback off-panel. Overflow keeps the `[…]` marker; the full content remains in the alt-screen overlay (Alt+Shift+C). paint_width.zig gains `WrapIterator` + `wrapIter()`; 8 tests cover short / word-break / multi-word / hard-break / wide-char / empty / all-whitespace / cols=0 fallback. renderTurnContent now returns rows used; the scrollback loop advances `row` by that count instead of the previous hardcoded `+= 1`. Envelope turns still single-row — the structured `desc → cmd` summary doesn't gain readability from being split. 786/786 unit tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(llm): multi-line chat input via Shift+Enter Shift+Enter (kitty kbd `\x1b[13;2u`) inserts `\n` into the inline chat input buffer instead of submitting. Plain Enter still submits the buffer verbatim (newlines preserved → sent as-is to the LLM). Empty-check accepts buffers of pure whitespace+newlines as no-op so accidental Shift+Enter strokes don't fire an empty request. The input area grows up from `input_row` by one row per embedded newline, capped at `panel_rows / 2` so scrollback isn't completely starved. Continuation rows show a dim `…` chrome glyph at col 1 to distinguish them from new turns. Cursor glyph (reverse-video block) lands on whichever line contains the cursor byte. parseChatKey's digit + param-scan branches collapse into one unified scanner that handles both `<n>~` (VT-style function keys) and `<n>;<mod>u` (CSI-u modified keys); other CSI shapes still get consumed silently so private/unknown sequences don't leak as printables. Two new tests: Shift+Enter inserts the newline + cursor advances; all-whitespace buffer + plain Enter clears without firing a request. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(llm): Ctrl+Alt+Up/Down resizes the inline chat panel New actions `llm_chat_inline_grow` / `llm_chat_inline_shrink` bump the panel height by one row per press, clamped to a minimum of 3 rows (divider + 1 scrollback + input) and a loose upper sanity cap. The live height lives in `Runtime.chat_inline_rows_override` and resets to `null` on every panel close, so the next open starts at the configured `cfg.inline_chat_rows` default. Bindings ship as dual-encoded Ctrl+Alt+Up / Ctrl+Alt+Down (legacy modified-arrow `\x1b[1;7A/B` + kitty kbd CSI-u sibling); both forms reach the same action so the keystroke lands on every terminal atty targets. `extraReserveRows` and paint both consult the override so the proxy's `applyReserveRows` path grows the statusbar reservation on the next tick. PageUp/PageDown already handle scrollback (existing `chat_scroll_page_up/down` bindings via `chat_inline_view_offset`) — no new scroll keys added. Three new tests cover grow + clamp at 3 + reset-on-close. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(llm): chat scrollback anchor + tab width + windowing on continuation rows Subagent review round 1 — addresses the two blocker findings and three smaller items: - **Newest turn was getting clipped** when prior multi-row turns filled the scrollback budget. Previous `start_turn = visible_end - scrollback_budget` math assumed one row per turn; with wrap enabled it anchored OLDEST turns at the panel top instead. Now walks backwards from `visible_end`, summing each candidate's rendered-row claim (mirroring the wrap iterator used at paint time) so the newest turn stays anchored at the bottom and only the OLDEST visible turn gets row-capped. - **TAB was billed as 0 cols** by `displayWidth` (and `Utf8Iterator`'s ASCII fast-path), but `writeSanitized` passes tabs through to the terminal. Chat content with tabs was mis-truncated and mis-wrapped. Treat as 1 col — conservative approximation but consistent with how the rest of paint counts. - **Empty-buffer fallback in paintInputBlock was dead code** — the main loop already paints prompt+cursor for the `pos == 0 == buf.len` iteration. Dropped the duplicate path. - **Apply 512-byte cursor windowing on every input line**, not just the first — long pasted prompts with embedded newlines previously let continuation rows overflow. - **Doc fixes**: keymap `llm_chat_inline_grow` referenced a non-existent `inline_chat_rows_min`; renderTurnContent narrated WHAT instead of WHY. +2 regression tests: newest-turn-anchor + TAB width. 793/793 unit tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(llm): UTF-8-safe cursor cell + label_buf + UTF-8 paste Copilot review round 1 — six findings, five addressed: - **#1 cursor cell mid-byte slice** — `paintInputLine` rendered the cell under the cursor as `line[c_abs..c_abs+1]`, which cut multi- byte codepoints (`•`, emoji) mid-sequence and the terminal drew `�`. Now walks the UTF-8 codepoint starting at `c_abs` via `pw.utf8Iter` and slices the full byte range. Tail slice (`c_abs + cell_advance .. win_end`) follows. - **#2 label_buf re-cut** — the chat divider's provider-label truncation used `min(truncated.len, label_buf.len - 3)` to bound the @memcpy, which could re-cut a codepoint-aligned slice mid- sequence on a wide terminal. Capped `label_cap` at 32 cols (provider labels are model names — never longer in practice), bumped `label_buf` to 256 bytes, dropped the inner min. - **#5 UTF-8 paste through parseChatKey** — printable-insert arm only accepted 0x20..0x7E, so pasting `•` (0xE2 0x80 0xA2) hit `.none` for every byte. Extended to 0x80..0xFF — the terminal reassembles the codepoint on render. Also handles a bare CSI-u ASCII codepoint (`\x1b[N;1u` with N in 0x20..0x7E) as an insert fallback for terminals at higher progressive-enhancement levels. - **#4 CSI scanner overflow guard** — `p1 = p1 * 10 + (x - '0')` had no overflow check; malicious input with a long digit run could trap in safety builds. Cap accumulation at 1M. - **#6 doc nit** — `\x1b[1;7A` is xterm modifier 7 = Ctrl+Alt (not Shift+Alt+Ctrl, which is 8). Comment fixed. Deferred: #3 (continuation-row indent under role prefix — UX preference, not a bug). +1 test pinning `•` paste through onInput. 794/794 unit + fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(llm): oldest-turn clip + UTF-8-safe windowing + overflow marker fit Review-loop round 2 — five findings, all addressed: - **Subagent #3 (newest-turn tail clip)**: round-1 fix anchored `start_turn` correctly via back-walk, but the render loop walked oldest→newest with a generic `min(per_turn_max_rows, remaining)` cap that still let the newest turn eat the deficit. Now the back- walk records `oldest_turn_cap = rows_remaining` when forced to include-and-clip; the render loop applies it ONLY to the first rendered (oldest) turn. Newer turns each keep their full per-turn claim. Regression test seeds 4 × 3-row turns with the marker in chunk 3 of the newest — verified fail-without/pass-with. - **Copilot #1 (paint.zig:486)**: `renderWrappedRaw` appended the `" […]"` overflow marker after a full-width chunk, pushing it past `cols`. Restructured the loop to look one chunk ahead via a `pending` slot: when the last allowed row is about to flush AND more content remains, trim the pending chunk to `cols - 5` and append the marker inline so the row stays within cols. - **Copilot #2 (paint.zig:578)**: input-line windowing used raw byte offsets; UTF-8 content >512 bytes could land win_start / win_end on a continuation byte and the first emitted slice would be invalid. Now snaps both edges forward past any continuation byte (`(b & 0xC0) == 0x80`). - **Copilot #3 (paint.zig:282)**: 1024-col comment claimed byte-for-byte parity with the old 1024-byte path; truncateToCols can return more bytes when content has zero-width chars. Comment now states the col semantics explicitly. - **Copilot #4 (hooks.zig:165)**: CSI scanner trigger only fired on param bytes (0x30..0x3F); intermediates in 0x20..0x2F (e.g. `ESC [ ! p`) fell through to the consume-3-byte arm and could leak their tail. Broadened to 0x20..0x3F (full P+I range per ECMA-48). 794/794 unit tests + fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
fentas
added a commit
that referenced
this pull request
May 26, 2026
…re (#239) * fix(line_state): preserve cursor across mid-line delete syncFromCapture User-reported regression (#5): typing "comand tesd aaa", arrow- lefting back across whitespace, backspacing the 'd' — ghost text wrongly re-engages mid-line and paints over the live characters. Root cause: `syncFromCapture`'s rewrite branch unconditionally clamped `cursor_pos = n` after a successful sync. The no-change branch already documented the hazard ("clamping to EOL would make mid-line cases paint ghost over the text-to-right") and correctly preserved cursor_pos. The rewrite branch carried the same bug just not the same fix. Fix: capture `cursor_was_at_eol = self.cursor_pos == self.len` before the rewrite, then: - if it WAS at EOL, land cursor at the new EOL (typical append / tab-completion path — covered by an existing test). - if it was MID-LINE, preserve the prior cursor clamped to the new range (mid-line backspace, mid-line delete, mid-line paste — newly covered by `syncFromCapture preserves cursor when mid-line delete shrinks the buffer`). Without DSR-6n cursor info OSC 133 can't tell us where the cursor actually went after the redraw; the heuristic is the best we can do with content-only sync. 881/881 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(line_state): pin the mid-line GROW path + document cursor drift Subagent: the EOL-completion test pinned only regression-survival of an already-correct branch; the @min(prior, n) math was only exercised by the shrink test. Added a mid-line-grow (paste) test that pins the modeled cursor stays at the prior position (drifts left of physical, harmless for ghost gating since cursor_moved stays true). Comment in syncFromCapture extends the prior cursor-drift rationale to call out the grow case explicitly. 882/882 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(line_state_tests): rephrase test comment per WHY-only — no fix-history reference * test(line_state): align mid-line GROW scenario — paste at cursor pos 4 produces 11-byte string * docs(CLAUDE.md): update syncFromCapture invariant — rewrite branch now mid-line-preserving too * test(line_state): align mid-line-delete repro — arrow×4 + backspace deletes 'd' producing 'comand tes aaa' --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fentas
added a commit
that referenced
this pull request
May 26, 2026
…s + cleanups PR #243 review surface (subagent, Copilot-grade thorough): - (#1) `probe_head_sha`: split HTTP-non-2xx from network errors so journald shows distinct breadcrumbs. 403/404 from GitHub used to render identically to "DNS down" with no log — operators couldn't tell rate-limited from renamed repo. Now: `HTTP 403`, `body read failed`, `JSON parse — ...` each get their own eprintln line. - (#2) **Behind-since anchor was reset on probe failure mid-drift.** Walk- through: tick N drifts at T0, tick N+1 network blips → anchor cleared, tick N+2 probe succeeds → anchor stamped with N+2's time. Operator loses the audit trail of "behind for 3 weeks." Fix: extracted `diff_step` as a pure helper that preserves the anchor when probe fails while pinned. Four edge cases now have direct tests (fresh / sustained / recovery / probe-fail-while-drifted). - (#3) `write_snapshot`: PID-suffixed tmp filename + `OpenOptions:: create_new(true)` to refuse following an attacker-pre-created symlink at the tmp path. Mirrors the posture used by `trust_store::write_atomic`. Also fixes the parallel-test collision risk + bubbles `create_dir_all` errors so the operator sees a useful message instead of generic ENOENT-on-tmp downstream. - (#5) Central drift logic is now testable via `diff_step` and has 5 new unit tests — was completely untested previously. - (#6) `DriftEntry::is_behind` lives on the wire-shape struct so the CLI doesn't duplicate the match. Removed the manual `matches!` clone in cli_client.rs. - (#8 + #9) `pin-init`: atomic write via tmp + `create_new` + rename. A Ctrl-C between the previous `fs::write` start and end could leave /etc/atty-guard/atoms.pins.toml truncated, which the daemon's `load_pins` HARD-fails on (empty file is a rejected state) — bricking the cron tick. Also closes the TOCTOU between the `path.exists()` check and the write. - (#16) `--json` mode now honors the exit-2 drift convention. Previously `atty-guard atoms drift --json | jq ...` always exited 0 even when drift was detected, defeating CI gating for json users. - (#17) `PIN_INIT_DEST` removed — uses `atom_fetcher::DEFAULT_PIN_FILE` so the two paths can't silently diverge. Test plan: 226/226 pass (5 new drift tests). Default + atoms-fetch builds both warning-free. Closes review round 1 on PR #243. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fentas
added a commit
that referenced
this pull request
May 26, 2026
…bootstrap (#243) * feat(security_guard): #209 — atom-fetcher drift detection + pin-init bootstrap Adds the V2-I drift telemetry agreed on in #208's review: after each successful atom-refresh tick the daemon probes upstream's master HEAD for each source, diffs it against the operator's pin, and persists the result to `/var/lib/atty-guard/atoms.drift.json`. Operators can inspect via `atty-guard atoms drift` (exit 2 on detected drift, CI- gateable) and the daemon logs a journald breadcrumb on the in-sync → behind transition so the signal isn't pull-only. Pairs with `atty-guard atoms pin-init`: a one-shot subcommand that seeds `/etc/atty-guard/atoms.pins.toml` from the bundled template so the operator's opt-in flow is `sudo atty-guard atoms pin-init && \ $EDITOR /etc/atty-guard/atoms.pins.toml` instead of "find the example in /usr/share or grep the repo." Refuses to overwrite (--force opts in to clobber). Drift module is read-only telemetry — never affects fetch behavior; the sha256 verification in atom_fetcher remains the security gate. Probe failures leave the previous snapshot in place. Producer side gated behind `atoms-fetch`; the read path + types stay unconditional so a no-feature build can still serve `AtomsDrift` RPC (returns `available: false`). Closes #209. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security_guard): address review round 1 — drift detection blockers + cleanups PR #243 review surface (subagent, Copilot-grade thorough): - (#1) `probe_head_sha`: split HTTP-non-2xx from network errors so journald shows distinct breadcrumbs. 403/404 from GitHub used to render identically to "DNS down" with no log — operators couldn't tell rate-limited from renamed repo. Now: `HTTP 403`, `body read failed`, `JSON parse — ...` each get their own eprintln line. - (#2) **Behind-since anchor was reset on probe failure mid-drift.** Walk- through: tick N drifts at T0, tick N+1 network blips → anchor cleared, tick N+2 probe succeeds → anchor stamped with N+2's time. Operator loses the audit trail of "behind for 3 weeks." Fix: extracted `diff_step` as a pure helper that preserves the anchor when probe fails while pinned. Four edge cases now have direct tests (fresh / sustained / recovery / probe-fail-while-drifted). - (#3) `write_snapshot`: PID-suffixed tmp filename + `OpenOptions:: create_new(true)` to refuse following an attacker-pre-created symlink at the tmp path. Mirrors the posture used by `trust_store::write_atomic`. Also fixes the parallel-test collision risk + bubbles `create_dir_all` errors so the operator sees a useful message instead of generic ENOENT-on-tmp downstream. - (#5) Central drift logic is now testable via `diff_step` and has 5 new unit tests — was completely untested previously. - (#6) `DriftEntry::is_behind` lives on the wire-shape struct so the CLI doesn't duplicate the match. Removed the manual `matches!` clone in cli_client.rs. - (#8 + #9) `pin-init`: atomic write via tmp + `create_new` + rename. A Ctrl-C between the previous `fs::write` start and end could leave /etc/atty-guard/atoms.pins.toml truncated, which the daemon's `load_pins` HARD-fails on (empty file is a rejected state) — bricking the cron tick. Also closes the TOCTOU between the `path.exists()` check and the write. - (#16) `--json` mode now honors the exit-2 drift convention. Previously `atty-guard atoms drift --json | jq ...` always exited 0 even when drift was detected, defeating CI gating for json users. - (#17) `PIN_INIT_DEST` removed — uses `atom_fetcher::DEFAULT_PIN_FILE` so the two paths can't silently diverge. Test plan: 226/226 pass (5 new drift tests). Default + atoms-fetch builds both warning-free. Closes review round 1 on PR #243. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security_guard): address review round 2 — drift detection polish PR #243 review round 2 surface (parallel Copilot + subagent): - **Copilot #3305280448** (atom_fetcher.rs cron comment) — the prior comment said probe failures "leave the existing file in place," which is misleading: we WRITE a new snapshot every successful tick with `upstream=null` for the failed source. Rewrote to reflect what actually happens: new snapshot is written with null upstream AND `behind_since` anchor preserved via `diff_step`. - **Copilot #3305280534** (atom_drift behind_since on probe failure) — stale-cache false-positive. The round-1 fix in 2814214 via `diff_step` rule 4 already preserves the anchor when `upstream=None`. Replying + resolving the thread. - **Copilot #3305280580** (json shape divergence from ResponseBody serde tag) — declining. The CLI's `--json` deliberately strips the type tag from the wire envelope so operators get `{available, updated_at, sources}` directly for `jq` pipelines; the type tag would just be noise. - **Copilot #3305280626** (protocol.rs docstring "drift-fetch") — renamed to "the `atoms-fetch` build feature is disabled" to match the feature flag name used everywhere else in the codebase. - **Copilot #3305280664** (test uses fixed /tmp path) — fixed. `read_snapshot_missing_returns_none` + `write_then_read_round_trip` now use `tempdir + pid + nanos` so repeated runs and parallel test binaries can't collide on a fixed slot. - **Subagent N1** — `println!` + `process::exit(2)` flush gap. `process::exit` runs C-level atexit (flushes libc stdio) but skips Rust's stdout LineWriter Drop. On `atty-guard atoms drift --json | jq ...` (block-buffered) the JSON body could be sitting in Rust's buffer when the exit fires. Added explicit `stdout().flush()` before both `exit(2)` sites in `handle_atoms_drift`. - Docstring at top of atom_drift.rs corrected to match the actual failure posture (new snapshot every tick, anchor preserved). Test plan: 226/226 pass. Default + atoms-fetch builds both clean. Closes review round 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security_guard): address review round 3 — drift detection nits PR #243 review round 3 (Copilot returned 8 inline comments, 3 stale from before the round-2 push, 5 actionable). Subagent round 3 verdict was ship-with-no-new-findings; this commit clears Copilot's nits so both reviewers are clean. - **protocol.rs:86 (Request::AtomsDrift doc)** — clarify preconditions: snapshot only appears when daemon is built with the `atoms-fetch` cargo feature AND started with `--atoms-update-interval`. Without those, this RPC returns `available=false` and that is the expected steady state — operators correlating "no snapshot" with daemon config now have an actionable hint. - **cli_client.rs `handle_atoms_drift` "no snapshot" branch** — reformatted the message into a numbered preconditions list so the "what to fix" path is scannable. Prior wording assumed the cron was configured and just hadn't fired yet. - **cli_client.rs handle_atoms_drift doc comment** — exit-code example in the doc now warns about `set -o pipefail` for the `... | jq` case, and shows the non-pipe `> out.json; [ $? -eq 0 ]` alternative that captures the exit code unambiguously. - **atom_drift.rs write_snapshot** — chmod 0640 error no longer silently swallowed via `let _ = ...`. Logs an eprintln when chmod fails so the operator sees the loose-permissions warning instead of the snapshot quietly inheriting umask. Not propagating the error because the snapshot content is operator-readable telemetry (no secrets) and failing the whole tick would hide the chmod hiccup; surfacing it lets the operator tighten by hand. - **contrib/atoms.pins.toml.example** — softened the `atty doctor` language: the Zig-side integration is deferred to a follow-up PR per the original PR description, not in-tree yet. Updated wording to say "A follow-up release will add..." so operators reading the template don't go hunting for a feature that doesn't exist. Stale-but-resolved Copilot comments (no code change needed, just replies): - atom_fetcher.rs:1140 — same as round 2; cron comment rewritten in cab9034 to match diff_step rule 4. False-positive against a stale cache. - protocol.rs:328 — "drift-fetch disabled" already renamed to "atoms-fetch" in cab9034. - atom_drift.rs:515 — test path collision-resistance added in cab9034 via tempdir + pid + nanos. Test plan: 226/226 pass. Both builds clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Phase 2 of the LLM module, plus the supporting statusbar surfaces, transparent failure UX, shell-integration command, and docs catch-up. 13 feat/fix commits + 1 docs commit, 14 net.
Lands on top of #4 (merged). Tested locally against Ollama with
qwen3-coder.What's new
#: <prompt>+ Enter → atty intercepts, sends to${api_base}/chat/completions, injects the returned command into the readline buffer for review. One-shot explanation rendered above the status bar via the new hint row. Failures (no endpoint, HTTP non-2xx, unparseable) surface as muted-red ⚠ notifications instead of silent no-ops.Config.api_basewins over$LLM_API_BASE/$OLLAMA_HOSTso users can bake the endpoint into theirconfig.zigand skip env-var games entirely.Config.context_env_varsexposes named env vars to the model as aContext: KEY=value, …blob appended to the user message.✨ prompt. Both configurable..replace_commitAction: prompts land in atuin / history via the proxy's existingdispatchLineCommitpath even though the typed line never reaches the shell. Ghost-suggest surfaces previous#: …prompts next time.provideHintText→setHint, dim italic) and error slot (provideErrorText→setError, muted red + ⚠ glyph) sit above the status text with a blank padding row between them and status. Errors take precedence over hints; expired errors fall back to suppressed hints automatically. Defaultreserve_rowsbumped 2 → 3 for the breathing room.atty init [shell]subcommand:eval "$(atty init bash)"(or zsh) drops into.bashrcand re-execs the current shell under atty + wires the shell-side OSC 133 prompt markers so atty'sosc133.zigcan capture the input region precisely instead of guessing from keystrokes. POSIX-compatible recursion guard via the existingATTYenv injection.provideTermByteshook: sibling ofpollShellInputfor the outer-terminal direction — modules can push OSC sequences (cursor colour, title, palette) to STDOUT_FILENO without those bytes reaching the child shell.client.deinitsignature change this PR fixes).docs/llm.mdend-to-end guide,docs/modules.mdupdated with the four new hooks +.replace_commit,docs/architecture.mdextended with the hint/error slots,docs/index.mdquickstart includes theevalinstall path. Floating right-sidebar TOC on ≥1200px screens, inline on narrow ones.Test plan
zig build test -Dtarget=x86_64-linux-musl— 226 tests pass (was 200 before phase 2 started)zig build -Dtarget=x86_64-linux-gnu -Doptimize=ReleaseSafe— clean buildzig fmt --check src/ build.zig— clean#: list filesagainst local Ollama +qwen3-coderresolves the command + shows the explanation$LLM_API_BASE+$OLLAMA_HOST→ muted-red ⚠ notification names the missing env varsatty init bashemits a valid POSIX snippet with the recursion guard + OSC 133 hooks🤖 Generated with Claude Code