Skip to content

refactor: merge duplicate helpers, remove dead code, add a lint step - #71

Merged
danoszz merged 4 commits into
mainfrom
refactor/cleanup
Oct 20, 2026
Merged

danoszz merged 4 commits into
mainfrom
refactor/cleanup

Conversation

@danoszz

@danoszz danoszz commented Oct 20, 2026

Copy link
Copy Markdown
Owner

Code cleanup with no behaviour change. Stacked on #70 (base fix/security-review-2), so retarget to main after #70 merges. PR #63 is still open, so this PR doesn't touch the dashboard client.

What changed

Area Before After
Git runners 4 copies: src/agent/git.js, lib/hook/index.js, lib/deps/lockfile.js, src/cli/detect.js lib/git.js: safe config, git(), gitSpawn(), gitToplevel(). The loop's allow-list runner builds on it. Reads that need the repo's real core.hooksPath pass raw: true
Markdown escaping mdInline plus 4 hand-rolled table-cell escapes in CI scripts src/report/md-escape.js: mdInline (untrusted text) and mdCell (our own CI tables)
Fetch wrappers 3 copies of fetch → status check → capped JSON parse in recon fetchJson() in src/utils/http.js (on top of #70's readBody)
Temp folders mkdtemp + try/finally rmSync in hook and install lib/tmp.js: withTempDir()
Dead code findShadowEndpoints, getTemplateStats, DEPS_FLAGS, stagedFiles (never referenced), runFile in machine.js, unused imports and bindings removed
Lint none eslint.config.js (recommended rules minus style-only ones), npm run lint, and a CI step before the tests. ESLint 10.12.0, @eslint/js 10.0.1 and globals 17.12.0 are pinned exactly, each at least two weeks old, and are dev-only

Unused flags: none found. Every parsed option is read and documented. fix --plan is the documented default mode.
Module splits: none. The largest files are cohesive: lib/machine.js (machine audit), lib/injections.js (probes) and lib/deps/index.js. src/cli.js is mostly a single 600-line run(), and splitting it would be a rewrite while #70 and #63 are in flight.
Runtime dependencies: still exactly one (yaml).

Numbers

Before (#70 head) After
Lines in bin + src + lib + scripts 35,028 35,026 (two new documented modules absorb the removed copies)
Lines in tests 13,323 13,322
Packed 683,240 B 683,617 B
Unpacked 1,814,746 B 1,815,454 B
Files in package 185 187 (lib/git.js, lib/tmp.js)
Locked packages (dev included) 68 145 (ESLint)

The line count barely moves. The gain is one place each for the git safety config, escaping and temp folders, not a smaller codebase.

Tests

Small side effects

  • The hook's checkout-index and the lockfile and detect reads now run with the same safe config overrides as the loop: no hooks, no signing, no attributes file. For a normal repo the output is the same.
  • If recon gets a JSON null body, it returns no results instead of throwing.

🤖 Generated with Claude Code

danoszz and others added 3 commits October 20, 2026 17:12
… fetches

- lib/git.js: the safe git config and runner, now used by the agent loop's
  allow-listed runner, the pre-commit hook, lockfile reads at a ref and
  project detection (four copies before). Reads that must see the repo's
  real core.hooksPath opt out with `raw: true`.
- lib/tmp.js: withTempDir() for the hook's and install's scratch folders.
- src/report/md-escape.js: mdCell() for the CI report tables (four copies).
- src/utils/http.js: fetchJson() for recon's three fetch-and-parse blocks.

AI-Assisted: Claude Code with Claude Opus 5.5; merged the duplicate helpers into single modules
Co-Authored-By: Claude Opus 5.5 <[email protected]>
- eslint.config.js: recommended rules minus style-only ones; `npm run lint`
  runs in CI before the tests. ESLint is a devDependency only; the package
  still has one runtime dependency.
- Removed exports nothing referenced (findShadowEndpoints,
  getTemplateStats, DEPS_FLAGS, stagedFiles), an unused runFile helper in
  machine.js, and unused imports and bindings the lint found.

AI-Assisted: Claude Code with Claude Opus 5.5; set up the lint step and removed what it found unused
Co-Authored-By: Claude Opus 5.5 <[email protected]>
AI-Assisted: Claude Code with Claude Opus 5.5; added the Changed entry
Co-Authored-By: Claude Opus 5.5 <[email protected]>
@danoszz
danoszz changed the base branch from fix/security-review-2 to main October 20, 2026 14:15
@github-actions

Copy link
Copy Markdown

Dependency capability diff

Compared against 0562cbaba42143bdce1c535b4b274e5cccf7be47: 75 added · 2 changed · 0 removed.

Upgrades that gained capabilities

Package Version Gained Evidence
ajv 8.20.0 → 6.15.0 computed require/import node_modules/eslint/node_modules/ajv/scripts/bundle.js:15

New packages

Package Version Can do
@​humanwhocodes/module-importer 1.0.1 computed require/import
cross-spawn 7.0.6 shell, env vars
debug 4.4.3 env vars, whole environment
eslint 10.12.0 env vars, dynamic code, computed require/import
fast-json-stable-stringify 2.1.0 computed require/import
ignore 5.3.2 env vars
isexe 2.0.0 env vars
minimatch 10.2.6 env vars
path-key 3.1.1 env vars
which 2.0.2 env vars
Evidence (file:line)

ajv@​6.15.0

  • computed require/import: node_modules/eslint/node_modules/ajv/scripts/bundle.js:15 var json = require(path.join(packageDir, 'package.json'));

cross-spawn@​7.0.6

  • shell: node_modules/cross-spawn/index.js:3 const cp = require('child_process');

debug@​4.4.3

  • whole environment: node_modules/debug/src/node.js:124 exports.inspectOpts = Object.keys(process.env).filter(key =› {

eslint@​10.12.0

  • dynamic code: node_modules/eslint/lib/rules/no-eval.js:58 description: "Disallow the use of ˋeval()ˋ",
  • dynamic code: node_modules/eslint/lib/rules/no-implied-eval.js:25 description: "Disallow the use of ˋeval()ˋ-like methods",
  • dynamic code: node_modules/eslint/lib/rules/no-param-reassign.js:12 /(?:Statement¦Declaration¦Function(?:Expression)?¦Program)$/u;

penthera deps v1.7.0: offline static analysis, no telemetry. Capabilities are what code can do, not a verdict.

Checked by Penthera, open-source security for the apps and AI systems you ship · https://github.com/danoszz/penthera

Updated for commit fdb6252 · workflow run. This comment is edited in place on every push.

@danoszz
danoszz merged commit 004a34d into main Oct 20, 2026
18 checks passed
danoszz added a commit that referenced this pull request Oct 20, 2026
… diff-bound approvals into the redesign

main (#70, #71) moved the dashboard to a bearer session in the URL fragment
and sessionStorage, a fetch-based run stream, SHA-256-bound approvals and
cut-diff acknowledgement, in files the redesign had rewritten. Resolved by
keeping the redesigned files and porting each change into them:

- app.js: signIn() before the first API call.
- center.js: the run stream uses api.stream; Apply (button, y, or y from
  the transcript) sends diffSha256 of the diff as shown; a cut diff shows
  the warning and needs the box ticked before Apply or y works.
- panels.js, menubar.js: report downloads go through api.download, since a
  plain link cannot carry the session header (it now gets a 401).
- right.js: session wording in Settings.
- tests/ui.test.js: static files need no session now; the font and logo
  test checks type and content instead.
- docs/dashboard.md: the cut-diff and SHA-256 rules move into the approval
  window paragraph; reopening a run points at the menu bar and History.
- scripts/dashboard-screenshots.mjs: the #t= link, and globalThis for page
  globals (new lint step).

AI-Assisted: Claude Code with Claude Opus 5.5; resolved the merge conflicts
  and ported main's session and approval changes into the redesign
Co-Authored-By: Claude Opus 5.5 <[email protected]>
@danoszz
danoszz deleted the refactor/cleanup branch October 20, 2026 17:08
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