Skip to content

Hoist the log tag into one constant per file #488

Description

@Pixnop

Summary

Every log line in the host layer rebuilds its provenance prefix by hand. Counted on origin/dev: 199 [back] ... tags across 17 files, and the retyping has already drifted into two spellings for the same file in two of them. The renderer solved this a while ago (18 files hoist a LOG_TAG constant and interpolate it) and one host file already does too. The rest never followed.

This is not a cosmetic sweep. A quarter of gameHandlers.ts's log lines carry a different tag from the other three quarters, so grepping the logs for that file's lines misses them.

Total estimated saving: about 9 KB of repeated literal, and one provenance spelling per file by construction.

1. The per-file tag is retyped 199 times across the host layer

src/ipc/handlers/gameHandlers.ts:161 and 198 other sites

Measured on origin/dev:

file tags
src/ipc/handlers/gameHandlers.ts 63
src/ipc/handlers/modsHandlers.ts 33
src/ipc/handlers/pathsHandlers.ts 25
src/main/index.ts 15
src/config/configManager.ts 14
src/ipc/adapters/modScan.ts 10
src/ipc/handlers/utilsHandlers.ts 9
src/ipc/handlers/netHandlers.ts 9
src/main/orphanedTempFiles.ts 7
src/ipc/handlers/accountHandlers.ts 7
7 more files 1 each

Replace with: the pattern the codebase already has. src/ipc/handlers/backgroundHandlers.ts:14 declares const LOG_PREFIX = "[back] [ipc] [ipc/handlers/backgroundHandlers.ts]" and interpolates it; 18 renderer files (useServerMods, useModBatchActions, useInstalledModActions, useModProfiles, adapters/errorLog, and the rest) do the same under the name LOG_TAG. One const per file, the channel name at the call site. One spelling per file by construction, and renaming a file becomes a one-line change.

Savings: about 9 KB of repeated literal, and the drift closed.

Risk and test: do not wrap logMessage in a per-file tagged helper. tests/log-provenance.test.ts scans source text for logMessage( / logMods( / window.api.utils.logMessage( followed by a template literal and checks every interpolated identifier; a wrapper under a different name would silently take all those call sites out of the scan. Keeping the literal logMessage("info", \${LOG_TAG} [CHANNEL] ...`)shape keeps them in it, and pulls the currently unscanned plain-string calls into the scan as well. The test must still pass and its match count must not drop.TAG` is not one of the risky words the test looks for, so a fixed constant renders byte-identical and passes untouched.

Worth: high.

2. Two files already log under two spellings of their own name

src/ipc/handlers/gameHandlers.ts:400, src/config/configManager.ts, src/main/orphanedTempFiles.ts, src/ipc/handlers/accountHandlers.ts

Counted on origin/dev:

  • gameHandlers.ts: 47 lines under [back] [ipc] [ipc/handlers/gameHandlers.ts] and 16 under the shorter [back] [ipc] [gameHandlers.ts].
  • configManager.ts: 9 under [back] [config] [config/configManager.ts] and 5 under [back] [config] [configManager.ts].
  • orphanedTempFiles.ts (7 tags) and accountHandlers.ts (7 tags) log under bare file names while their neighbours are path qualified.

git blame shows the short spelling came first and the path-qualified one arrived later in two separate commits, so this is organic drift, not a choice: neither PR #441 ("Keep paths and names out of the logs and pin it at the source") nor #468 ("Keep file names out of the task log tags") discusses keeping two spellings.

Replace with: the path-qualified spelling everywhere, hoisted as in item 1, starting with gameHandlers.ts and pathsHandlers.ts since they carry 88 of the 199 tags between them.

Savings: one spelling per file instead of two, and a prefix grep that finds all of a file's lines instead of three quarters of them.

Risk and test: the 16 short tags change text, which is the point. The rewritten long-form prefixes must render byte-identical to today's long spelling. tests/log-provenance.test.ts pins the shape of what is interpolated, not the fixed prose, and tests/ipc/gameHandlers.test.ts and tests/security-boundaries.test.ts read the file's source for IPC-channel assertions unrelated to log text; all stay green.

Suggested order

  1. gameHandlers.ts and pathsHandlers.ts first: 88 tags, both spellings, biggest single win and the clearest proof the pinned test does not move.
  2. modsHandlers.ts and modScan.ts next, 43 tags between them, one spelling each so it is mechanical.
  3. configManager.ts, orphanedTempFiles.ts, accountHandlers.ts: the remaining drifted spellings.
  4. The single-tag files last, or never: one occurrence is not worth a constant, and saying so in the PR is fine.

One file per commit, so a reviewer can diff the rendered strings rather than the diff.

Out of scope

The log provenance rule itself: tests/log-provenance.test.ts exists so a log line names where it came from and never interpolates a path, name, host or address, and this change strengthens that scan rather than relaxing it. The redaction in logMessage stays. So do the hexagonal split (pure src/domain, src/ipc and src/main as host, the renderer through window.api and feature adapters), the path policy, the IPC validation at the boundary, the mutation-tested guards and the accessibility work. tests/security-boundaries.test.ts, tests/text-contrast.test.ts and tests/i18n/i18n-parity.test.ts keep their rules (no HTML sinks, contrast floors, locale parity) untouched.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: hostMain process, IPC handlers, workerstech debtInherited debt, tracked to be paid down

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions