Skip to content

🤖 refactor: auto-cleanup#3559

Open
mux-bot[bot] wants to merge 4 commits into
mainfrom
auto-cleanup
Open

🤖 refactor: auto-cleanup#3559
mux-bot[bot] wants to merge 4 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 maintained by the Auto-Cleanup Agent. Each pass lands at most one extremely low-risk, behavior-preserving cleanup drawn from recently merged main activity, then advances the checkpoint below.

This pass

Deduplicated two repeated fallback error strings in src/browser/components/AutomationModal/AutomationModal.tsx, introduced by #3560 ("centralize workflow automations"). The literals "Failed to save automation." and "Failed to remove automation." each appeared three times across the save/remove flows (pre-check, RPC-result, and thrown-error/catch paths). They are now SAVE_AUTOMATION_ERROR_MESSAGE and REMOVE_AUTOMATION_ERROR_MESSAGE module-level constants, so the wording stays consistent if it ever changes.

  • Behavior-preserving: each constant holds the exact same string previously inlined at every call site; the rendered/thrown text is unchanged. No test asserts these literals, and the 13 AutomationModal.test.tsx tests still pass.
  • Scoped: single file, no new imports, no exported-API changes.

Validation

  • bun test src/browser/components/AutomationModal/AutomationModal.test.tsx — 13 pass / 0 fail.
  • make static-check — eslint, tsc (both configs), and Prettier pass. The only failing step is fmt-shell-check, which requires shfmt (not installed in this environment); no shell scripts were touched, so CI (which has shfmt) is unaffected.
Prior passes

Auto-cleanup checkpoint: f96aa91


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

@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.

@mux-bot mux-bot Bot force-pushed the auto-cleanup branch from 141167e to a913e8d Compare June 15, 2026 14:17
@mux-bot

mux-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

⚠️ Auto-fixup analysis: the CI failure was a flaky test, not caused by this PR's changes. No fix pushed.

Failure: Run 27552085157 (commit 141167eb8) failed only in Test / Unit, on:
built-in deep-research workflow > keeps prototype-key source identifiers during dedupe
with a Bun engine-level crash: RuntimeError: Unreachable code should not be executed.

Why it's flaky / unrelated:

  • This PR's only diff is a pure refactor of src/node/services/memoryConsolidationService.ts (extracting a harvestRecordTime helper). It has no connection to the deep-research workflow source-dedupe path that crashed.
  • RuntimeError: Unreachable code should not be executed is an intermittent Bun JIT/engine crash, not a deterministic assertion failure.
  • Reproduced locally on the PR head: the originally-failing test passed in 3/3 full-file runs, while a different test (auto-fix checkpoint retry preserves completed patch after HEAD advances) flaked once — confirming non-determinism in this file.
  • memoryConsolidationService.test.ts: 36/36 pass.
  • A subsequent CI run on the current head a913e8d48 (run 27552691244) already completed successfully.

Action: None required for this PR. The flakiness lives in builtInWorkflowDefinitions.test.ts and should be tracked/stabilized separately.


Generated with mux • Model: anthropic:claude-opus-4-8 • Thinking: xhigh

@mux-bot mux-bot Bot force-pushed the auto-cleanup branch 2 times, most recently from d4c35d0 to 2df82f7 Compare June 15, 2026 17:43
@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 17:51
@mux-bot

mux-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Auto-cleanup pass note: the one failing check (Test / Unit) is pre-existing on main, not caused by this change.

  • This pass only touches src/node/services/analytics/etl.ts (re-export the canonical CHAT_FILE_NAME instead of redefining the "chat.jsonl" literal). historyService.ts/historyService.test.ts are byte-identical to origin/main.
  • The failing tests are HistoryService > getMessagesForCompactionEpoch > … (introduced by 🤖 perf: rotate sealed chat history out of chat.jsonl at compaction boundaries #3541). Test / Unit is currently red on main HEAD (a6d11e3b5) and the prior commit (a8f811f96) independently of this PR.
  • All other checks pass here, including CI Static Checks (full make static-check incl. shfmt), Test / Integration, builds, E2E, Storybook, and Chromatic.

Per the auto-cleanup agent's "extremely low-risk, behavior-preserving" constraint, this pass does not attempt to fix the unrelated HistoryService test regression. Pausing for human direction on that pre-existing failure.

@mux-bot

mux-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

⚠️ Auto-fixup could not resolve CI failure: the failing tests are not caused by the cleanup commits — they are pre-existing failures already red on main. Manual intervention needed.

What failed

Test / Unit failed with 10 tests (failed run 27564995706):

  • HistoryService > getMessagesForCompactionEpoch (3 tests)
  • MemoryConsolidationService (7 tests, incl. 2 timeouts)

Why it is not from this branch

The same 10 tests fail on main HEAD's own CI (acb98ed6c, run 27564747006), and I reproduced them locally at the merge base a6d11e3b5 — neither of which contains this branch's two commits. Both cleanup commits are behavior-preserving and untouched by the affected code:

  • 426bc8d41 only refactors memoryConsolidationService.ts ordering (identical values).
  • 2df82f753 only re-exports CHAT_FILE_NAME in analytics/etl.ts.

Root cause (pre-existing, from #3558)

src/node/services/historyService.ts getMessagesForCompactionEpoch calls this.iterateForward(workspaceId, …), but iterateForward(filePath, …) expects a file path (every other call site passes getChatHistoryPath(...) / getChatArchivePath(...)). With a bare workspaceId, fs.stat hits ENOENT, the visitor never runs, the summary is never found, and the call returns Err("Compaction summary not found"). The MemoryConsolidationService harvest tests fail/time out downstream because the memory harvest reads compaction-epoch messages through this broken path. The line was introduced by 5584b6437 (#3558), which is on main.

Action

Per the auto-fixup scope ("only fix what the cleanup broke; do not add unrelated changes"), I did not push a fix. The fix belongs on main (e.g., pass this.getChatHistoryPath(workspaceId) and iterate the sealed archive too). Once main is green, this PR's CI should pass on rebase/merge.

@mux-bot mux-bot Bot force-pushed the auto-cleanup branch from 2df82f7 to 9832d32 Compare June 15, 2026 21:07
@mux-bot

mux-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

This pass replaces a duplicated "wfr_" literal in WorkflowRunner.ts's nested-step drift check with the canonical isWorkflowRunTaskId() predicate from taskId.ts. Behavior-preserving (the result is only used as a boolean inside Array.find(); both forms are falsy for a missing taskId).

@chatgpt-codex-connector

Copy link
Copy Markdown

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

@ThomasK33 ThomasK33 added this pull request to the merge queue Jun 15, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 15, 2026
mux-bot Bot added 4 commits June 16, 2026 00:38
Deduplicate the `completedAt ?? startedAt` harvest-record ordering key in
src/node/services/memoryConsolidationService.ts. After #3558 ("add memory
harvest pipeline"), the same effective-time expression was computed inline
four times across two functions that must rank records identically:

- findNewestHarvestRecord picks the record with the max completedAt/startedAt.
- pruneHarvestRecords sorts descending by the same key to keep the newest 20.

Both now derive recency from one `harvestRecordTime(record)` helper, documenting
that they must agree on "newest" so the format can't drift between call sites.

Behavior-preserving: the helper returns the identical value, the reduce/sort
comparisons are unchanged, and normalizeHarvestRecord's distinct
`completedAt ?? Date.now()` (finalizing a stale record) is intentionally left
inline. Verified with `bun test memoryConsolidationService.test.ts` (pass) plus
eslint + tsc.

---

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

<!-- mux-attribution: model=anthropic:claude-opus-4-8 thinking=xhigh costs=n/a -->
etl.ts redefined `export const CHAT_FILE_NAME = "chat.jsonl"` even though
#3541 added the canonical constant in src/common/constants/paths.ts (and
etl.ts already imports its sibling CHAT_ARCHIVE_FILE_NAME from there).

Re-export the canonical constant instead so the "chat.jsonl" literal lives
in one place; analytics consumers (workspaceDiscovery, tests) keep importing
CHAT_FILE_NAME from ./etl unchanged. Behavior-preserving: identical value.
#3565 reintroduced a hardcoded startsWith("wfr_") in WorkflowRunner's
nested-step drift detection. taskId.ts already exports the canonical
WORKFLOW_RUN_TASK_ID_PREFIX and isWorkflowRunTaskId() predicate as the
single source of truth for that prefix. Use the helper instead of the
duplicated literal; behavior is identical (both are falsy for a missing
taskId inside the find() predicate).
Extract the repeated "Failed to save automation." / "Failed to remove automation." fallback strings in AutomationModal.tsx into two module-level constants. Behavior-preserving: each call site now references an identical string constant.
@mux-bot mux-bot Bot force-pushed the auto-cleanup branch from 9832d32 to 350dcee Compare June 16, 2026 00:46
@mux-bot

mux-bot Bot commented Jun 16, 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.

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