Skip to content

🤖 refactor: auto-cleanup#3555

Merged
ThomasK33 merged 2 commits into
mainfrom
auto-cleanup
Jun 15, 2026
Merged

🤖 refactor: auto-cleanup#3555
ThomasK33 merged 2 commits into
mainfrom
auto-cleanup

Conversation

@mux-bot

@mux-bot mux-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Long-lived auto-cleanup PR. Each pass applies at most one extremely low-risk, behavior-preserving cleanup picked from recently merged main commits, then advances the checkpoint below.

This pass

Extracted a local DisclosureChevron helper in src/browser/features/Memory/MemoryBrowser.tsx. After #3553 ("prevent duplicate consolidation tooltip") touched this file, two sibling components still rendered the identical disclosure-caret toggle inline:

  • ScopeSection rendered {isOpen ? <ChevronDown className="text-muted h-3 w-3" /> : <ChevronRight className="text-muted h-3 w-3" />}.
  • DirRow rendered the same toggle with an extra shrink-0 class.

Both now call one DisclosureChevron helper that picks ChevronDown/ChevronRight from the open prop and appends an optional className to the shared text-muted h-3 w-3 sizing classes.

  • Behavior-preserving: the helper emits the same icon component, the same aria-hidden="true", and a byte-identical className string for each call site (text-muted h-3 w-3 for ScopeSection; text-muted h-3 w-3 shrink-0 for DirRow). The class string is built by plain concatenation — no cn/twMerge reordering is involved.
  • Single file, two inline ternaries collapsed into one helper; the unrelated Folder/FolderOpen toggle in DirRow is untouched. No type, schema, or public-API changes.

Validation

  • bun test src/browser/features/RightSidebar/Memory/MemoryTab.test.tsx — 17 pass (covers scope section + directory collapse/expand, which exercise both chevron call sites).
  • make static-check — eslint, typecheck x2 (tsgo), and prettier all pass. The shfmt/hadolint steps are skipped because those binaries are unavailable in this environment; no shell scripts or Dockerfiles were touched.

Risks

Negligible. Pure local dedup of two equivalent disclosure-caret ternaries into one helper within a single file, with per-call-site rendered output preserved and unit coverage over both consuming components.


Auto-cleanup checkpoint: a815fd3


Generated with mux • Model: anthropic:claude-opus-4-8 • Thinking: xhigh • Cost: n/a

Dedupe the identical 'isOpen ? ChevronDown : ChevronRight' disclosure
toggle that ScopeSection and DirRow each rendered inline. Both now use a
local DisclosureChevron helper; rendered className output is unchanged
(DirRow keeps its extra shrink-0).
@mux-bot

mux-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@ThomasK33 ThomasK33 enabled auto-merge June 15, 2026 10:16
The prior run failed only on
'built-in deep-review-workflow > auto-fix checkpoint retry preserves
completed patch after HEAD advances' with 'Workflow run is already
active' (a test-isolation lease collision). It passes locally on this
branch and is unrelated to the MemoryBrowser-only cleanup in this PR.
@mux-bot

mux-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Auto-fixup: CI failure is a pre-existing flaky test (no fix pushed)

The failed run (27539216491, "PR" workflow on a7aa63070) failed only in the Test / Unit job, on a single case:

(fail) built-in deep-review-workflow > auto-fix checkpoint retry preserves completed patch after HEAD advances
error: Workflow run is already active: wfr_deep_review_fix_replay_head_advance
  at runWithLease (src/node/services/workflows/WorkflowRunner.ts:334)
  at builtInWorkflowDefinitions.test.ts:5765   // the retry run() call

Why this is not caused by the cleanup

  • This PR's diff touches no workflow code. The failed commit a7aa63070 only changed src/browser/features/Memory/MemoryBrowser.tsx; across the whole PR (main...auto-cleanup) nothing under src/node/services/workflows/** is modified.
  • The failure is a timing race in the lease-renewal logic. WorkflowRunner's renewal setInterval fires on real wall-clock time (staleLeaseMs/2), but the lease acquiredAtMs is stamped from the deterministic test clock. Under CI load, an in-flight renewal from the interrupted first run can re-stamp the lease, leaving a non-stale lease that makes the retry's acquireLease return false. The test already tries to mitigate this (advancing nowMs to 2000 before retry), but the window is not fully closed.

Reproduction

  • Passes deterministically in isolation, but stress-running it locally produced 1 failure in 8 runs with the identical Workflow run is already active error — confirming intermittent flakiness rather than a deterministic break.

Action taken

  • Per the auto-fixup flaky-test policy, no code was pushed (the cleanup diff is unrelated and a workflow-runner change would be out of scope/risky for this agent).
  • A re-trigger commit (5284d7f5c, empty) is already on the branch and its fresh "PR" CI run (27539741118) is currently in progress.

If the flaky test recurs, hardening it belongs in a dedicated change to WorkflowRunner/builtInWorkflowDefinitions.test.ts (e.g. awaiting in-flight renewals before releaseLease, or making renewal use the same injected clock guard), not in this auto-cleanup PR.

@ThomasK33 ThomasK33 added this pull request to the merge queue Jun 15, 2026
Merged via the queue into main with commit 9b398b1 Jun 15, 2026
24 checks passed
@ThomasK33 ThomasK33 deleted the auto-cleanup branch June 15, 2026 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant