Fix 24 correctness, robustness, and security issues across the core crates

[davidrhodus]

Summary

A correctness-focused review of the core and agentic-coding crates (hi-agent, hi-ai, hi-tools, hi-cli, hi-lsp, hi-tui; mlx/cuda excluded) surfaced 24 major issues. Each was verified against a concrete failure scenario before fixing.

Crashes on untrusted input

• <|> slice panic: streamed text containing <|> (e.g. Haskell's alternative operator in a code answer) produced a reversed slice range and crashed the turn — in both hi-ai stream filters and hi-agent's strip_chatml_tokens. The suffix search now starts after the <| prefix.
• Unbounded SSE indices: one corrupt frame ("index": 9999999999) forced a multi-GB allocation in the OpenAI and Anthropic stream parsers. Indices are now capped at 512.
• Byte-slice panics: API-key masking (--show-config, /provider edit) and the TUI's Ctrl-R history preview panicked on multi-byte characters; both now truncate by chars.

Silent data loss / corruption

• Config saves deleted six profile fields: the hand-written Serialize impl omitted minimal_tools, curate_skills, explore_subagents, write_subagents, planner_model, skeptic_model — any /model switch or profile edit permanently destroyed them. Replaced with derived serialization so a new field can't be silently dropped again.
• /provider edit wiped uncovered fields (max_tokens, fallback, tool_mode, …) in both the REPL and TUI flows; edits now start from the existing profile.
• Non-UTF-8 files corrupted on edit: edit/multi_edit/apply_patch lossily decoded (e.g. Latin-1) files and wrote the mangled string back, destroying every non-ASCII byte file-wide. They now refuse with a clear error pointing at bash/iconv.
• Session JSONL interleaving: records >8 KB were split across multiple write() calls, so two hi -c processes (or a fleet child on --session-file) could corrupt each other's lines — silently dropped on resume. Each record is now one write_all on the O_APPEND fd.
• TUI transcript overwrite: the collapsed-reads merge (⏺ read 6 files · 743 lines) could replace committed assistant prose; it now merges only while its own line is still the last entry.
• Post-finish garbage discarded completed answers: a heartbeat/sentinel frame after finish_reason threw away the fully-streamed answer and re-billed a retry.

Security

• web_download had no SSRF check despite the module doc claiming protection — a prompt-injected URL could pull cloud-metadata/localhost content into a readable workspace file. It now runs the same validator as web_fetch (initial hop; redirect re-validation noted as follow-up).

Hangs and stuck states

• LSP: concurrent requests on one client stole each other's responses (120 s hangs); a timeout mid-frame permanently desynced the JSON-RPC stream with no recovery; and the synced-doc dedup map survived crashes//lsp off, so respawned servers silently reported "no problems found". Round-trips now serialize behind an io mutex, reads are two-phase (cancel-safe wait, then committed frame read) with poison-on-desync + respawn, and the dedup map is invalidated on respawn, disable, and failed syncs.
• Stale .memory.lock after a SIGKILL disabled memory persistence for all future sessions; locks older than 10 minutes are now broken.
• Failed goals were permanently dead: rederive_status never returned a Failed goal to Active after a plan revision activated new work, contradicting its own doc and disabling auto-drive.
• Loop firings could hang forever on a child that wedged after closing stdout, leaking the in-flight slot and eventually stalling every loop; child.wait() is now bounded like run_fix.
• Bash output readers aborted at the first invalid-UTF-8 byte — output lost, and a child filling the pipe stalled until the 600 s timeout. They now read raw bytes and lossy-decode per line.

Agent-loop correctness

• replace_last_nudge erased the current turn: a stale verify marker from an earlier turn triggered the pop path mid-turn, destroying the model's just-completed work and leaving consecutive user messages (invalid transcript; debug-assert panic). It now replaces only when the tail is exactly the prior nudge's response cycle.
• Pre-pass dependency violations: explore/delegate/record_decision ran before earlier mutations in the same batch, handing subagents a pre-edit tree (and tripping respects_deps in debug builds). They now dispatch inside the dep-aware scheduler.
• Context gauge clobbered by side calls: finalize/skeptic/curate/memory/explore/compaction requests overwrote context_used, so a ~3 K recap at the end of a 150 K session reported "ctx 2%" and silently skipped auto-compaction. Side calls now count spend without touching the gauge.
• Interrupt flag leak: an Esc landing as a batch's last tool finished cancelled the next round's (or next turn's) entire batch with fabricated "interrupted" results.
• Missing tool-call index collapsed parallel calls into slot 0 ("readread", concatenated JSON args) on local OpenAI-compat servers; routing now uses a new-call heuristic.
• Anthropic usage backfill sets estimated and context_occupancy; memory bullets ending in a sentence period are no longer misclassified as paths and dropped; REPL provider flows honor --config.

Test plan

• cargo test -p hi-agent -p hi-ai -p hi-tools -p hi -p hi-lsp -p hi-tui — 829 tests pass
• cargo clippy --all-targets on the six crates — only the four pre-existing hi-tui test warnings remain
• The <|> panic and prose-bullet drop were reproduced standalone before fixing

Known issues left for follow-up

1. Merged config save design: saving writes the merged global+local config to a single file — with a local hi.toml present, /model copies global profiles (including literal API keys) into the project file, and removing a globally-defined profile resurrects on next launch. Needs a decision on file ownership.
2. verify_skipped_for_prose_only_changes is flaky under full-suite parallelism (pre-existing): tests share the crate cwd, the verifier snapshots it, and a concurrent test's extension-less scratch file mid-window makes the change-set look non-prose. Harness refactor needed.
3. web_download follows redirects without re-validating (initial-hop check only).

🤖 Generated with Claude Code