feat(harness): str_replace L2 TEAL/EMBER side bands - #692
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Implement-plan for #691.
Not merged. Suggested next: adversarial-review. |
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #692
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/str-replace-side-bg @ 53abb6f · 3 files · str_replace L2 EMBER/TEAL side bands (Wasm paint only)
Lenses run: L1, L3, L5, L6, L8, L9 (skip L2: no trust-boundary / emit / redact change. skip L4: no workflow edit; build-harness already ran zig build test-rich + wasm. skip L7: no deploy bind)
AGENTS.md read: yes · docs/feature-divide.md yes (paint stays in-canvas; no DOM chat)
Findings
No surviving Blocker / Major after self-refutation. Splitter traces match appendStrReplaceDiff + buildStrReplacePreview (line-exact sentinels, escaped -old_string / +new_string is content, … (N more lines) is content, ERROR / one-sentinel → null → existing single-block path). id_extra +4..+8 stay inside the 1024-wide item namespace and are mutually exclusive with the fallback +4 body. Palette uses ember_surface/ember_text and teal_surface/teal_text (not ember_accent). No protocol bump.
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Nit | L8 | native/harness/src/ui/toolrun.zig still documents “up to 5 widget slots” and +4 detail body (~177–179). Paint now mounts +5 old box, +6 old tl, +7 new box, +8 new tl. |
Next L2 widget added at +5 by following the comment collides with the old band (expander state / hit-target mix). |
1024-wide stride still has room; a reader 20 lines down sees the new extras. Fails as documentation of the locked collision rule — plan-review #691 called unique extras a Minor for a reason. | high |
Residual risk
Attack did not breach a runtime path with current evidence.
- Goal 2 (empty new → visible TEAL strip) rests on dvui honoring
min_size_content.h = fontMono().lineHeight()on a box whose childtextLayoutreceived no glyphs (mixed_text.addTextMixedreturns atlen==0). 2 px pad alone was already rejected as insufficient. Splitter tests locknew.len==0; they do not paint. Operator checklist in the PR body is still unchecked. teal_surface(#0a1215) on transcriptteal_bgis a small delta — empty strip can look like a hairline rather than a band at some displays.- Fail-soft: malformed / host-fallback
str_replacedetail without both sentinels still paints oneteal_textslab (headers visible). Correct, but a clipped preview that lost+new_stringdegrades silently. - No 390 px wrap / no-h-gutter proof in CI.
Merge guidance
PASS WITH NOTES — safe to merge from this attack after the operator smoke (expand success, delete strip, exec/read_file still one block). The Nit is optional (+0..+8 in the slot comment).
zig → harness.wasm is green (test-rich includes src/rich/toolrun.zig). Do not treat this as --approve.
What was not attacked
Live Preview paint, restored-session L2 after refresh, Copy of the still-sentinel payload, dvui testing-backend layout of the empty new box, full vitest (N/A — no TS).
Summary
Implements #691 (source #687).
Successful
str_replaceL2 no longer dumps the whole audit block as oneteal_textmono slab. Wasm paints:teal_text)ember_surface/ember_textteal_surface/teal_text-old_string/+new_stringstay on the payload (emit + 20/6 preview) and are stripped from paint. Empty new still mounts a one-mono-line TEAL strip. Other tools keep the single-block path.No protocol bump. No TS emit/preview change.
Files
native/harness/src/rich/toolrun.zig—splitStrReplaceDetail(line-exact sentinels, slices intodetail, no alloc) + 8 unit testsnative/harness/src/ui/toolrun.zig—paintToolRunL2 side bands;id_extra+4..+8; min height =fontMono().lineHeight()docs/harness-limits.md— Tool-run L2 row: payload vs paintVerification
tsc --noEmitvitest run --changedharness.wasmin this sandbox — not this diffzig build test-rich/test-rich-invariants/harness -Doptimize=Debugzigin this agent workspacebuild-harnessSTR_REPLACE_DIFF_SIDE_MAX_BYTES= 4096 and 20/6 windows unchangedExpected vitest pass count: unchanged (0 new TS tests). New coverage is the 8 Zig splitter cases on
build-harness/test-rich.Test plan (operator, after Preview wasm)
str_replaceL2 → two bands, no visible sentinelsnew_stringempty) → old band + visible empty TEAL stripexec/read_fileL2 still oneteal_textblockstr_replaceL2 still splitsFixes #691
Refs #687