Repository navigation
fix(console): pin the long-name overlap invariant and drop the dead credential-saved line - #2943
Conversation
…rets feedback order - Export the long-name rule's alternatives and check directly that no two match the same character and no combining mark starts one, over every code point; the 50 ms bound could not catch an overlap (finding 2013). Both mutants the bughunt named fail it. - Remove the channel Secrets 'Credential changes may have been saved' line: it could never show, because an unknown outcome always carries its save error, whose 'Outcome unknown' line is now pinned as a danger alert (finding 2014). - Set the feedback tone before its text on Secret select and save refusal, pinned by a mutation-order test (finding 2015).
Review follow-up: the contexts were single characters, so a \p{M}* lookahead
was never tried with a mark before its target. Also writes the context
characters as escapes.
|
🦞👀 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. |
clawsweeper
Bot
added
clawsweeper:intake:bug
|
Codex review: needs maintainer review before merge. What this changesThis PR checks long-name regex alternatives for overlap, removes an unreachable credential warning, and sets feedback tones before updating text. Example: Saving Slack channel Secret bindings returns HTTP 403.
Review scores
ProductKind: Maintenance · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep this PR open: it provides useful regression protection and focused console cleanup, with no introduced correctness defect found. Priority: P3 Before mergeNone. FindingsNone. Tests
Agent review detailsHow this fits togetherThe console classifies Agent names for wrapping and turns channel Secret save results into feedback, reload controls, and deployment gates. flowchart TB
A[Agent names] --> B[Long-name classifier]
B --> C[Agents list rendering]
D[Secret picker and save] --> E[Configuration API]
E --> F[Credential feedback]
F --> G[Reload and deployment controls]
Technical reviewBest possible solution: Land the focused invariant coverage and feedback cleanup while preserving existing regex, save, reload, and deployment behavior. Do we have a high-confidence way to reproduce the issue? Current-main source demonstrates both text-before-tone calls and the unreachable warning condition; no target code or tests were run during this review. Is this the best way to solve the issue? The focused changes preserve established behavior, and the new overlap check protects an invariant that the existing timing threshold cannot reliably detect. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 8fe071637376. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
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 19, 2026, 10:11 AM ET / 14:11 UTC (Revision 2). |
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
clawsweeper
Bot
added
clawsweeper:intake:bug
Problem
Three follow-ups from bughunt-128 on #2926/#2935 and #2931:
list.mjs) must never let two of its alternatives match the same character, nor let a combining mark start one: each overlap doubles the ways to split a run (finding 1999). The only guard was a 50 ms timing bound inconsole-agent-name-runs.test.mjs, which cannot see a small overlap at the 200-character limit. The bughunt's two mutants (droppingLETTER_EMOJIfrom the last alternative's exclusion, and dropping the(?<![A-Za-z0-9])lookbehind on the hyphen-before-digit alternative) both passed 15/15.credentials.mjs, made an alert in fix(console): announce danger feedback lines as alerts and pin the form refusals #2931) could never show.outcomeUnknownis only set together with a non-definitivesaveError, and the line required the opposite. Its only test asserted that it is absent.setStatusTonecallers in the channel Secrets panel wrote the text before the tone (Secret select and save refusal), againstsetStatusTone's rule from fix(console): announce danger feedback lines as alerts and pin the form refusals #2931.Change
list.mjs: export the alternatives asUNBROKEN_ALTERNATIVES(the joined regex is unchanged).console-agent-name-runs.test.mjsgains a direct check over every code point. Each alternative is widened by stripping its lookarounds (a class-aware scan). Widening only matches more, so it is a sound filter. Characters that two widened alternatives can consume, and combining marks that any can, are then tried with the real alternatives between 38x38 contexts chosen to satisfy each lookaround (digits of several scripts, joiners, marks, dashes, quotes, letter emoji, and a mark before several lookahead targets). It asserts that no two alternatives match the same character and no mark starts one. It takes about 0.2 s. The 50 ms bounds stay.credentials.mjs: remove the unreachable line. An unknown outcome is still reported, as a danger alert: the save error line says "Outcome unknown. Reload this Agent to confirm the saved Secret bindings.", Reload draft shows, and the deploy gate names it. That line is now pinned asrole=alert,data-tone=danger.credentials.mjs: set the tone before the text at both callers.UX plan follow-ups to #2931 and #2935 (findings 2013, 2014, 2015).
Verification
console-agent-name-runs.test.mjs: the new test passes, and fails on both named mutants (M1 at the letter-emoji exclusion, M2 at the hyphen-before-digit lookbehind).console-agent-channels.test.mjs: new "Agent credentials set the channel Secrets feedback tone before its text". A MutationObserver on the feedback line records["data-tone", "text"]for a 403 save refusal and again for the next Secret choice. Reverting either swap alone fails it (["text", "data-tone"]).console-agent-draft-guards.test.mjs: the outcome-unknown line is a danger alert. A mutantsaveErrorTone()returning"warning"fails it.taskset -c 2,3).Open-PR conflict check
3-way
git merge-treeof this head with each open PR in the plan's conflict map (#906, #2253, #2255, #2258, #2445, #2524, #2599, #2833, #2922, #1619 and freeqaz-openai's open PRs), compared with origin/main: no new conflicted file or hunk with any of them. #2524 already conflicts with main inlist.mjs, and #2599 touchesconsole-agent-draft-guards.test.mjs; the hunk counts are unchanged.Risks
UNBROKEN_ALTERNATIVESis a new export that only the test reads.