refactor: extract shared tool boilerplate in tools.ts (plan 018)#89
Merged
Conversation
Replace copy-pasted boilerplate in the Tester tool belt:
- errorText() replaces 9 identical `error instanceof Error ? ...` ternaries
(banned by CLAUDE.md).
- commitNote() replaces 9 screenshot-guard+commit pairs — the "fix one, miss
eight" hazard collapses to one definition.
- 3 `...(action.lastError && {...})` conditional spreads become plain ifs.
- createIframeTools() replaces createSpecialContextTools(explorer,'iframe');
drop the unreachable `context !== 'iframe'` guard and the banned conditional
spread at the tester call site (exitIframe key is unique — assigning last is
equivalent).
- createLearnExperienceTool() factory removes Navigator's
`null as unknown as Researcher` type-lie; Navigator calls it directly.
- getVisitedStates routes through successToolResult (gains the `action` field).
- First unit tests for tools.ts (validation paths + factories).
Deferred (documented): click/hover fallback-loop extraction (step 4) and the
disambiguateElements move (step 8) are behavior-risky without a live browser;
the currentActionResult alias is cosmetic. countAriaChanges unification is
handled by the plan-015 PR. All behavior-preserving; 63 integration + 770 unit
pass, tsc reduced by 1 in touched files.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Explorbot Self-RegressionCommit
Attempt details
Session analysis — basic (native): Session AnalysisThe issue tracking feature works end-to-end: creating, filtering by label/status, searching, and viewing details all function correctly. The core flows hold, but test automation experienced repeated locator/click failures that required retries before passing. Coverage
What works
Execution Issues
Defects(None identified — all failures were automation-related, not product failures.) |
DenysKuchma
approved these changes
Jul 13, 2026
…efactor/tools-boilerplate-consolidation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plan 018 — Consolidate duplicated boilerplate in
tools.tssrc/ai/tools.ts(the Tester's tool belt) is built from copy-paste. This extracts the shared shapes so each tool contains only what's tool-specific. Behavior-preserving.Done
errorText(error)— replaces the 9 identicalerror instanceof Error ? error.toString() : 'Unknown error occurred'ternaries (banned by CLAUDE.md).commitNote(activeNote, result, toolResult, action)— replaces the 9screenshot-guard + commitpairs; a fix to one copy previously missed eight. (TheformPASSED path's screenshot-then-branch was proven equivalent:!hasObservablePageChange⟹ the aria/url guard is false.)...(action.lastError && { error })→ plainif.createIframeTools(explorer)— renamed fromcreateSpecialContextTools(explorer, 'iframe'); deletes the unreachablecontext !== 'iframe'guard. Tester call site's banned conditional spread → plainif (isInsideIframe) Object.assign(...);exitIframeis unique so assigning last is equivalent.createLearnExperienceTool({ experienceTracker, getState })factory — removes Navigator'snull as unknown as Researchertype-lie.getVisitedStatesroutes throughsuccessToolResult, gaining theactionfield.tools.ts(TEST-01) — validation paths + factories, no browser.Deferred / coordinated
disambiguateElements→ Navigator) — DEFERRED (optional, high churn).countAriaChanges→utils/aria.ts) — handled by plan-015 PR refactor: simplify ActionResult loading and Diff API (plan 015) #87; not duplicated here.currentActionResultalias — deferred (cosmetic;createAgentToolsdeclaresstateManagerper-execute).Verification
bun test tests/unit→ 770 pass / 0 fail (+6 new).bun test tests/integration/→ 63 pass / 0 fail.bun run lint→ clean;tscin touched files 4 → 3, no new errors.🤖 Generated with Claude Code