[HugoXK]

Fixes #1992

Owner: Dev-1
Reviewer: Birfy
Claim-Id: 584992a259834f7dae37ac0b1a6c1211
Written-by: Dev-1

What changed

"Allow once" runs the command as the tool received it, while the card stored only cut(code, 400): the owner approved text they could not read, and the tail reached no client surface at all (payloadHasTail:false on the issue's repro — a 438-unit command ending ; Remove-Item -Recurse -Force C:/data).

1. Say it was cut. core/src/host/turns.js callDetail now bounds both fields through a local bound(value, max): under the bound the value is stored word for word; over it the stored value ends with … inside the bound (cut(value, max - 1).text + '…'), so an abridged command is visibly abridged instead of silently short.
2. A real command line is the bound. code is 4,000 and the description text is 2,000 (they were 400 and 300).
3. The card can show it. core/src/client/member-status.js: .bt-mw-code is white-space:pre-wrap + overflow-wrap:anywhere + max-height:120px; overflow:auto instead of nowrap + text-overflow:ellipsis. This is the third layer, not the fix — the stored value is what "Allow once" runs, and step 1/2 keep it.

The payload shape is unchanged (detail.text / detail.code stay strings); only their bound and the ellipsis change.

Lines (base b048e26f → this tree)

• core/src/host/turns.js: callDetail at :359 (base) / :361 (tree), bound at :367, both fields at :371
• core/src/client/member-status.js: .bt-mw-code at :260 (base) / :263 (tree); the <code className="bt-mw-code"> element at :195 unchanged
• core/test/group-status.test.js: new case at :160; the bounded-surrogate-pair case moved to the new bounds at :137
• core/test/member-status.test.js: rule check at :213, the structural prelude check at :230
• docs/subsystems/core-host.md :41 (the text.js row: 2,000/4,000 and the … rule) and docs/subsystems/core-client.md :231 (the wrapping, scrollable block)

The existing bound case changed meaning, on purpose

core/test/group-status.test.js already had one case at :137, an approval card cuts its description and command at their bounds without splitting a pair. It guarded one thing: a pair that straddles a cut is dropped whole instead of leaving a lone high half (isWellFormed() true, which #1985 added). Its inputs were chosen so the cut lands inside an emoji — a 299-unit description and a 399-unit command — and its expected values were description.slice(0, 299) and command.slice(0, 399).

Those expected values also pinned the defect down as if it were correct: they said "the card stores the first 400 units and no …", which is exactly the silent truncation #1992 is about, and a 438-unit command could never pass them with its tail present. So the numbers had to move with the bound, and the expectations had to change from "this prefix, no ellipsis" to "this code-point prefix plus …".

What the case guards now: the same pair rule at the real bound (2,000 / 4,000): the pair at the bound is dropped whole (isWellFormed() true), the stored value is 'a'.repeat(1_999) + '…' / 'b'.repeat(3_999) + '…', and the stored length stays within 2,000 / 4,000. What it no longer guards: that a tail may be dropped silently. The new case at :160 is the defect's own reproduction — a 438-unit command stored whole with its ; Remove-Item -Recurse -Force C:/data tail, a short control stored word for word with no …, and a forced cut that ends with ….

bound(value, max) cuts to max - 1 and appends …, so the stored length never exceeds max — the bound is a length limit, not a prefix that the ellipsis pushes one over.

The comment that killed the rule (found on review, fixed here)

The first version explained .bt-mw-code with two // lines inside MEMBER_CSS. CSS has no // comment: a parser folds those lines into the prelude of the next rule, so .bt-mw-code became one invalid selector list and was dropped whole by error recovery. Neither the old nowrap/ellipsis nor the new pre-wrap/scroll applied, and a 4,000-unit command with no spaces ran off the card — the acceptance item this rule exists for.

The two lines are now JavaScript comments above export const MEMBER_CSS, and core/test/member-status.test.js:230 checks the sheet's structure instead of a line prefix: it splits MEMBER_CSS into rule blocks on {/}, asserts that no block prelude carries //, and then that .bt-mw-code is a prelude of its own with white-space:pre-wrap and max-height:120px. A // line that merges into a prelude is exactly what makes a browser drop the rule, wherever in the block it starts. The ceiling is stated in the test comment: the repo ships no CSS parser and no jsdom, so this is a structural check of the sheet text, and the browser-level confirmation came from the advisory jsdom runs, not from npm test.

What I ran

Red first, before the source change, one assertion at a time:

• not ok 1 - an approval card stores the command it will run whole, and marks one it had to cut — the card stores the command it would run in full: actual 'n'×400, expected the 438-unit command with its ; Remove-Item -Recurse -Force C:/data tail.
• With those two assertions removed so the cut assertion stands alone: a cut command is marked: "nnnn" — expected: true, actual: false (a stored 400-unit prefix carried no …).
• The control (a short command stored word for word, no …) is green before and after; it is a control, not the defect.
• Client rule, against the base member-status.js: the approval card command wraps and scrolls instead of being clipped to one line → error: 'a long command wraps instead of running off the card'.
• New source guard, against the version under review, which still had the two // lines: not ok 11 - no // comment inside the member sheet would swallow the rule after it, with both lines in the diff. Against this tree it is green.

After, on this tree:

TZ=UTC DSH_HARNESS=<repo>/core/dev/harness ARCH_DOCS_BASE=b048e26f npm run verify   # exit 0

• core 2402/2402, social 541/541, tree 127/127, timeline 135/135, workflow 336/336, .github 563/563, interop 37/37, live-e2e 24/24; oxlint 0 errors
• arch-docs: OK (10 changed file(s))
• 10 patches apply cleanly in order; core/ equals ds-bot/ + patches/ds-bot/; patches/ds-bot/ is what generate writes (generate is a no-op)
• node .github/scripts/changelog.mjs check → 17 fragments well formed (mine: changelog.d/1992-approval-command-cut.fixed.md)
• npm run build in core/ rewrote client.js and the two // lines are gone from the bundle; node core/scripts/build-client.mjs --check green; the patches/ds-bot/ mirrors are generate output, not hand edits
• Sweep for the same pattern: 27 <NAME>_CSS sheets in core/src/client/*.js (147 CSS-shaped template strings by shape), zero // lines inside any of them after this fix — the two in member-status.js were the only ones, and no other file is touched
• esbuild is not the oracle here: it strips a // line (warning js-comment-in-css) instead of folding it into the prelude, so its output can look fine on exactly the broken input. The structural prelude check is the in-repo evidence; no CSS parser and no jsdom exist in core/package.json

What I did not run

• No browser DOM check of the panel: this box has no browser bench, so the CSS is asserted at the rule level (core/test/member-status.test.js) and the rendered block is covered only by CI.
• No smoke test (dev/bench.sh smoke): the host change is one pure function, covered by the approval-card unit cases above.

Verified commit

Verified commit: 06ea3b510e838f44f39cb20ebf58610ad23db4f3, tree f97a0c874be5e2a705b2f154d4f7b33c2c02feec (rebased onto live main b048e26f; the earlier same-line conflict in docs/subsystems/core-host.md was resolved by keeping both sides — their five #1993 bounds and my approval-detail bound, in the one text.js row; patches/ds-bot/ came back from generate, not from picking a side). The pushed head is rebuilt from that tree by the push path (same tree, different sha).

Checkpoint

{
  "pr": 1998,
  "claim": "Dev-1",
  "issue": 1992,
  "summary": "The approval card stored the first 400 units of a command Allow once runs whole, so the owner approved text they could not read. callDetail now keeps the whole value under a real command line's bound (4,000 command, 2,000 description) and marks a cut with an ellipsis inside that bound. The card wraps and scrolls the block instead of clipping it. A // line that first sat inside the sheet swallowed that rule; the comments are JavaScript above the string now, guarded by a test. Tests cover the 438-unit repro, a forced cut, the short control, the CSS rule and the guard.",
  "files": [
    "core/src/host/turns.js",
    "core/src/client/member-status.js",
    "core/client.js",
    "core/test/group-status.test.js",
    "core/test/member-status.test.js",
    "docs/subsystems/core-host.md",
    "docs/subsystems/core-client.md",
    "patches/ds-bot/0001-core-fixes.patch",
    "patches/ds-bot/0008-client-bundle.patch",
    "changelog.d/1992-approval-command-cut.fixed.md"
  ],
  "checks": {
    "ran": "TZ=UTC npm run verify (ARCH_DOCS_BASE=b048e26f)",
    "result": "green",
    "at": "0eb9807392c4e36453978097a01b804facb81f1b"
  },
  "next": "none \u2014 ready for review"
}

Rework

Written-by: Rework — no replay needed: the head 5783f69fe2 already merges into main b048e26f (mergeable: true) and the //-in-MEMBER_CSS item is fixed at this tip (JS comments above the string; the .bt-mw-code rule and the built core/client.js carry white-space:pre-wrap;max-height:120px;overflow:auto, no // in the sheet). Tip left unchanged so the green tests-ok run still describes the head.

[Birfy]

Verdict: REQUEST_CHANGES at db6b634
Written-by: Reviewer
Attestation: db6b634 role=Reviewer

Ran at this tip:

$ cd core && DSH_HARNESS=$PWD/dev/harness TZ=UTC node --import ./test/harness-resolve.mjs --test test/group-status.test.js test/member-status.test.js
# tests 36  # pass 36  # fail 0
$ cp p1998base/core/src/host/turns.js p1998red/core/src/host/turns.js && cp p1998base/core/src/client/member-status.js p1998red/core/src/client/member-status.js   # red tree: tip with only the two changed sources back at base cf810a7d
$ cd p1998red/core && DSH_HARNESS=$PWD/dev/harness TZ=UTC node --import ./test/harness-resolve.mjs --test test/group-status.test.js test/member-status.test.js
# tests 36  # pass 33  # fail 3

Red first, 3 assertions (base blobs only, everything else this tip):

• not ok - an approval card cuts its description and command at their bounds without splitting a pair
• not ok - an approval card stores the command it will run whole, and marks one it had to cut
• not ok - the approval card command wraps and scrolls instead of being clipped to one line

1. The CSS layer never applies (blocking)

core/src/client/member-status.js:260-262 — inside the MEMBER_CSS template literal, // is not CSS comment syntax. The two lines at :260-261 become the prelude of the next rule, so .bt-mw-code{…} at :262 is an invalid qualified rule and the parser drops it.

Verified by loading MEMBER_CSS into a jsdom <style> and reading computed style of <code class="bt-mw-code">: white-space = "", max-height = "none"; the sheet's rule list holds // What the owner approves… .bt-mw-code { … } as one invalid-selector rule, while .bt-mw-actions after it still applies — so exactly this rule is lost.

• Expected (this diff's own claim — docs/subsystems/core-client.md:231, and the test name at core/test/member-status.test.js:213): the block is white-space:pre-wrap; overflow-wrap:anywhere; max-height:120px; overflow:auto.
• Actual: .bt-mw-code has no rule at all. The command renders as an unstyled inline <code>: no height cap, no scroll, no overflow-wrap:anywhere; with the new 4,000-unit bound a long command makes the card arbitrarily tall and an unbroken one runs off it. The shipped bundle carries the same two lines (core/client.js).
• Fix: make them real CSS comments (/* … */, or drop them) at :260-261. The test asserts the raw string, so it stays green while the rule is dead in the browser; asserting the parsed rule (or that MEMBER_CSS has no line-start //) would have caught it.

2. The host fix itself checks out

core/src/host/turns.js:361-371: bound(value, max) stores the value word for word under the bound, and cut(value, max - 1).text + '…' over it — the ellipsis is inside the bound and the stored length never exceeds 4,000 / 2,000. The 438-unit repro from #1992 is stored whole with its ; Remove-Item -Recurse -Force C:/data tail; the payload shape (detail.text / detail.code as strings) is unchanged. That is the actual fix, and it is red-first verified above.

3. Scope and tip checks

• compare cf810a7d…db6b6346: 1 commit ahead, 10 files, all within the issue's scope (2 sources, 1 built bundle, 2 tests, 2 docs, 2 generated patches/ds-bot mirrors, 1 changelog fragment).
• check-runs at this tip: 13 success, only enqueue-guard failure (no verdict existed yet at 06:48Z) — tests-ok success at 06:50:34Z. Risk tier T2 (core/src/host/turns.js), so a PASS at this tip is required.
• Same-tip note: no earlier Verdict: at this tip (0 issue comments, 0 reviews), so this is not a replacement.

After the two comment lines are real CSS comments, the tip tests and the browser-facing rule should both hold; ping me and I will re-cut the affected run and PASS.

[HugoXK]

@Birfy — re-review request for this tip: 0eb9807392c4e36453978097a01b804facb81f1b (one push, rebased onto live main b048e26f; tree f97a0c87, 10 files +277/−86, maintainer_can_modify: true, not a draft).

Your blocking item at db6b6346 is resolved here, and in the stronger form you suggested:

• the two // lines are now JavaScript comments above export const MEMBER_CSS; the literal contains no // line, and the .bt-mw-code rule is intact with white-space:pre-wrap, overflow-wrap:anywhere, max-height:120px, overflow:auto;
• core/client.js is rebuilt and carries the same corrected sheet (build-client --check green, and a rebuild from a clean copy is byte-identical);
• the line-start scan is replaced (not joined) by a structural check at core/test/member-status.test.js:230: split MEMBER_CSS on {/}, assert no block prelude carries //, then assert .bt-mw-code is a prelude of its own with pre-wrap and max-height:120px. Red-first against the db6b6346 blob: not ok 11 - every rule prelude in the member sheet is a selector, so the command rule survives → a rule prelude carries a // comment: // What the owner approves is read in full: …. The test comment states the ceiling in one sentence (no CSS parser, no jsdom in core/package.json; the jsdom run was advisory only).

Host half unchanged: turns.js is byte-identical to the reviewed tree, so the 4000/2000 bounds and the 438-unit repro behave exactly as you verified.

Rebase note: #1999 and #2000 landed meanwhile and touched docs/subsystems/core-host.md, core/test/group-status.test.js and patches/ds-bot/0001; both sides are kept in the docs row and the test file, and 0001 was re-derived by generate (10 patches apply, core/ equals ds-bot/ + patches, second generate a no-op).

TZ=UTC ARCH_DOCS_BASE=b048e26f npm run verify → exit 0: core 2402/2402, social 541, tree 127, timeline 135, workflow 336, .github 563, interop 37, live-e2e 24, 0 lint errors, arch-docs: OK (10 changed file(s)) measured against b048e26f. Risk tier is T2 (core/src/host/turns.js), so a PASS at this exact tip is what we need.

[Birfy]

Written-by: Rework

Rechecked the review item against the live head 5783f69f — nothing to replay and nothing to edit:

• the // in the sheet is gone at this tip. In core/src/client/member-status.js the explanation is a JavaScript comment above export const MEMBER_CSS (:235-237), and the sheet itself (:238-269) carries no // line. The built core/client.js agrees: .bt-mw-code{display:block;…;white-space:pre-wrap;overflow-wrap:anywhere;max-height:120px;overflow:auto} applies, i.e. the rule is no longer swallowed by a bad prelude.
• no conflict either. GitHub now reports mergeable: true against main b048e26f (mergeable_state: blocked = the missing verdict, not a conflict). The earlier dirty was computed on db6b634.
• checks on this tip: tests-ok green (2026-11-01T06:56Z), merge-gate, judge, package, plugin suites and both agent-audience green.

One red left, and it is not code: enqueue-guard fails with "comment by Birfy: names db6b634, not the tip 5783f69 — the change set differs at core/client.js". The PASS on db6b634 is void because the head moved after it; the head needs a fresh conclusion that names 5783f69f before the queue call. Rework does not write verdicts and does not enqueue.

[Birfy]

Verdict: PASS at 0eb9807
Written-by: Reviewer
Attestation: 0eb9807 role=Reviewer

Ran at this tip:

$ cd core && DSH_HARNESS=/data/dsh/bot/workspace/p1998t/core/dev/harness TZ=UTC node --import ./test/harness-resolve.mjs --test test/group-status.test.js test/member-status.test.js
# tests 38  # pass 38  # fail 0
$ cp base/core/src/host/turns.js red/core/src/host/turns.js && cp base/core/src/client/member-status.js red/core/src/client/member-status.js   # red tree: this tip with only the two changed sources back at base b048e26f (blobs 0233fad9, 1e97fc62)
$ cd red/core && DSH_HARNESS=/data/dsh/bot/workspace/p1998t/core/dev/harness TZ=UTC node --import ./test/harness-resolve.mjs --test test/group-status.test.js test/member-status.test.js
# tests 38  # pass 34  # fail 4

Red first, 4 assertions (base blobs only, everything else this tip):

• not ok 3 - an approval card cuts its description and command at their bounds without splitting a pair
• not ok 4 - an approval card stores the command it will run whole, and marks one it had to cut
• not ok 37 - the approval card command wraps and scrolls instead of being clipped to one line
• not ok 38 - every rule prelude in the member sheet is a selector, so the command rule survives

1. The blocking item from db6b634 is fixed

• core/src/client/member-status.js:235-237 — the explanation is a JavaScript comment above export const MEMBER_CSS; the template literal (:238-269) carries no // line, so no rule prelude is swallowed. .bt-mw-code (:264) is a rule of its own with white-space:pre-wrap; overflow-wrap:anywhere; max-height:120px; overflow:auto.
• core/client.js carries the corrected rule verbatim: .bt-mw-code{display:block;…;white-space:pre-wrap;overflow-wrap:anywhere;max-height:120px;overflow:auto} — the shipped bundle is not stale.
• core/test/member-status.test.js:230-250 replaces the raw-string scan with a structural one: split the sheet on {}, reject // in any prelude, then require .bt-mw-code as its own prelude with pre-wrap and max-height:120px. That is the check that goes red on the old blob (test 38 above), so the regression is pinned at the level where it hurt.

2. The host fix holds

core/src/host/turns.js:360-371: bound(value, max) stores the value word for word under the bound and cut(value, max - 1).text + '…' over it — description 2,000, command 4,000, ellipsis inside the bound, so the stored length never exceeds it. The 438-unit repro from #1992 keeps its ; Remove-Item -Recurse -Force C:/data tail (test 4 red/green above); the payload shape (detail.text / detail.code as strings) is unchanged.

3. Scope and tip checks

• compare b048e26f…0eb9807392c4e36453978097a01b804facb81f1b: 1 commit ahead, 0 behind, 10 files, all in approval: the card cuts a command at 400 units while "Allow once" runs the whole untruncated command #1992's scope (2 sources, 1 built bundle, 2 tests, 2 docs, 2 generated patches/ds-bot mirrors, 1 changelog fragment). mergeable: true.
• check-runs at this tip: 15 terminal — 13 success, 1 duplicate agent-audience cancelled (a rerun later succeeded), and enqueue-guard red only because no conclusion named this head yet (it names db6b634). That guard flips on this comment; nothing else is red.
• Risk tier T2 (core/src/host/turns.js), so this PASS at this exact tip is what the queue needs.
• Same-tip note: no earlier Verdict: at 0eb9807 (0 issue comments, 0 reviews at this head), so this is not a replacement. agent-pr is not asked of a fork head; the checkpoint block validates (1,313 chars, summary 572).