Skip to content

Codebase review: fix correctness bugs, rebuild analyzers, refresh libs and docs (desktop 0.5.0) - #8

Merged
Falkicon merged 7 commits into
mainfrom
claude/mechanic-codebase-review-a74f35
Oct 1, 2026
Merged

Falkicon merged 7 commits into
mainfrom
claude/mechanic-codebase-review-a74f35

Conversation

@Falkicon

@Falkicon Falkicon commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Summary

A full review pass over the addon, desktop tool, dashboard, libraries, docs and agent skills, followed by the fixes. Six commits, grouped by area so each can be read on its own.

Highest-impact fixes

  • fencore-* commands always crashed (config.wtf_path does not exist).
  • addon.test reported a pass when busted crashed; addon.output agent mode reported API-bench failures as Lua errors.
  • The desktop tooling read/wrote the repo-root UI/APIDefs tree while the game loads Mechanic/UI/APIDefs; the root copy is removed and the addon tree regenerated (4521 APIs).
  • Dashboard script injection through a non-numeric BugGrabber counter; the Settings "Add Addon" button crashed.
  • mech setup wrote null paths that broke every later run.
  • Inspect never hid stale detail sections; several UI panels leaked frames; the toggle keybinding and /mech gc were broken; unknown /mech commands raised a locale error.
  • sandbox.exec ran user code with full os/io access; it now runs in a restricted environment.
  • The static analyzers were mostly false positives (on Mechanic/: 1005 dead-code findings down to 125, 10 of 12 security findings false down to 0); rebuilt on a Lua tokenizer.

Maintenance

  • afd is the hosted package (afd>=0.8.0,<0.9) instead of a vendored 0.1.0 copy. Desktop version is 0.5.0.
  • FenUI synced; AceConfigDialog 92 to 93; LibDBIcon/LibDataBroker vendored (they were gitlinks with no .gitmodules).
  • Skills/workflows consolidated: one reload protocol, a generated command reference, .agent/ generated from .claude/, with a pytest drift guard. About 40 broken doc links fixed.

Test plan

  • pytest in desktop/: 704 passed (243 before the work), with MCP installed and Lua 5.1 available
  • Ruff focused rules and format check clean
  • Offline Lua harnesses (tests/*_regressions.lua) and Node dashboard suites (tests/*_regressions.cjs) pass
  • Luacheck 0 warnings / 0 errors on first-party Lua
  • Wheel builds as 0.5.0 and passes the smoke script from a clean venv
  • CI on GitHub: the Windows jobs, Python 3.13 and the new template workflow have not been run
  • In game after /reload: Inspect then a plain table (no stale sections), Tests tab, API tab (switching APIs, Run Namespace confirmation), Performance sorting and CPU ms/s, /mech gc, toggle keybind, unknown /mech foo, Settings panel, minimap button, esMX/ptPT client text

Notes for review

  • .claude/settings.json: the invalid permissions.defaultAllow key is replaced by permissions.allow (read-only git and mech commands); the blanket Bash(mech*) is gone.
  • The deprecated-API list was built from a local wow-ui-source-live (UI 11.2.7) and will miss newer deprecations until regenerated with python -m mechanic.deprecations_builder.
  • Third-party library freshness upstream was not checked (no network); the local AceEvent MINOR 9 patch was intentionally not applied.

🤖 Generated with Claude Code

Jason Falk and others added 7 commits September 30, 2026 20:59
…DataBroker

- FenUI synced from the shared _dev_/Libs copy (theme-switch refresh APIs,
  animation fixes); every FenUI function first-party code calls still exists.
- AceConfigDialog-3.0 92 -> 93 in the addon and the template. The semantic change
  is the larger confirm popup; the large diff is upstream formatting versus the
  previous StyLua-formatted copy.
- LibDBIcon-1.0 (r55) and LibDataBroker-1.1 (4) were gitlinks without a
  .gitmodules, so they were empty in any fresh clone and embeds.xml pointed at
  missing files. They are now tracked as plain files.
- MechanicLib copies made identical (header comments only).
- AceEvent MINOR 9 from the shared Ace3 folder is an uncommitted local patch, not
  upstream, and is intentionally not applied.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…UI/ copy

The game loads Mechanic/UI/APIDefs, but the desktop tooling read and wrote the
repo-root UI/ tree, so regenerated definitions never reached the addon. The two
trees had diverged and neither was a superset of the other.

- Regenerated Mechanic/UI/APIDefs from 12.0.1.64914 (4521 APIs). Widget
  ScriptObject methods that the old fallback parser keyed as globals and Events
  that were read as APIs are no longer emitted.
- Only SecretArguments=NotAllowed is classified restricted/protected;
  AllowedWhenUntainted no longer marks an API as protected.
- Entries carry funcPath only; the function is resolved lazily at call time.
- Removed the root UI/ tree, including the unused APIDefinitions_Registry.lua.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…re and the UI tabs

Bootstrap/Core
- Toggle keybinding called a method that does not exist; `/mech gc` formatted
  strings with %f; unknown `/mech` commands raised a missing-locale error.
- SyncAllAddonData now pcalls every third-party callback, copies results before
  annotating them and rebuilds addonData so unregistered addons are pruned.
- Bootstrap fails closed on a non-table diagnostic target (as Core does); the
  Lua eval path no longer tostring()s secret or hostile values.
- DetectErrorSource tried the generic pattern first and attributed every error to
  "Interface"; MechanicLib versions were never listed; Health Log is now fed by
  hub-sync failures; dead code and stale version fallbacks removed.

UI
- Inspect: detail sections were never hidden when switching targets; panels are
  isolated with pcall and secret-safe; property rows are pooled instead of leaking
  frames; protected frames are not edited in combat; Reset All now resets.
- Errors/Console/Tools/Tests: session dropdown and source counts, dedup mutating
  the buffer, non-string log messages, print capture via hooksecurefunc, panels
  built once, third-party callbacks pcalled, test ids containing ':'.
- API tab: export scoped to the namespace instead of the topic bucket, Run
  Namespace confirms first and only runs read-only APIs in time slices, one
  reusable detail panel, lazy funcPath resolution, secret-safe serialization.
- Performance: working CPU ms/s column, header-click sorting, dead event-frequency
  tracking removed.

Locales
- Added the missing help key, removed enGB (AceLocale maps it to enUS), filled
  esMX/ptPT from esES/ptBR, removed dead and duplicate keys.

Adds offline regression harnesses for the inspector, API/performance code and the
bootstrap/Core paths, plus a locale-parity test.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…tatic files

- A non-numeric BugGrabber `counter` was interpolated unescaped into innerHTML, so
  any installed addon could execute script in the dashboard (which can POST to
  /api/execute). All interpolated values are escaped, counters are coerced, and
  the pure renderers live in render.js with hostile-fixture tests.
- The Settings "Add Addon" button called an undefined function.
- index.html is now markup only; logic is split into render/state/api/views/app/ws
  /main scripts and dashboard.css, and the regression harness loads the real files
  instead of slicing the inline script.
- Sidebar buttons, version, port and server status come from commands.list and
  /health; the output pane loads on startup and after a reconnect; keyboard
  shortcuts ignore modifier keys; toast warning style; bounded command history.
- Accessibility: real buttons with aria-expanded, restored focus outlines,
  aria-live toasts. No external font request; `unload` replaced by `pagehide`.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ild the analyzers

Infrastructure
- `mech setup` wrote null paths that crashed every later run; it now merges into the
  existing config and verifies tool checksums before writing anything.
- sv.parse reports PARSE_ERROR for truncated SavedVariables instead of success; the
  watcher parses off the event loop, skips unrelated files and no longer dies on
  Windows console encodings; the reload trigger matches the exact game window.
- `mech release` calls release.all (preflight/dry-run); `--agent addon.output`
  works; failing commands exit non-zero; server.shutdown is graceful.
- /health reports version and port; history is capped and large results are stored
  as stubs; invalid command input returns VALIDATION_ERROR with field details.
- afd is now the hosted package (afd>=0.8.0,<0.9) instead of a vendored 0.1.0 copy.
  Data files ship in the wheel via mechanic.resources; SSE MCP defaults to
  127.0.0.1:3101 and warns that it is unauthenticated.
- CI: Windows for wheel/dashboard/MCP jobs, Python 3.13, least-privilege token,
  harnesses run by glob. Version bumped to 0.5.0, single-sourced.

Commands
- addon.output reported API-bench failures as Lua errors in agent mode; addon.test
  reported a pass when busted crashed; fencore-* always raised AttributeError;
  lua.queue emitted a syntax error for snippets ending in `]`; git.commit
  committed every staged file; changelog.add buried the intro; assets.sync deleted
  hand-made assets; addon.create/sync accepted unsafe names and untemplated files;
  addon.validate rejected the project's own 120100 interface.
- addon.deprecations ships a database built from Blizzard's Deprecated_* shims.

API pipeline and sandbox
- Tooling resolves Mechanic/UI/APIDefs; api.info returned the wrong name for most
  APIs; definitions are cached; the generator is safe against wiping the wrong
  directory and escapes generated Lua.
- sandbox.exec/test run in a restricted environment (no os/io/package/debug), with
  a packaged test framework and structured errors.

Analyzers
- Rebuilt on a Lua tokenizer and block walker. Results are sorted before
  truncation and report truncated/total_issues. On Mechanic/ dead-code findings
  went from 1005 to 125 and false security findings from 10 of 12 to 0.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
- One diagnostic-target and reload protocol (new using-mechanic skill) that the
  other skills link to; CLI-first guidance and the nonexistent `mech call -i`
  option removed everywhere; s-working merged into k-ecosystem; k-create-skill
  rewritten for Claude Code; new k-desktop and k-apidefs skills.
- The command reference is now generated from the registry (.claude/
  gen_command_reference.py) and `.agent/` is generated from `.claude/`
  (.claude/sync_ide.py); desktop/tests/test_agent_docs.py fails on drift, broken
  relative links or the stale phrases the audit found.
- Wrong ports, routes, MechanicLib/FenUI APIs, slash commands and keybinding
  claims corrected; dev guide and integration docs moved off the removed
  addon-dev CLI; ~40 broken links and anchors fixed.
- CHANGELOGs: 0.5.0 entry, addon entries, missing 1.4.x entries sourced from git;
  SECURITY supported versions per component; docs/cli-reference regenerated
  (60 commands); PLAN files marked with real status, stale note removed.
- _TemplateAddon: current interface version, current deprecation-safe luacheck
  globals, guarded MechanicLib registration, CI workflow.
- .claude/settings.json: the invalid `permissions.defaultAllow` key is replaced by
  `permissions.allow` with read-only git and mech commands (blanket `mech*`
  removed).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
- test_commit_reports_git_failure_with_suggestion wrote a pre-commit hook without
  the executable bit. Windows git runs it anyway, POSIX git ignores it, so the
  commit succeeded on Linux. Mark the hook executable.
- wheel_smoke.py asserted installed resources were not under the whole checkout,
  but CI creates its throwaway venv at desktop/wheel-env, inside the checkout.
  Compare against the source package directory (desktop/src) instead.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Falkicon
Falkicon merged commit 167160c into main Oct 1, 2026
14 checks passed
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