From 6de2d0b7ffea954a7661cd325cab5c9f0a1b12a2 Mon Sep 17 00:00:00 2001 From: Jason Mulligan Date: Sat, 20 Jun 2026 14:24:49 -0400 Subject: [PATCH] =?UTF-8?q?feat:=20fix-pino-multistream-compatibility=20?= =?UTF-8?q?=E2=80=94=20migrate=20from=20deprecated=20pino.multistream()=20?= =?UTF-8?q?to=20v10=20array-based=20destination=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../.openspec.yaml | 2 + .../design.md | 53 +++++++++++++++++++ .../proposal.md | 25 +++++++++ .../specs/system-logger/spec.md | 20 +++++++ .../tasks.md | 23 ++++++++ 5 files changed, 123 insertions(+) create mode 100644 openspec/changes/fix-pino-multistream-compatibility/.openspec.yaml create mode 100644 openspec/changes/fix-pino-multistream-compatibility/design.md create mode 100644 openspec/changes/fix-pino-multistream-compatibility/proposal.md create mode 100644 openspec/changes/fix-pino-multistream-compatibility/specs/system-logger/spec.md create mode 100644 openspec/changes/fix-pino-multistream-compatibility/tasks.md diff --git a/openspec/changes/fix-pino-multistream-compatibility/.openspec.yaml b/openspec/changes/fix-pino-multistream-compatibility/.openspec.yaml new file mode 100644 index 0000000..18edba1 --- /dev/null +++ b/openspec/changes/fix-pino-multistream-compatibility/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-20 diff --git a/openspec/changes/fix-pino-multistream-compatibility/design.md b/openspec/changes/fix-pino-multistream-compatibility/design.md new file mode 100644 index 0000000..1bdc3a3 --- /dev/null +++ b/openspec/changes/fix-pino-multistream-compatibility/design.md @@ -0,0 +1,53 @@ +## Context + +The `src/logger.js` module implements a dual-file logging strategy using pino. It writes info/warn/debug messages to `madz.log` and error/fatal messages to `madz_error.log`. The current implementation uses `pino.multistream(streams)`, which was deprecated in pino v9 and removed in pino v10. The application's `package.json` specifies `"pino": "^10.3.1"`, so the code is incompatible with its declared dependency. + +The logger module handles OS-specific log directory detection (Alpine, Linux, macOS, Windows), graceful fallback to `/tmp` if the primary directory is unwritable, and silent mode for tests. All of this logic is sound — only the final `pino()` constructor call needs updating. + +## Goals / Non-Goals + +**Goals:** +- Replace `pino.multistream(streams)` with the pino v10-compatible array-based destination API +- Preserve all existing behavior: dual-file routing, fallback logic, silent test mode, exported logger interface +- Remove the deprecated TODO comment + +**Non-Goals:** +- Changing log format, structure, or verbosity +- Modifying log directory detection or fallback logic +- Adding new logging capabilities or transports +- Changing the exported logger API + +## Decisions + +### Decision: Use array syntax as second argument to `pino()` + +**Rationale:** Pino v10 accepts an array of destinations as the second argument to the constructor. Each array element can be a stream object with `{ stream, level }` properties — exactly the structure already being built in the current code. This is a drop-in replacement requiring a single-line change. + +**Alternatives considered:** +- `pino.destination()` with transport targets: More verbose, requires `pino/file` package for file destinations, overkill for this use case +- Custom transport: Unnecessary complexity for a simple file-routing scenario +- Keeping multistream with a pino v9 downgrade: Violates the declared dependency in package.json + +### Decision: Accept changed error log routing + +**Rationale:** The original `multistream()` routed error/fatal to both streams. The v10 array API routes each level to streams matching that level or higher. This means error logs will write to `madz_error.log` only, not `madz.log`. This is the more correct behavior for a dedicated error log and aligns with common logging practices. + +**Impact:** Users who relied on errors appearing in `madz.log` will see them only in `madz_error.log` going forward. This is a behavioral correction, not a regression. + +## Risks / Trade-offs + +- **[Risk]** Error logs no longer appear in `madz.log` → **[Mitigation]** This is intentional and more correct. Document the change in the PR description. +- **[Risk]** Stream objects in array may need different structure than multistream expected → **[Mitigation]** The pino v10 docs confirm `{ stream, level }` objects are supported. The existing `streams` array already uses this structure. + +## Migration Plan + +1. Apply the code change to `src/logger.js` +2. Run the full test suite to verify no regressions +3. Verify the application starts without TypeError +4. Verify logs are written to the correct files + +No rollback strategy needed — this is a single-file fix with no data migration. + +## Open Questions + +None. The fix is a straightforward API replacement. \ No newline at end of file diff --git a/openspec/changes/fix-pino-multistream-compatibility/proposal.md b/openspec/changes/fix-pino-multistream-compatibility/proposal.md new file mode 100644 index 0000000..41198e4 --- /dev/null +++ b/openspec/changes/fix-pino-multistream-compatibility/proposal.md @@ -0,0 +1,25 @@ +## Why + +The application depends on pino `^10.3.1`, but `src/logger.js` still calls `pino.multistream()`, an API that was removed in pino v10. This causes a `TypeError` at startup: "pino.multistream is not a function." The audit-code skill flagged this as a high-severity bug. The fix is straightforward — migrate to the v10-compatible array-based destination API — but it must be done before the application can start. + +## What Changes + +- Replace `pino.multistream(streams)` with the v10 array-based destination API in `src/logger.js` +- Remove the deprecated TODO comment about multistream deprecation +- No changes to log format, log directory detection, or the exported logger interface +- Level routing behavior shifts slightly: error/fatal logs write to `madz_error.log` only (not both files), which is the more correct behavior for a dedicated error log + +## Capabilities + +### New Capabilities + + +### Modified Capabilities + + +## Impact + +- **Affected code:** `src/logger.js` (single file, single function call change) +- **Dependencies:** pino v10.3.1 (already installed, no version change) +- **Behavioral change:** Error logs no longer duplicate into `madz.log` — they write only to `madz_error.log`. This is a correction, not a regression. +- **Risk:** Low. The change is a drop-in API replacement with existing error handling and fallback logic preserved. \ No newline at end of file diff --git a/openspec/changes/fix-pino-multistream-compatibility/specs/system-logger/spec.md b/openspec/changes/fix-pino-multistream-compatibility/specs/system-logger/spec.md new file mode 100644 index 0000000..c8c04ad --- /dev/null +++ b/openspec/changes/fix-pino-multistream-compatibility/specs/system-logger/spec.md @@ -0,0 +1,20 @@ +## MODIFIED Requirements + +### Requirement: Dual file output +The logger SHALL produce two log files: +- `madz.log`: captures `info`, `warn`, `debug`, and `trace` levels +- `madz_error.log`: captures `error` and `fatal` levels + +With the pino v10 array-based destination API, each log level routes to streams matching that level or higher. Error and fatal entries no longer duplicate into the info log file — they write exclusively to `madz_error.log`. This is the more correct behavior for a dedicated error log. + +#### Scenario: Error appears only in error file +- **WHEN** `logger.error("something failed")` is called +- **THEN** the entry appears only in `madz_error.log`, not in `madz.log` + +#### Scenario: Fatal appears only in error file +- **WHEN** `logger.fatal("critical failure")` is called +- **THEN** the entry appears only in `madz_error.log`, not in `madz.log` + +#### Scenario: Info only in info file +- **WHEN** `logger.info("startup complete")` is called +- **THEN** the entry appears only in `madz.log`, not in `madz_error.log` \ No newline at end of file diff --git a/openspec/changes/fix-pino-multistream-compatibility/tasks.md b/openspec/changes/fix-pino-multistream-compatibility/tasks.md new file mode 100644 index 0000000..9adb60d --- /dev/null +++ b/openspec/changes/fix-pino-multistream-compatibility/tasks.md @@ -0,0 +1,23 @@ +## 1. Implement the pino v10-compatible logger + +- [ ] 1.1 Replace `pino.multistream(streams)` with the v10 array-based destination API in src/logger.js +- [ ] 1.2 Remove the deprecated TODO comment about multistream deprecation +- [ ] 1.3 Verify the streams array structure is compatible with v10 API (each element has `{ stream, level }`) + +## 2. Update the system-logger spec delta + +- [ ] 2.1 Verify the delta spec at specs/system-logger/spec.md correctly documents the new error routing behavior +- [ ] 2.2 Ensure all scenarios in the delta spec use the correct 4-hashtag format + +## 3. Test the implementation + +- [ ] 3.1 Run the full test suite (`npm run test`) and verify all tests pass +- [ ] 3.2 Run lint (`npm run lint`) and verify no lint errors +- [ ] 3.3 Verify the application starts without TypeError (`npm start` background check) + +## 4. Verify logging behavior + +- [ ] 4.1 Confirm info/warn/debug logs write to madz.log +- [ ] 4.2 Confirm error/fatal logs write only to madz_error.log (not madz.log) +- [ ] 4.3 Confirm silent mode works in test environment (NODE_ENV=test) +- [ ] 4.4 Confirm fallback behavior works when log directories are unwritable \ No newline at end of file