fix: close preview, knowledge and delegation release blockers - #135
Merged
Conversation
Eleven confirmed release blockers, plus the defects found while fixing them. Every fix carries a regression that was shown to fail without it. Delegation and steering - Accepted pre-start child steering is durable across every failure path. The idempotency probe no longer skips when the message has no id, which the CLI always sends (`session send` posts `"id": null`), so a steer is stored exactly once instead of twice. Identity falls back to (role, content, provenance); `created` is excluded because the CLI restamps it on every retry. - The initialization queue settles only once durability is confirmed, on both the success and error branches, and is bounded at 64 entries with dedup that no longer depends on a caller-supplied turn id. - A transient read failure no longer kills a child turn that is already streaming; recovery never publishes a message it failed to persist. Capability containment - A SubAgent session whose delegated runtime profile is missing or invalid now fails closed on both the injected and direct turn paths, with a typed code, and the partially restored agent is evicted. Previously a pre-runtime-profile row could re-acquire `workspace`, whose tools include `workspace_open` — a route out of the delegated grant set entirely. Preview panel - Headless artifact reads open no-follow and verify the opened descriptor's identity, so a symlink swapped in after validation can no longer redirect the read; deny checks now apply to the file actually opened. - Decoded raster dimensions and total pixels are bounded before the renderer sees the bytes, and the revision is a keyed digest of content rather than size and mtime, which collide on a same-size edit within one mtime tick. - HEIC sources are measured before conversion, so a bomb is decoded by no process; embedded Office media and pdf.js source images are bounded before decode rather than only at the output canvas. - Office documents are no longer labelled trusted content. Prompt injection - One sanitizer for untrusted labels, shared by Rust and the renderer, dropping control, bidi and zero-width characters instead of angle brackets alone. - Page-controlled title, locator and revision moved inside the untrusted envelope, so a title containing newlines can no longer forge the fields that describe it, and cannot reach composer prose unsanitized. Knowledge - A base is retired only on a positive pre-OKF signal confirmed under the locks that authorize the deletion. A manifest that merely fails to state its generation is kept and reported: previously any parseable YAML defaulted to the legacy generation and was deleted, and the deletion logged as success. - One damaged unrelated base no longer prevents startup, and `knowledge list` reports the store without altering it. - A missing knowledge directory lints as a structural error rather than a clean report. Also: an ingest-stream test asserted a pre-OKF page directory the fixture had moved away from, reporting a working digest as writing nothing; `stage_import` is split so the archive budget is demonstrably accumulated before anything is written; and two test-isolation faults are fixed where recycled session ids met process-global registries.
`test (windows-latest)` failed 25 tests across four crates; `test (ubuntu-latest)` failed a streaming decoder test. Two independent Windows causes and one Linux test-isolation bug. The per-KB write lock lived inside the base it guards, at `<kb_root>/.biorouter-knowledge/write.lock`. Windows refuses to rename or remove a directory that still holds an open handle, so every staged deletion and every base rename failed with "Access is denied" while the lock was held — and the lock has to be held, because the legacy purge revalidates a base's format under exactly that lock before destroying it. The lock now lives beside the tree rather than inside it, at `<root>/.kb-locks/<id>.lock`, so nothing is ever open under a base root and both operations succeed with every lock still held. The revalidation window is narrower than releasing the lock would have made it, and a rename carries the lock with the base so the two cannot separate. The second cause is that a contended file lock was recognised by `ErrorKind::WouldBlock`, which only Unix produces. Windows reports contention as ERROR_LOCK_VIOLATION, which `std` leaves uncategorised, so every contended acquisition returned an error where it should have waited — a queued macro failed instead of queueing, and a cancelled operation reported a lock violation instead of cancellation. Contention is now asked of the platform. On Linux, two decoder tests that read more than one tool_use block were absent from the serial group guarding `BIOROUTER_TOOL_CALL_BATCHING`, so a sibling test turning batching off process-wide could make them observe unbatched output. Reproduced 7 times in 8 runs; 10 of 10 green once they join the group. Bases created before this change keep an inert `write.lock` inside them, which the ignore rules already excluded. `.kb-locks` cannot be mistaken for a base: its name fails id validation, which is what every root scan filters on.
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
1 participant
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.
Closes eleven confirmed release blockers ahead of 1.89.7, plus the defects found while fixing them. Every fix carries a regression that was demonstrated to fail without it — several lanes reverted their own change in place to prove the new test could go red.
Why this is larger than a normal patch
Three of the eleven blockers had no fix written at all when this work was picked up, and two of them destroy user data:
manifest.yamlwas read as legacy and deleted. Every field is#[serde(default)]with nodeny_unknown_fields, so a manifest that merely lost itsschema_versionline defaulted to the legacy generation and the base was removed — git history included — and the deletion logged as success. No concurrency required. A base is now retired only on a positive pre-OKF signal, confirmed under the same locks that authorize the deletion.File::open. With the system temp directory in the root set, a symlink swapped in after validation could redirect the read. Opens are now no-follow and the opened descriptor's identity is verified before anything is read.What changed
Delegation — accepted pre-start steering is durable across every failure path. The idempotency probe no longer skips when a message has no id, which the CLI always sends, so a steer is stored once rather than twice. The initialization queue settles only on confirmed durability and is bounded.
Capability containment — a SubAgent whose delegated runtime profile is missing or invalid now fails closed on both turn paths. Previously such a row could re-acquire
workspace, whose tools includeworkspace_open— a route out of the delegated grant set entirely. Mutation testing printed the actual escalated tool list.Preview — decoded raster dimensions and total pixels are bounded before the renderer sees bytes; revisions are a keyed digest of content rather than size+mtime, which collide on a same-size edit inside one mtime tick; HEIC sources are measured before conversion so a bomb is decoded by no process; embedded Office media and pdf.js source images are bounded before decode rather than only at the output canvas.
Prompt injection — one shared sanitizer for untrusted labels across Rust and the renderer, dropping control, bidi and zero-width characters instead of angle brackets alone. Page-controlled title, locator and revision moved inside the untrusted envelope, so a title containing newlines can no longer forge the fields describing it, and cannot reach composer prose unsanitized.
Knowledge — one damaged unrelated base no longer prevents startup,
knowledge listreports the store without altering it, and a missing knowledge directory lints as a structural error rather than a clean report.Verification
cargo test -p bioroutercargo test -p biorouter-servercargo test -p biorouter-mcpcargo test -p biorouter-clinpm run test:run./scripts/clippy-lint.shjust check-everythingjust preview-panel-e2eAlso fixed along the way: an ingest-stream test asserted a pre-OKF page directory the fixture had moved away from, so a working digest read as writing nothing;
stage_importexceeded thetoo_many_linesbaseline and is split so the archive budget is demonstrably accumulated before anything is written; and two test-isolation faults where recycled session ids met process-global registries.Known limitations, stated rather than implied
schema.mdis now quarantined rather than purged — data kept, migration incomplete for that base.🤖 Generated with Claude Code