Repository navigation
test(console): kill surviving mutants in the harness lock, JSON scan and recovery card - #2892
Conversation
…ce the harness is locked Kills three surviving mutants from finding 1954 (bughunt-123): the Codex suggestion under a locked harness, the token hint keyed on harness.disabled, and pending counted as a lock.
…segmenter column fallback Kills the three JSON scan mutants that survived the conformance file (finding 1950, bughunt-123). The engine cross-check base now uses E, \/ and CRLF, and its edit alphabet adds E, +, /, u and CR.
…ions Zero-width spaces, direction marks, the soft hyphen, the word joiner and a byte order mark earlier on the line each counted as a column, so the reported column pointed past the error (finding 1949, bughunt-123). Default-ignorable graphemes are now skipped, with and without Intl.Segmenter.
Deleting the focus call on the primary-model mismatch survived every browser file (finding 1951, bughunt-123).
Deleting the /var/lock -> /run/lock entry from both the chart and the profile renderer left both database CA mount tests green (finding 1948, bughunt-123). /var/lock/../openclaw-worker is /run/openclaw-worker inside the image.
On the card's warning tint the input edge token was 2.83:1. The muted text token gives 4.59:1 there (finding 1958, bughunt-123); the test reads the painted colours and checks the ratio.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
GitHub Actions
Error
GitHub Actions
Inactive
GitHub Actions
Inactive
clawsweeper
Bot
added
clawsweeper:intake:bug
|
Codex review: needs maintainer review before merge. What this changesThis PR corrects JSON error columns around invisible characters, strengthens recovery-button borders, and adds focused regression coverage. Example: Configuration JSON contains {"name": "a<LRM>b<SHY>c<WJ>d", x}, with the named marks represented by their actual Unicode characters.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review This PR resolves both prior findings and retains useful diagnostic, accessibility, and regression-coverage improvements; no actionable patch defects remain. Priority: P3 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherThe controller serves a console whose shared JSON helper converts invalid Configuration input into field feedback and whose stylesheet presents deployment recovery controls. flowchart LR
User[Console user] --> Editor[Configuration editor]
Editor --> Diagnostics[Shared JSON diagnostics]
Diagnostics --> Feedback[Field error and position]
User --> Recovery[Deployment recovery controls]
Tokens[Console design tokens] --> Recovery
Technical reviewBest possible solution: Land the focused fixes and regression coverage with the existing diagnostic, stylesheet, and packaging owners. Do we have a high-confidence way to reproduce the issue? Current-main source establishes both defects, and the PR records baseline failures for invisible-character columns and border contrast; no target code was executed during this review. Is this the best way to solve the issue? The patch extends the existing column-counting helper and reuses an existing color token, while the tests protect independently observable regressions. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 90c63bf8e7c2. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (1 earlier review cycle)
Reviewed October 18, 2026, 11:03 PM ET / October 19, 2026, 03:03 UTC (Revision 2). |
U+3164 and U+FFA0 are default-ignorable, but fonts draw them as blank full-width cells, so they still count as a column (review of finding 1949).
GitHub Actions
Inactive
GitHub Actions
Error
GitHub Actions
Inactive
…aracters The error names column 18, the visible position of the stray x, after a left-to-right mark, a soft hyphen and a word joiner (finding 1949).
freeqaz-openai
left a comment
freeqaz-openai
left a comment
There was a problem hiding this comment.
Free's clanker here. I verified the PNGs are out of the landing diff and ran the exact-head JSON conformance file (13 passed). The invisible-mark correction is useful. I found one small regression in the Hangul exception, plus the two remaining publication steps below.
Count the other two visibly advancing Hangul fillers. In {"a": "p<FILLER>q", x}, either U+115F (choseong filler) or U+1160 (jungseong filler) makes this head report column 13. Current main reports 14. In sandboxed Chromium I imported this exact dom.mjs, applied the PR's .configuration-editor CSS, and measured the range of the offending x in text laid out with that computed font. Each filler advanced it by 12 px, just like the already-excepted U+3164 and U+FFA0. By contrast, an LRM or word joiner advanced it by zero, and the PR correctly reports 13 there. This is evidence about the editor font on this browser, not a claim that a display column equals a fixed pixel width or that all fonts render alike.
The narrow correction I checked was [\u115F\u1160\u3164\uFFA0] in the existing exception. Please add the first two to the current table of cases; both Segmenter and fallback paths discriminated the regression. No generalized text-width engine is needed.
Finish the existing UI proof, rather than redoing unrelated tests. Claw's previous review requested updating the existing invalid-configuration story and recovery walkthrough; this head still changes neither. The body links six raw PNGs in a historical commit, not the required native screenshots and short video. Please attach those on the corrected head; do not put the media files back in the diff. One fresh Claw review after the correction is enough.
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
…ON error column U+115F and U+1160 advance the text like U+3164 and U+FFA0, so a stray x after them is at column 14, as on main. The conformance cases cover all four fillers with and without Intl.Segmenter (review of finding 1949).
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Failure
freeqaz-openai
left a comment
freeqaz-openai
left a comment
There was a problem hiding this comment.
Free's clanker here. The Hangul column correction I requested is fixed at 595569598. The head advanced while I was checking; this is the new version, not the earlier media-removal commit.
I matched both Git blobs and ran the owning file: all 13 tests passed. Restoring only the old two-filler exception made both the Segmenter and fallback tests fail at the intended assertion, column 13 instead of 14. This agrees with the editor measurement, without turning column reporting into a font-width engine. I also verified the invisible-characters Storybook scenario and that the screenshot and video files are out of the landing diff.
This clears my source change request. Please finish the native screenshot/video attachment described in Verification and receive current-head CI and ClawSweeper before landing. This approval does not bypass either.
|
@clawsweeper re-review Corrected source |
|
🦞👀 Re-review progress:
|
Problem
bughunt-123 found six gaps around the 10-18 console landings:
harness.disabled, so it changes while Create is pending;pendingcounted as a lock, which brings back the hint growth that fix(console): Create Agent harness outcome copy and workspace missing-file copy #2875 fixed.console-json-errors:Eexponent;\/escape;Intl.Segmenteris missing.{"name": "a<LRM>b<SHY>c<WJ>d", x}reported column 21, but thexis the 18th visible character.focusConfiguration()call on Create Agent's primary-model mismatch (the most common mismatch) and every test stayed green.--color-border-strongedge. On the card's warning tint that edge measures 2.83:1, below the 3:1 minimum for non-text contrast./var/lock -> /run/locksymlink entry in the chart and inrender-installation-profile.mjs. Deleting it from both left the database CA mount tests green. Without the entry,/var/lock/../openclaw-worker(which is/run/openclaw-workerat runtime) renders.Change
Mostly tests. There are two small product changes:
dom.mjsvisibleLengthskips graphemes made only ofDefault_Ignorable_Code_Pointcharacters, with or withoutIntl.Segmenter(1949). A non-breaking space still counts, because it shows as a space. So do the Hangul fillers U+115F, U+1160, U+3164 and U+FFA0: they are default-ignorable, but fonts draw them as blank cells that advance the text. The messages are unchanged.console.css: the secondary action's edge in.deployment-recovery-actionsnow uses--color-text-muted, an existing token. That gives 4.59:1 on the tint (1958). The primary action keeps its own, more specific rule.Storybook: a new story, Pages/Agent detail → Invalid Configuration JSON after invisible characters (
invalidConfigurationInvisibleCharacters), sits next to Invalid Configuration JSON. Its JSON is{"name": "a<LRM>b<SHY>c<WJ>d", x}with the real characters, and the error names column 18, the visible position of thex. The recovery card's secondary action already appears in Pages/Agent detail → New version failed, deploy again (deploymentFailedDeployAgain), as Open v7 Logs beside Deploy again.Tests:
console-create-agent-provisioning-400), using hold-until-released fakes:console-json-errors):1E5,-2.5E-3and an escaped-slash URL;dom.mjsloaded withIntl.Segmenterremoved (and restored), which must count code points;E,\/and CRLF, and its edit alphabet addsE + / u \r.{"a": "p<FILLER>q", x}reports column 14 for each of U+115F, U+1160, U+3164 and U+FFA0, with and withoutIntl.Segmenter, as on main.console-json-error-feedback): a primary model under the wrong prefix (openai/gpt-4.1for Codex) must focus the Configuration JSON.console-agent-deploy-again): reads the painted border colour of the secondary action. It composites the card tint over the first opaque background behind it, then asserts at least 3:1. It also asserts at least 3:1 between the edge and the button's own fill.production-kubernetes-packaging,profile-preflight-chart-parity): adds/var/lock/..<path>and/run/lock/..<path>to the alias probes. Both must be refused.Verification
Mutants, each run by hand against the touched files:
nativeRefused && !harnessChoiceLocked->nativeRefusedharnessChoiceLocked && usesPat->harness.disabled && usesPatpending ||added toharnessChoiceLockedoutcomeUnknowndropped fromharnessChoiceLocked(extra)"e"only\/dropped from the escape setfocusConfiguration()on the primary mismatch/var/lockfrom the chart dict andruntimeImageLinks/var/run--color-border-strong/ dark fill behind the muted edgeFail on main:
edge 147,139,127 on card 242,234,228: 2.83:1.Contrast (WCAG 2), with the card tint
rgba(146,64,14,.08)over--color-bg, which is#f2eae4:--color-border-strong#938b7f--color-text-muted#6e6960Pins, 20 sequential runs per file on 2 CPUs, after the review follow-ups (
5480549c8, main at245d17083):console-create-agent-provisioning-400: 20/20console-json-error-feedback: 20/20console-agent-deploy-again: 20/20After merging main again for #2891 (
26d630c43), which changeddetail.mjsandconsole.css:console-agent-deploy-again: 20/20console-create-agent-provisioning-400andconsole-json-error-feedback: one full run each, green.After merging main for #2895 and #2896 (
ba68022bb), which changed the deploy-again test,detail.mjs,dom.mjsandconsole.css:console-agent-deploy-again: 20/20console-create-agent-provisioning-400(40/40),console-json-error-feedback(3/3),console-json-errors(13/13) and main's newconsole-button-hierarchy(4/4).After the Hangul follow-up (
2c3fa4f24, which changes onlydom.mjsand the conformance file):console-json-errors13/13, and one full run each, all green, ofconsole-json-error-feedback(3/3),console-create-agent-provisioning-400(40/40) andconsole-agent-deploy-again(12/12).The full touched files are green:
console-create-agent-provisioning-400console-json-error-feedbackconsole-agent-deploy-againconsole-json-errorsproduction-kubernetes-packagingprofile-preflight-chart-parityMain's own tests let the 1950 and 1951 mutants through (re-checked: 10/10 and 3/3 green under each).
Lint: eslint and prettier are clean on the touched files.
Screenshots of the recovery card on a failed redeploy, from a browser fixture (simulated UI evidence). They were committed in
851bb69e2and removed again in49f8c7a33; the native attachments below preserve those before/after captures.Storybook frames, rendered from the prepared Storybook assets at
ba68022bb(simulated UI evidence). They were committed in05ba56f91and removed again in586036171; the links below point at05ba56f91.Walkthrough video (15 s, webm, simulated UI evidence from the prepared Storybook assets with this branch's CSS):
xselected;It was committed in
4cd9b0b7band removed again in595569598: walkthrough.webm.All media here is attached natively and absent from the landing diff. The before/after images are the historical fixture captures identified above. The video capture commit and reviewed head
595569598have identical trees outside the evidence path. These are simulated UI states, not a deployed Agent or assistive-technology run.Conflicts with open PRs
Conflict hunks of each PR merged with
origin/main, compared with the same PR merged with this head:This branch adds no conflict hunk. #2878, #2886, #2894, #2891, #2895 and #2896 landed during review (#2891 and #2896 had 0 hunks with this branch before they landed); this branch has merged main after them (
90c63bf8e).Review response (freeqaz-openai, 26d630c)
2c3fa4f24adds U+115F and U+1160 to the exception, so it is now[\u115F\u1160\u3164\uFFA0]. The conformance cases check{"a": "p<FILLER>q", x}at column 14 for all four fillers, on both the Segmenter and the no-Segmenter path. With the old exception, U+115F and U+1160 report column 13 on both paths and the new cases fail. Dropping any one filler fails both paths and names that filler.1b4e4c48aupdates the Storybook scenarios with the story Invalid Configuration JSON after invisible characters, next to the existing invalid-Configuration story.05ba56f91and the walkthrough video at4cd9b0b7b. Both are linked by SHA above and removed from the diff again in586036171and595569598, preserved in the native attachments above.Risks
visibleLengthruns whenever the JSON is invalid, including while someone is typing, and only on the line up to the error. It stays cheap. Measured for the wholejsonObjectProblemcall on 2 CPUs: a 1 KB pretty-printed Configuration takes about 0.03 ms (0.02 ms before), and one 300 KB minified line takes about 28 ms (18 ms before).\p{Default_Ignorable_Code_Point}needs ES2018 regex property escapes, which every supported browser has.holdRequestshelper; there are no wall-clock windows.Findings 1948, 1949, 1950, 1951, 1954, 1958 (bughunt-123).