* fix: harden codex stream parser and AI install for v1.5.2
Address findings from two adversarial code reviews of the v1.5.2
release set. Six correctness/security fixes in src/commands/codex/stream.rs
plus a separate parser rewrite for performance:
stream.rs hardening
- turn.failed now propagates as a non-zero exit so shell pipelines
detect codex failures via $? / $PIPESTATUS without inspecting stderr.
parse_stream returns Result<bool>; stream() process::exit(1) on
failure after flushing stdio.
- ANSI/OSC escape sequences are stripped from every passthrough field
(thread_id, reasoning, agent_message, command_execution, turn.failed
message). Closes the OSC 52 clipboard-write / terminal-spoof vector
that a hostile repo could inject through model output.
- JSONL line length is capped at 8 MiB. Oversize lines are drained via
BufRead::skip_until without copying, the read buffer is shrunk back
to 64 KiB so a single pathological event cannot pin 8 MiB for the
rest of the run, and a single warning fires on stderr.
- Schema-drift warning fires when the stream contained events but none
of them matched a known type — surfaces silent passthrough failures
if codex renames or restructures its event schema.
stream.rs performance rewrite
- Replace per-line serde_json::Value DOM parse with an internally
tagged Event<'a> enum and #[serde(other)] Unknown variant. Borrowed
Cow<'a, str> fields skip allocation when the JSON has no escapes.
- Drop String::from_utf8_lossy ahead of the JSON parser; from_slice
parses bytes directly. Saves one full-line scan plus an allocation
on the lossy-replacement path.
- sanitize_text is now a single-pass byte-level walk: a fast scan
locates the first sanitize-byte, the clean prefix is appended as
one slice, and subsequent clean runs are copied as slices instead
of decoded char-by-char. Splitting at sanitize bytes is UTF-8 safe
because all of them are < 0x80.
A/B on a 77 MB / 400k-event synthetic stream: -14% user CPU, -5% wall
time, byte-identical output to the prior implementation.
agents.rs
- is_installed_at requires sentinel files unique to the v1.5.2 bundle
(skills/code_review/SKILL.md + agents/adversarial-reviewer.md, and
the OpenCode/Copilot equivalents). v1.5.1 users with the right
top-level directories but no /code_review files now correctly
report not-installed and get re-prompted to install on next
status / configure.
opencode/commands/code_review.md
- Clarify that the tier header always shows the configured model that
actually ran. The --model value, when passed, is appended in
parentheses with "may not have been honored" since OpenCode's
per-spawn model override is best-effort only.
cargo test: 140 passed. cargo clippy --all-targets -D warnings: clean.
* style: realign rustfmt drift on unrelated files
Pure rustfmt output difference — no behavior change. Surfaced when
running `cargo fmt --check` during v1.5.2 release prep; predates this
branch's work.
- src/commands/thoughts/backend_display.rs: collapse a short closure
body onto one line.
- src/commands/thoughts/init.rs: collapse a one-method chain onto
one line.
- src/config.rs: split a long ok_or_else chain across two lines.
* fix: address regressions from final codex sanity pass
Final adversarial review of chore/v1.5.2-release-prep flagged three
regressions introduced by the prior parser rewrite. All three reproduce
against the new binary; all three are fixed here with regression tests.
read_line_capped misclassified EOF as truncation
- A final JSONL record without a trailing newline (codex sometimes
flushes its last event that way) was treated as cap-truncated: the
buffer was cleared, the line was dropped, and a bogus
"dropped at least one JSONL line longer than 8388608 bytes" warning
fired. A final turn.failed silently exited 0, breaking the
exit-code-propagation contract that motivated the original v1.5.2
hardening.
- Fix: use `Take::limit() == 0` to detect "hit the byte cap". Truncation
now requires both hitting the cap AND no trailing newline. EOF-no-
newline is processed as a complete line.
TurnFailed deserialize was poisoned by wrong-type fields
- The internally tagged enum declared `message: Option<Cow<str>>` on
TurnFailed. Codex schema drift (top-level `message` shipped as an
object/number/etc. while `error.message` is valid) caused the whole
event to fail to deserialize, dropping the failure marker, the
failure flag, and the non-zero exit. Regression vs the prior Value
parser, which matched on `type` first and treated unknown field
shapes as absent.
- Fix: declare both `error` and `message` as `Option<Value>` on the
TurnFailed variant and probe inside `extract_failure_message`. Any
non-string just returns None and we fall through to the next shape.
Invalid UTF-8 inside a valid JSON envelope dropped the whole event
- The prior parser used `from_utf8_lossy` so a stray bad byte in
`agent_message.text` became U+FFFD and the human still saw the
message. The new `from_slice` rejects it and the event vanishes
silently, especially dangerous when a later turn.completed arrives
and suppresses the schema-drift warning.
- Fix: two-tier path. Fast path uses `std::str::from_utf8` to borrow
zero-copy into the read buffer (the overwhelming case for codex
output). Slow path falls back to `String::from_utf8_lossy().into_owned()`
on bad bytes, parses from the repaired string, and the message
emits with U+FFFD substitution. Slow path allocates only on the
pathological input.
Five new tests cover the regressions:
- final_line_without_trailing_newline_still_processed
- final_agent_message_without_trailing_newline_is_emitted
- turn_failed_with_non_string_top_level_message_falls_back_to_error_message
- turn_failed_with_object_error_field_no_message_still_marks_failure
- invalid_utf8_inside_agent_message_falls_back_to_lossy
cargo test: 145 passed (was 140). cargo clippy --all-targets -D warnings: clean.
* feat(schema): add pr to thoughts type enum
Add pr alongside plan, research, handoff, and note as a valid value
in the type select. This lets the describe_pr / ci_describe_pr skills
file PR descriptions as typed thoughts artifacts on Notion / Anytype
backends. Three test sites that asserted the enum verbatim are
updated.
* feat(describe_pr): dispatch template and record by backend
describe_pr and ci_describe_pr previously hardcoded the git/filesystem
layout: template at thoughts/shared/pr_description.md, body at
thoughts/shared/prs/{number}_description.md. Add per-backend dispatch
so they work on every configured thoughts backend:
- git/obsidian: keep the thoughts/shared paths (sync only on git).
- notion: read the template from a workspace page titled "PR
Description Template"; persist the description as a type=pr row
in the configured Thoughts data source. A /tmp scratch file feeds
gh pr edit --body-file and is deleted after.
- anytype: equivalent (named template object; type=pr
hyprlayer_thought object).
Hard fail on missing template - no silent fallback to a hardcoded
one. describe_pr_nt is the skill for that case.
The dispatch table lives in a new shared
claude/skills/_thoughts/pr-description.md that the Claude SKILL.md
files reference. opencode/ and copilot/ inline the same table per
the existing convention in those install trees.