Skip to content

Cleanup: one unused local, and the negative result for everything else - #420

Merged
philcunliffe merged 1 commit into
masterfrom
autophagy/cleanup-2026-07-28
Jul 28, 2026
Merged

Cleanup: one unused local, and the negative result for everything else#420
philcunliffe merged 1 commit into
masterfrom
autophagy/cleanup-2026-07-28

Conversation

@philcunliffe

Copy link
Copy Markdown
Contributor

A full-tree mechanical sweep for dead code. One trim survived. Almost everything this pass looked at came back alive, and the bulk of the value here is the negative result: the searches below are recorded so the next pass does not repeat them.

The one trim

test/core/remote-login-command.test.js:444 - unused local err

 test('a session without a gateway credential seeds nothing and prints no forwarding output', async () => {
const hypHome = await tmpHome()
- const { ctx, out, err } = await makeCtx({+ const { ctx, out } = await makeCtx({

err is destructured out of makeCtx() and never read anywhere in that test body. The test asserts on out (assert.doesNotMatch(out.join(''), /seeded|provisioned/)) and on the filesystem; it never touches stderr.

Reachability evidence:

  • tsc -p tsconfig.json --noEmit --noUnusedLocals reports exactly test/core/remote-login-command.test.js(444,21): error TS6133: 'err' is declared but its value is never read. After the change that diagnostic is gone and no new one appears.
  • Reading the whole test body (lines 442-459): no occurrence of err.
  • This is a block-scoped binding inside a single test() callback, so it has no reachability surface at all beyond those 18 lines. Nothing imports it, no string can name it, no plugin kernel can dispatch to it.
  • makeCtx() is unchanged and still returns the same object; only the binding is dropped. The three sibling tests in the same file that destructure { ctx, out, err } or { ctx, err } and do assert on err are untouched.

Not a package entry point (test/ is not in package.jsonfiles), not a CLI surface, no @ref LLP annotation attached.

What was searched and came back alive

Recording these so the next pass can skip them.

Exports never referenced outside their own file: 82 found, 0 taken. A scan of all 1228 export function|class|const|let|var declarations across src/, bin/, scripts/, hypaware-core/, and test/ found 82 whose name appears in no other .js file in the tree. Removing the export keyword from any of them is an API change, not a dead-code trim, so all 82 were left alone - consistent with the previous pass (#408), which deferred the same category.

Exports with zero references anywhere, including their own file: exactly 3. Re-running the scan against every .js, .ts, .json, .md, .toml, and .yaml file in the tree, only three exported symbols have a single occurrence (the declaration itself) and no mention anywhere else:

  • getGascityRuntime (hypaware-core/plugins-workspace/gascity/src/runtime.js:35)
  • stopSystemdUnit (src/core/daemon/linux.js:286)
  • stopLaunchAgent (src/core/daemon/macos.js:357)

These are precisely the three the previous pass deliberately left alone: a set/get/require trio member, and two members of a symmetric platform-adapter lifecycle family. This pass found no new evidence to overturn that, so they stay. The useful outcome is the confirmation that the "zero references anywhere" set is only these three - there is no fourth candidate hiding.

Never-imported modules: none. An import graph was built by resolving every from '...', import('...'), and require('...') specifier across the whole tree, then checked against all 325 non-test, non-smoke-flow .js modules. 21 came back with no importer, and every one is a declared entrypoint: bin/hypaware.js (package.jsonbin), src/core/index.js / src/core/query/index.js / src/core/sinks/index.js (package.jsonexports), hypaware-core/smoke/index.js (package.jsonscripts.smoke), 14 plugin index.js files (each named as entrypoint in its hypaware.plugin.json), and two skills/hypaware-report-to-html/assets/copy-md.js files (loaded by <script defer src="assets/copy-md.js"> in assets/head.html). Test files and hypaware-core/smoke/flows/* are discovered dynamically by name and were excluded from the candidate set for that reason.

Unreachable statements: none.tsc -p tsconfig.json --noEmit --allowUnreachableCode false reports zero TS7027. A separate AST walk over every statement list (Block, SourceFile, ModuleBlock, CaseClause, DefaultClause), looking for anything following a return / throw / continue / break, plus a pass for statements after process.exit() / process.abort(), also found zero.

Commented-out code blocks: none. Runs of two or more consecutive // lines whose content parses as JS statements: zero hits. A deliberately looser scan surfaced four blocks (hypaware-core/plugins-workspace/claude/src/index.js:108-116, hypaware-core/plugins-workspace/codex/src/index.js:89-96, src/core/commands/clients.js:457-498, src/core/daemon/runtime.js:324-343); all four were read and are prose rationale carrying @ref LLP annotations, not code.

Constant-false guards, write-only locals: none. No if (false) / if (0) / while (false) / && false / || true. The only single-occurrence local bindings are src/core/cache/purge.js:155 (for await (const _row of ...)) and src/core/cache/retention.js:201 (for await (const _ of ...)), both intentional discard bindings in counting loops.

Unused JSDoc @import type names: ~100, 0 taken. Still the largest mechanical signal in the tree (TS6196 plus JSDoc-sourced TS6133/TS6192), and still wide, low-value churn. Deferred, same as last pass.

Deliberately not touched

Two files import the same module twice:

  • src/core/cli/remote_commands.js:20 and :24 both import from ../remote/gateway_seed.js
  • src/core/commands/clients.js:13 and :16 both import from ../runtime/client_assets.js

Every imported name in all four statements is used. These are redundant statements, not dead bindings, so folding them together would be style churn rather than a dead-code trim. Left alone.

Also flagged and not acted on: pidFilePath is re-exported from src/core/daemon/runtime.js:1089 but every consumer imports it from src/core/daemon/pid.js instead. Dropping a name from an export list is an API change, so it stays.

Verification

npm test - unchanged, no regression:

beforeafter
tests27432743
pass27342734
fail88
skipped11

All 8 failures are the pre-existing ones in test/core/leave-command.test.js (not ok 796, 797, 798, 800, 801, 802, 803, 804), identical before and after by name and number. No other file fails. This change adds none.

npm run typecheck - clean, no output.

npm run build:types - the emitted types/ tree is byte-identical before and after: diff -r types-before types produces no output across all 340 files. This is the expected result and worth stating plainly, since tsconfig.build.json sets rootDir: src and the change is in test/; the published declaration surface could not have moved, and the diff confirms it did not.

Generated by neutral's code-cleanup initiative (LLP 0036). Proposed, not asserted: every trim above is a claim to check, not a fact to trust.

The only mechanically dead construct a full-tree sweep turned up this
pass. `err` is destructured from `makeCtx()` and never read in that
test body; the sibling tests that do assert on stderr keep theirs.
Emitted `types/` tree is byte-identical before and after (340 files),
which it must be: the change is in `test/`, outside the
`tsconfig.build.json` rootDir.
Generated by neutral's code-cleanup initiative (LLP 0036).
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Calibration for whoever reviews this: the diff is one line, and closing this unmerged is a perfectly reasonable call.

This pass found essentially nothing, which is the expected result now that #408 took the obvious pickings. The single trim is an unused err binding inside one test callback. It is genuinely dead (tsc --noUnusedLocals flags exactly that position and no other in the file, and a block-scoped binding in an 18-line callback has no reachability surface), but it is worth roughly nothing on its own.

The part that may be worth more than the diff is the negative result, so it is recorded here for the next pass rather than being re-derived:

  • 1228 exported symbols scanned. 82 are referenced in no other .js file; all left alone, since removing export is an API change.
  • Exports with zero references anywhere in the tree (including their own file and all .md/.json/.toml): exactly three, and they are precisely the three Cleanup: drop five Phase 0 placeholder modules and unreferenced imports #408 deliberately spared (getGascityRuntime, stopSystemdUnit, stopLaunchAgent). The useful finding is that there is no fourth.
  • Never-imported modules: none. A real import graph over all 325 non-test modules leaves 21 with no importer, and every one is a declared entrypoint (bin/hypaware.js, three exports barrels, the smoke index, 14 plugin index.js files named in their manifest entrypoint, and two scripts loaded via <script src> in assets/head.html).
  • Unreachable statements: none (--allowUnreachableCode false reports zero, and a separate AST walk including after process.exit agrees).
  • Commented-out code: none. Four candidates surfaced by a loose scan turned out to be prose rationale carrying @ref LLP annotations.
  • The newly merged Show the user their own rows at the end of hyp init #407 code (overview.js, first_look.js, stream_errors.js) had nothing mechanically dead in it.

Two things were found and deliberately left, since they are style churn rather than trims: duplicate import statements at remote_commands.js:20/:24 and clients.js:13/:16 (every imported name in all four is live), and pidFilePath re-exported from daemon/runtime.js:1089 though every consumer imports it from daemon/pid.js (dropping it from an export list is an API change).

build:types emits an identical 340-file tree before and after, as expected for a test/ change. npm test is 2743 tests with the same 8 pre-existing leave-command.test.js failures before and after.

Generated by neutral's code-cleanup initiative (LLP 0036). Proposed, not asserted.

@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Review round 1 - 6f4cea3

Verdict: clean. No findings. Review kept proportionate to a one-line change.

  • The binding is genuinely unused. Read the whole callback at test/core/remote-login-command.test.js:442-459 ("a session without a gateway credential seeds nothing..."): only ctx and out are referenced, in runRemoteLogin, assert.equal(code, 0), and two out.join('') calls. No use of err in assertions, template literals, or comments.
  • Only the one callback was touched. Sibling tests at :425-440 and :461+ still destructure and use err (e.g. assert.match(err.join(''), /enrollment failed/)) and are unaffected.
  • The diff really is one line in one file, confirmed against origin/master.
  • The test still asserts what it did before: assert.equal(code, 0), assert.doesNotMatch(out.join(''), /seeded|provisioned/), and assert.rejects(fs.access(persistedPath)) are all unchanged.
  • Master moved to 701982c and that commit does touch both remote-login-command.test.js and remote_commands.js, so this was worth checking rather than assuming. The branch is diverged by one commit each way, and a trial merge auto-merged with no conflicts. Reset cleanly afterward.

Verification run

node --test test/core/remote-login-command.test.js: 59 tests, 59 pass, 0 fail. npm test: 2743 tests, 2734 pass, 8 fail, 1 skipped, all 8 in test/core/leave-command.test.js and still the only failing file, count unchanged. npm run typecheck clean.

See the note above on this PR for the calibration: the diff is worth little on its own, and the negative search result recorded there is the more useful artifact. Closing this unmerged remains a perfectly reasonable call.

@philcunliffe
philcunliffe marked this pull request as ready for review July 28, 2026 01:19
@philcunliffephilcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Jul 28, 2026
@philcunliffe
philcunliffe merged commit 322db2d into masterJul 28, 2026
8 checks passed
@philcunliffe
philcunliffe deleted the autophagy/cleanup-2026-07-28 branch July 28, 2026 05:31
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:approvedneutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@philcunliffe