V2.1.0 rc #1
V2.1.0 rc #1
Conversation
- Implements strict Hexagonal Architecture and DDD principles - Adds comprehensive unit tests for domain entities and services - Formalizes error handling with custom domain errors - Standardizes project structure matching @git-stunts/plumbing - Includes architecture and contribution documentation
…treaming log parser, and docker-only safety guards across all blocks.
Applied all AUDITS.md security and quality fixes: Security Hardening: - Add 5MB message size limit to prevent DoS attacks - Implement 100-char max on trailer keys (ReDoS prevention) - Fix regex consistency between schema and service parsing - Use TRAILER_KEY_REGEX constant across validation boundaries Code Quality: - Extract _findTrailerStartIndex() private method - Add comprehensive JSDoc to decode algorithm - Replace z.array(z.any()) with strict z.array(GitTrailerSchema) - Remove stale test imports Package Metadata: - Update package.json with repository URLs, keywords, engines - Change plumbing dependency from file: to npm version ^2.7.0 - Add standard files: LICENSE, NOTICE, SECURITY.md, CODE_OF_CONDUCT.md - Create GitHub Actions CI workflow Documentation: - Enhance README with badges, validation rules table, security section - Add detailed usage examples - Document all constraints and error types - Update CHANGELOG with security fixes All 23 tests passing in Docker. Ready for npm publication.
Comprehensive re-evaluation of trailer-codec after security fixes. Health Score: 9/10 - Production Ready - Zero critical issues remaining - All AUDITS.md mitigations verified - Documentation complete and accurate - Dependencies healthy - Architecture exemplary Status: APPROVED FOR NPM PUBLICATION
Identified two key code snippets with high educational value: 1. Backward Walk Algorithm (_findTrailerStartIndex) - Demonstrates Git protocol expertise - Shows optimization via domain knowledge - Foundation for git-cms stunt 2. Key Normalization (GitTrailer.js:13) - Mirrors Git's internal behavior - Subtle but critical for query compatibility - Example of production-ready stunt engineering Blog angle: 'Most parsers walk forward. This one walks backward.' Rationale: Trailers are at the END, so backward is optimal. Use cases: - Git Stunts #1 (Git as CMS) intro material - Architecture patterns: boring engineering + wild ideas - Teaching systems thinking via Git object model
…ral cleanup - API Ergonomics: Added `encode()`/`decode()` aliases to `TrailerCodec` for improved DX and synchronized `FacadeAdapter` error handling. - Refactor: Reduced cyclomatic complexity in `GitCommitMessage`, `TrailerCodecService`, and `GitTrailer` constructors by extracting validation logic into private methods. - Infrastructure: Removed unnecessary Docker artifacts (`Dockerfile`, `docker-compose`) to align with hexagonal architecture principles (pure domain logic/zero I/O dependencies). - Documentation: Added comprehensive JSDoc to all `index.js` exports, created `docs/MIGRATION.md`, and expanded `SECURITY.md` with DoS protection strategies. - Quality Assurance: Resolved all ESLint complexity warnings and migrated CI workflow to direct Node.js execution. Refs: #v2.1.0
|
Warning Rate limit exceeded@flyingrobots has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughAdds a new npm package Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Facade as FacadeAdapter/TrailerCodec
participant Service as TrailerCodecService
participant Normalizer as MessageNormalizer
participant Parser as TrailerParser
participant Entity as GitCommitMessage
Client->>Facade: decodeMessage(raw)
activate Facade
Facade->>Service: decode(raw)
activate Service
Service->>Normalizer: guardMessageSize(raw)
Normalizer-->>Service: ok / throws TrailerTooLargeError
Service->>Normalizer: normalizeLines(raw)
Normalizer-->>Service: lines[]
Service->>Parser: split(lines)
Parser-->>Service: { bodyLines, trailerLines, trailerStart } or throws TrailerNoSeparatorError
Service->>Entity: new GitCommitMessage({ title, body, trailers })
Entity-->>Service: GitCommitMessage
Service-->>Facade: GitCommitMessage
Facade-->>Client: result
deactivate Facade
sequenceDiagram
participant Client
participant Facade as FacadeAdapter/TrailerCodec
participant Service as TrailerCodecService
participant Entity as GitCommitMessage
Client->>Facade: encodeMessage(payload)
activate Facade
alt payload is GitCommitMessage
Facade->>Entity: toString()
Entity-->>Facade: formatted string
else payload is plain object
Facade->>Service: encode(payload)
Service->>Entity: new GitCommitMessage(payload)
Entity-->>Service: GitCommitMessage
Service->>Entity: toString()
Entity-->>Service: formatted string
Service-->>Facade: formatted string
end
Facade-->>Client: encoded string
deactivate Facade
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In @API_REFERENCE.md:
- Line 8: The API reference currently labels the functions as "Deprecated
convenience wrapper[s]" but omits the migration guidance present in the source
@deprecated tags; update the two entries that read "Deprecated convenience
wrapper around `new TrailerCodec().decode(message)`" to instead say "Deprecated
convenience wrapper around `new TrailerCodec().decode(message)`. Use
`TrailerCodec` instances instead." and, if there is a planned removal version,
append a short deprecation timeline (e.g., "to be removed in v3.0") or
explicitly state they remain as compatibility shims; cross-check the @deprecated
notes in index.js (the tags that read "Use TrailerCodec class instead") to
ensure wording matches.
In @CONTRIBUTING.md:
- Around line 25-33: The fenced code block showing the directory tree (starting
with "src/" and lines under "domain/") is missing a language specifier; update
the opening fence from ``` to ```text (or another appropriate language like
```bash) so the block is labeled (e.g., ```text) to improve rendering and
accessibility.
In @docs/ADVANCED.md:
- Around line 10-13: The keyPattern manipulation is broken: stop using
string.replace('-', '.') and appending '\\.+'; either supply an explicit correct
regex (e.g. keyPattern: '[A-Za-z0-9_.]+') or, if you must derive from
TRAILER_KEY_RAW_PATTERN_STRING, surgically replace hyphens inside the character
class with a global replacement and do NOT append a literal-dot requirement —
for example, derive newPattern by using a regex that targets the [...] class and
does cls.replace(/-/g, '.') on that capture, then pass that newPattern to
createGitTrailerSchemaBundle (refer to createGitTrailerSchemaBundle and
TRAILER_KEY_RAW_PATTERN_STRING).
In @docs/INTEGRATION.md:
- Around line 33-35: The current commit parsing in the process.stdin.on('end'
...) handler is fragile because it splits on '\n\n' which breaks when commit
bodies contain blank lines; update the logic to use a robust delimiter: emit
commits from git using a null-prefixed format (e.g., --format=%x00%B or similar)
and then split buffer on '\0' instead of '\n\n' (keep the existing
filter(Boolean) to drop empties), or at minimum add a note in INTEGRATION.md
documenting the limitation; locate the buffer/commits handling inside the
process.stdin.on('end' callback and change the git invocation to use the null
delimiter and the split call to buffer.split('\0').
- Around line 55-61: The example mixes ESM and CommonJS: it uses ESM imports for
decodeMessage and formatBodySegment but calls require('readline') for reader;
replace the CommonJS require with an ESM import for the Node readline module and
then call the existing createInterface usage on that imported readline object
(i.e., import readline and then use readline.createInterface({ input:
process.stdin }) to initialize reader) so the example consistently uses ESM
syntax.
In @docs/MIGRATION.md:
- Around line 256-259: The grep pattern is using incorrectly escaped brackets
and won't match uppercase trailer keys; replace it with an extended or PCRE
pattern. Update the command to use an extended regex such as grep -rE
"trailers\['[A-Z]" src/ or a stricter PCRE like grep -rP
"trailers\['[A-Z][a-zA-Z-]*'\]" src/ so the character class [A-Z] is interpreted
correctly and full trailer keys (e.g., trailers['Foo-Bar']) are matched.
- Around line 199-210: The docs import of MessageNormalizer is invalid because
MessageNormalizer is not part of the package public API; either export it from
the package entry (add an export for MessageNormalizer from index.js that
re-exports src/domain/services/helpers/MessageNormalizer.js) so the example can
keep using new MessageNormalizer(...), or update the MIGRATION.md example to
avoid direct instantiation and instead show using the public API (e.g., pass a
configuration object to TrailerCodecService or use the existing public
factory/constructor options) and replace the import of MessageNormalizer with
only public exports such as TrailerCodecService.
In @docs/SERVICE.md:
- Around line 25-27: The table cell for step 3 contains literal newline
characters instead of escaped sequences; update the
MessageNormalizer.normalizeLines() description to show the actual escape
sequences by replacing the raw newlines with code spans like `\n` and `\r\n`
(e.g., "MessageNormalizer.normalizeLines() converts `\n` to `\r\n` and splits
into lines.") so the markdown table renders correctly and the backslashes are
preserved.
- Around line 12-14: The table row for messageNormalizer contains literal
newline characters that break the markdown table; edit the `messageNormalizer`
row (referencing `MessageNormalizer` and `TrailerTooLargeError`) to escape or
render line endings as inline code (e.g., use `\n` and `\r\n` inside backticks)
and ensure the cell remains a single line with the required pipe separators;
also keep the note about guarding against messages > 5 MB (throws
`TrailerTooLargeError`) and how to override max size or normalization logic.
In @scripts/pre-commit.sh:
- Around line 1-2: The script uses a POSIX sh shebang but also sets the
non-POSIX option `pipefail` in the `set -euo pipefail` line, which will break on
shells like dash; fix by making the script consistently Bash or POSIX: either
change the shebang from `/usr/bin/env sh` to `/usr/bin/env bash` so `pipefail`
is valid, or if you need strict POSIX compatibility remove `pipefail` from the
`set -euo pipefail` invocation (leaving `set -eu`), then run the script under a
POSIX shell.
In @src/domain/services/helpers/TitleExtractor.js:
- Around line 1-7: Remove the duplicate JSDoc block at the top of
TitleExtractor.js and keep only the complete JSDoc that includes the @param
annotation for the title extraction function (the block that documents "Extracts
the title line and the body start index" and contains "@param {string[]}
lines"). Ensure there is a single JSDoc immediately above the corresponding
function/export so the function comment is not duplicated.
In @src/domain/value-objects/GitTrailer.js:
- Around line 18-30: The constructor of GitTrailer validates raw inputs before
normalization, which allows whitespace-only values to pass the
GitTrailerSchema.min(1) check and then become empty after trim; to fix,
normalize the inputs before schema validation (e.g., call trim() on value and
lowercase on key prior to calling actualSchema.parse), or alternatively perform
an explicit post-parse check that ensures trimmed value.length > 0 and throw via
_handleValidationError if empty; update the constructor logic around
_validateSchema, the parse call, and the error handling in the constructor to
use the normalized values for validation (refer to constructor, _validateSchema,
GitTrailerSchema, _handleValidationError, this.key, this.value).
In @test/unit/domain/services/TrailerCodecService.test.js:
- Around line 66-73: The test "rejects trailers without a blank line separator"
currently uses a try/catch around service.decode(raw) so it will pass silently
if no exception is thrown; change the assertion to explicitly expect an
exception by replacing the try/catch with expect(() =>
service.decode(raw)).toThrow(TrailerNoSeparatorError) (or alternatively call
fail() right after service.decode(raw) inside the try block) so the test fails
when service.decode does not throw the TrailerNoSeparatorError.
- Around line 75-81: The test currently swallows failures by using try/catch;
replace the try/catch pattern in the 'rejects trailer values containing line
breaks' test with a proper Jest assertion that expects the synchronous call to
service._buildTrailers([...]) to throw TrailerValueInvalidError (e.g., use
expect(() => service._buildTrailers(['Key:
Value\nInjected'])).toThrow(TrailerValueInvalidError)); target the test named
'rejects trailer values containing line breaks' and call the _buildTrailers
method directly so the assertion fails if no exception is thrown.
In @TESTING.md:
- Line 29: The doc incorrectly references `.eslintrc.js`; update the TESTING.md
sentence that mentions `.eslintrc.js` to reference the new flat config filename
`eslint.config.js` (keep the `.prettierrc` mention and the lint/format command
instructions unchanged) so the docs match the PR's introduced config file.
🧹 Nitpick comments (28)
.github/workflows/ci.yml (1)
19-19: Consider usingnpm cifor reproducible CI builds.
npm ciis preferred overnpm installin CI environments as it ensures clean, reproducible installs frompackage-lock.jsonand fails if the lock file is out of sync.Proposed fix
- run: npm ci - - run: npm installApply to both jobs (lines 19 and 31).
Also applies to: 31-31
.gitignore (5)
3-3: Optional: Remove duplicatejspm_packages/entry.The pattern
jspm_packages/appears at both line 3 and line 61. Consider removing one of the duplicates for cleaner maintainability.♻️ Suggested cleanup
# Dependency directories jspm_packages/ - -# TypeScript v1 declaration files -typings/Also applies to: 61-61
17-17: Optional: Remove duplicate.cacheentry.The pattern
.cacheappears at line 17 (under "Build outputs") and line 95 (under "parcel-bundler cache"). Consider consolidating to avoid duplication.Also applies to: 95-95
12-12: Optional: Remove duplicatedist/entries.The pattern
dist/appears at lines 12, 104, and 111. Consider removing the duplicates for cleaner maintainability.Also applies to: 104-104, 111-111
14-14: Optional: Remove duplicate.nextentries.The pattern
.next(line 14) and.next/(line 99) both ignore Next.js build outputs. Consider keeping only one form for consistency.Also applies to: 99-99
18-18: Optional: Remove duplicateoutentries.The pattern
out/(line 18) andout(line 100) are duplicates. Consider removing one for cleaner maintainability.Also applies to: 100-100
SECURITY.md (1)
96-96: Optional: Wrap email address in angle brackets.Per markdown best practices, email addresses can be wrapped in angle brackets for improved parsing and rendering.
♻️ Suggested formatting
-If you discover a security vulnerability, please send an e-mail to james@flyingrobots.dev. +If you discover a security vulnerability, please send an e-mail to <james@flyingrobots.dev>.CHANGELOG.md (1)
43-49: Consider moving[unreleased]section to the top.Per Keep a Changelog convention, the
[unreleased]section should appear at the top of the changelog, before all versioned releases. This allows users to easily see what's coming next and makes it easier to promote unreleased changes when cutting a new version.Suggested reordering
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [unreleased] + +### Planned +- Integration test suite for real Git commit scenarios +- Performance benchmarks for large messages +- Additional edge case tests (unicode, empty input, concurrent usage) + ## [2.1.0] - 2026-01-11Then remove the
[unreleased]section from lines 43-49.eslint.config.js (1)
9-15: Consider using theglobalspackage for Node.js globals.Manual global definitions may miss some commonly used Node.js globals (e.g.,
setInterval,clearInterval,URL,URLSearchParams). Theglobalspackage provides comprehensive predefined sets.Example using globals package
import js from '@eslint/js'; +import globals from 'globals'; export default [ js.configs.recommended, { languageOptions: { ecmaVersion: 2022, sourceType: 'module', - globals: { - process: 'readonly', - Buffer: 'readonly', - console: 'readonly', - setTimeout: 'readonly', - clearTimeout: 'readonly', - }, + globals: { + ...globals.node, + }, },test/unit/domain/services/TrailerParser.test.js (1)
8-13: Consider consistency in array handling.Line 10 uses
lines.slice()to create a copy, but line 22 passeslinesdirectly. Whilesplit()doesn't mutate the input array, using.slice()inconsistently may confuse future readers.Suggested fix for consistency
Either remove
.slice()on line 10:- const { bodyLines, trailerLines } = parser.split(lines.slice()); + const { bodyLines, trailerLines } = parser.split(lines);Or add
.slice()to other tests for consistency.ARCHITECTURE.md (1)
21-29: Add language specifier to fenced code block and include adapters directory.The directory structure code block lacks a language specifier, and the
src/adapters/directory (containingCodecBuilder.js,FacadeAdapter.js) is missing from the structure.Suggested fix
-``` +```text src/ +├── adapters/ # CodecBuilder, FacadeAdapter ├── domain/ │ ├── entities/ # GitCommitMessage │ ├── errors/ # TrailerCodecError and validation subclasses │ ├── schemas/ # Zod schemas │ ├── services/ # TrailerCodecService │ └── value-objects/ # GitTrailer +└── index.js # Public API facade</details> </blockquote></details> <details> <summary>src/domain/services/helpers/BodyComposer.js (1)</summary><blockquote> `1-8`: **Duplicate JSDoc comment block.** Lines 1-3 and 4-8 describe the same functionality. Consider removing the redundant block. <details> <summary>Suggested fix</summary> ```diff -/** - * Trims leading/trailing blank lines from the decoded body block while preserving interior spacing. - */ /** * Trim leading/trailing blank lines while preserving interior spacing. * @param {string[]} lines * @returns {string} */src/domain/schemas/GitCommitMessageSchema.js (1)
12-16: Consider adding a default fortrailers.The
bodyfield has.default(''), buttrailershas no default. If the schema is used with.parse()on input lacking atrailersfield, it will fail validation. Consider adding.default([])for consistency if optional trailers are intended.Suggested change
export const GitCommitMessageSchema = z.object({ title: z.string().min(1), body: z.string().default(''), - trailers: z.array(GitTrailerSchema), + trailers: z.array(GitTrailerSchema).default([]), });src/domain/errors/TrailerCodecError.js (1)
13-13: GuardError.captureStackTracefor non-V8 environments.
Error.captureStackTraceis V8-specific and will throw in Firefox, Safari, or other non-V8 runtimes. Consider adding a guard:Suggested fix
- Error.captureStackTrace(this, this.constructor); + if (Error.captureStackTrace) { + Error.captureStackTrace(this, this.constructor); + }test/unit/domain/value-objects/GitTrailer.test.js (1)
35-61: Simplify complex test patterns.These tests use a convoluted try/catch/re-throw pattern. A cleaner approach captures the error directly:
Suggested simplification
- it('throws TrailerValueInvalidError when value includes newline', () => { - const attempt = () => { - try { - new GitTrailer('Key', 'Line\nBreak'); - } catch (error) { - expect(error).toBeInstanceOf(TrailerValueInvalidError); - throw error; - } - }; - - expect(attempt).toThrow(TrailerValueInvalidError); - }); - - it('includes documentation link in TrailerValueInvalidError metadata', () => { - const attempt = () => { - try { - new GitTrailer('Key', ''); - } catch (error) { - expect(error).toBeInstanceOf(TrailerValueInvalidError); - expect(error.meta.docs).toBe('docs/ADVANCED.md#custom-validation-rules'); - expect(/invalid trailer/i.test(error.message)).toBe(true); - throw error; - } - }; - - expect(attempt).toThrow(TrailerValueInvalidError); - }); + it('throws TrailerValueInvalidError when value includes newline', () => { + expect(() => new GitTrailer('Key', 'Line\nBreak')).toThrow(TrailerValueInvalidError); + }); + + it('includes documentation link in TrailerValueInvalidError metadata', () => { + let caughtError; + try { + new GitTrailer('Key', ''); + } catch (error) { + caughtError = error; + } + expect(caughtError).toBeInstanceOf(TrailerValueInvalidError); + expect(caughtError.meta.docs).toBe('docs/ADVANCED.md#custom-validation-rules'); + expect(caughtError.message).toMatch(/invalid trailer/i); + });src/adapters/CodecBuilder.js (1)
32-34: Inconsistent validation between options fields.
keyPatternandkeyMaxLengthgo through Zod validation (line 32), butparserOptions,formatters, andbodyFormatOptionsare extracted from rawoptions(line 34) and only checked to be objects. This could allow invalid nested structures to pass through.Consider validating all fields through the same Zod schema or document this as intentional pass-through behavior.
docs/PARSER.md (1)
9-11: Consider avoiding private method names in public documentation.Lines 9-11 reference private method names (
_findTrailerStartIndex,_validateTrailerSeparation,_trimBody). Documenting internal implementation details in user-facing docs creates tight coupling and may confuse users who cannot call these methods directly. Consider either:
- Rephrasing to describe behavior without naming private methods ("walks backward until a non-matching line", "validates blank line separation")
- Moving this implementation detail to internal developer documentation
📝 Suggested rephrasing
-3. **Walk backward** with `_findTrailerStartIndex` until either a non-matching line or an empty line appears; contiguous `Key: Value` patterns form the trailer block. -4. **Validate the separator**: `_validateTrailerSeparation` ensures there is a blank line before the trailers. Messages that omit the blank line now throw `TrailerNoSeparatorError`. -5. **Trim the body** without double allocations: `_trimBody` trims leading/trailing blank lines via index arithmetic and one `join`. +3. **Walk backward** until either a non-matching line or an empty line appears; contiguous `Key: Value` patterns form the trailer block. +4. **Validate the separator**: The parser ensures there is a blank line before the trailers. Messages that omit the blank line throw `TrailerNoSeparatorError`. +5. **Trim the body** without double allocations: leading/trailing blank lines are trimmed via index arithmetic and one `join`.API_REFERENCE.md (2)
39-44: Consider using JSDoc or plain JavaScript syntax for consistency.Lines 39-44 use TypeScript-style type annotations (
payload: { title: string; body?: string; ... }), but this appears to be a JavaScript library. This syntax will not work if users copy-paste it into JavaScript code. Consider either:
- Using JSDoc format:
/** * @param {Object} payload * @param {string} payload.title * @param {string} [payload.body] * @param {GitTrailerInput[]} [payload.trailers] * @param {Object} [options] */ new GitCommitMessage(payload, options);
- Using plain JavaScript with inline comments:
new GitCommitMessage( { title, body, trailers }, // payload object { trailerSchema, formatters } // optional options );
88-94: Clarify the distinction betweenTrailerInvalidErrorandTrailerValueInvalidError.Lines 92 and 93 document both
TrailerValueInvalidErrorandTrailerInvalidError, but their meanings overlap:
TrailerValueInvalidError: "trailer value violated theGitTrailerSchema"TrailerInvalidError: "Trailer key or value failed validation"It's unclear when each error is thrown. Consider clarifying:
- Is
TrailerInvalidErrorthe parent class for all trailer validation failures?- Is
TrailerValueInvalidErrorthrown specifically for value violations, whileTrailerInvalidErrorcovers key violations?- Or do they represent different validation stages?
Adding concrete examples for each would help users understand when to catch which error.
docs/ADVANCED.md (1)
62-62: Avoid referencing private methods in public documentation.Line 62 states "custom parsing strategies replace
_findTrailerStartIndexand_validateTrailerSeparationwithout subclassing the service." This references private implementation details that users shouldn't depend on.Consider rephrasing to focus on the behavior, not implementation:
-Pass your parser into `TrailerCodecService` so custom parsing strategies replace `_findTrailerStartIndex` and `_validateTrailerSeparation` without subclassing the service. +Pass your parser into `TrailerCodecService` to customize trailer detection and validation logic without subclassing the service.src/utils/zodValidator.js (1)
27-37: Consider exposing a reset mechanism for testing.The module-level
schemaRegistrysingleton works well for production, but tests that callregisterSchemamultiple times with the sametypeIdwill fail due to the duplicate check. Consider adding an internal_resetRegistry()function (or exportingschemaRegistryfor test access) to support test isolation.🧪 Proposed test helper
+/** + * Clears the registry. Intended for testing only. + * @internal + */ +export function _resetRegistry() { + schemaRegistry.clear(); +}src/domain/entities/GitCommitMessage.js (1)
91-102: Minor inconsistency: Extra whitespace in template literal.Line 98 has trailing whitespace before the closing backtick in the template literal (
${...} }\n). This appears intentional for formatting but is inconsistent with thetrimEnd()on line 101.🔧 Suggested cleanup
if (this.trailers.length > 0) { - message += `${this.trailers.map((t) => t.toString()).join('\n') }\n`; + message += `${this.trailers.map((t) => t.toString()).join('\n')}\n`; }package.json (1)
28-34: Consider pinning @babel/parser more tightly or documenting its purpose.The
@babel/parserdev dependency is unusual for a runtime package. If it's used for testing or tooling, consider adding a comment in the README or moving it to optional dependencies if not essential fornpm test.src/domain/services/TrailerCodecService.js (1)
142-150: Consider usingflatMapfor cleaner trailer extraction.The current implementation works correctly, but
flatMapwould be more idiomatic for this filter-and-transform pattern.♻️ Optional refactor
_buildTrailers(lines) { - return lines.reduce((acc, line) => { - const match = this.parser.lineRegex.exec(line); - if (match) { - acc.push(this.trailerFactory(match[1], match[2], this.schemaBundle.schema)); - } - return acc; - }, []); + return lines.flatMap((line) => { + const match = this.parser.lineRegex.exec(line); + return match + ? [this.trailerFactory(match[1], match[2], this.schemaBundle.schema)] + : []; + }); }src/adapters/FacadeAdapter.js (4)
12-38: Consider case-insensitive duplicate key detection.Looking at
GitTrailer(from relevant snippets), keys are normalized to lowercase (this.key = normalizedKey.toLowerCase()). The current duplicate check at line 28 uses the originaltrailer.key, which meansSigned-Off-Byandsigned-off-bywould not be detected as duplicates here, but would both normalize to the same key in the domain model.♻️ Suggested fix
function normalizeTrailers(entity) { if (!entity || !Array.isArray(entity.trailers)) { throw new TypeError('Invalid entity: trailers array is required'); } return entity.trailers.reduce((acc, trailer) => { if (!trailer || typeof trailer.key !== 'string' || trailer.key.trim() === '') { throw new TypeError('Invalid trailer: non-empty string key is required'); } if (typeof trailer.value !== 'string') { throw new TrailerInvalidError('Trailer value must be a string', { key: trailer.key, invalidValue: trailer.value, valueType: typeof trailer.value, }); } - if (acc[trailer.key] !== undefined) { - throw new TrailerInvalidError(`Duplicate trailer key: "${trailer.key}"`, { + const normalizedKey = trailer.key.toLowerCase(); + if (acc[normalizedKey] !== undefined) { + throw new TrailerInvalidError(`Duplicate trailer key: "${trailer.key}"`, { key: trailer.key, - existingValue: acc[trailer.key], + existingValue: acc[normalizedKey], duplicateValue: trailer.value, }); } - acc[trailer.key] = trailer.value; + acc[normalizedKey] = trailer.value; return acc; }, {}); }
55-64: Remove duplicate JSDoc block.There are two consecutive JSDoc comments before
createMessageHelpers. The first block (lines 55-57) is redundant.♻️ Suggested fix
-/** - * Advanced helper factory for tests or tools that need direct access to helpers. - */ /** * Advanced helper factory for tests or when you need to control the service instance. * @param {Object} [options] * @param {TrailerCodecService} [options.service] - Optional custom service (defaults to new one). * @param {Object} [options.bodyFormatOptions] - Options forwarded to `formatBodySegment`. * @returns {{ decodeMessage: (input: string) => { title: string, body: string, trailers: Record<string,string> }, encodeMessage: (payload: { title: string, body?: string, trailers?: Record<string,string> }) => string }} */
84-90: Remove duplicate JSDoc block.There are two consecutive JSDoc comments before the
TrailerCodecclass. The first block (lines 84-86) is redundant.♻️ Suggested fix
-/** - * TrailerCodec is the main public API. Provide a `TrailerCodecService` to reuse configuration - * and helper instances. - */ /** * TrailerCodec is the main public API for encode/decode through an injectable service. */
147-163: Fix duplicate and incomplete JSDoc blocks for deprecated functions.The deprecated functions have duplicate JSDoc blocks and
encodeMessageis missing its description.♻️ Suggested fix
/** - * @deprecated Use `TrailerCodec` instances for most call sites. - */ -/** * @deprecated Use `TrailerCodec.decodeMessage` directly. * Convenience wrapper that builds a default codec and decodes the message. */ export function decodeMessage(message, bodyFormatOptions) { return createDefaultTrailerCodec({ bodyFormatOptions }).decodeMessage(message); } /** - * @deprecated Use `TrailerCodec` instances for most call sites. + * @deprecated Use `TrailerCodec.encodeMessage` directly. + * Convenience wrapper that builds a default codec and encodes the payload. */ export function encodeMessage(payload, bodyFormatOptions) { return createDefaultTrailerCodec({ bodyFormatOptions }).encodeMessage(payload); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (45)
.github/workflows/ci.yml.gitignoreAPI_REFERENCE.mdARCHITECTURE.mdCHANGELOG.mdCODE_OF_CONDUCT.mdCONTRIBUTING.mdNOTICEREADME.mdSECURITY.mdTESTING.mddocs/ADVANCED.mddocs/INTEGRATION.mddocs/MIGRATION.mddocs/PARSER.mddocs/SERVICE.mdeslint.config.jsindex.jspackage.jsonscripts/pre-commit.shsrc/adapters/CodecBuilder.jssrc/adapters/FacadeAdapter.jssrc/domain/entities/GitCommitMessage.jssrc/domain/errors/CommitMessageInvalidError.jssrc/domain/errors/TrailerCodecError.jssrc/domain/errors/TrailerInvalidError.jssrc/domain/errors/TrailerNoSeparatorError.jssrc/domain/errors/TrailerTooLargeError.jssrc/domain/errors/TrailerValueInvalidError.jssrc/domain/schemas/GitCommitMessageSchema.jssrc/domain/schemas/GitTrailerSchema.jssrc/domain/services/TrailerCodecService.jssrc/domain/services/TrailerParser.jssrc/domain/services/helpers/BodyComposer.jssrc/domain/services/helpers/MessageNormalizer.jssrc/domain/services/helpers/TitleExtractor.jssrc/domain/value-objects/GitTrailer.jssrc/utils/zodValidator.jstest/unit/adapters/CodecBuilder.test.jstest/unit/adapters/FacadeAdapter.test.jstest/unit/domain/entities/GitCommitMessage.test.jstest/unit/domain/services/TrailerCodecService.test.jstest/unit/domain/services/TrailerParser.test.jstest/unit/domain/value-objects/GitTrailer.test.jstest/unit/index.test.js
🧰 Additional context used
🧬 Code graph analysis (19)
test/unit/adapters/FacadeAdapter.test.js (1)
src/domain/services/TrailerCodecService.js (1)
TrailerCodecService(15-159)
src/domain/errors/TrailerTooLargeError.js (1)
src/domain/errors/TrailerCodecError.js (1)
TrailerCodecError(4-15)
src/domain/errors/TrailerInvalidError.js (1)
src/domain/errors/TrailerCodecError.js (1)
TrailerCodecError(4-15)
src/domain/errors/CommitMessageInvalidError.js (1)
src/domain/errors/TrailerCodecError.js (1)
TrailerCodecError(4-15)
src/domain/services/TrailerParser.js (2)
src/domain/schemas/GitTrailerSchema.js (2)
TRAILER_KEY_RAW_PATTERN_STRING(72-72)TRAILER_KEY_RAW_PATTERN_STRING(72-72)src/domain/errors/TrailerNoSeparatorError.js (1)
TrailerNoSeparatorError(4-12)
test/unit/adapters/CodecBuilder.test.js (2)
src/adapters/CodecBuilder.js (1)
createConfiguredCodec(31-53)src/adapters/FacadeAdapter.js (2)
decodeMessage(154-156)encodeMessage(161-163)
test/unit/domain/entities/GitCommitMessage.test.js (3)
src/domain/entities/GitCommitMessage.js (1)
GitCommitMessage(21-114)src/domain/value-objects/GitTrailer.js (1)
GitTrailer(11-84)src/domain/errors/CommitMessageInvalidError.js (1)
CommitMessageInvalidError(4-12)
src/domain/services/helpers/MessageNormalizer.js (1)
src/domain/errors/TrailerTooLargeError.js (1)
TrailerTooLargeError(4-12)
src/domain/schemas/GitCommitMessageSchema.js (1)
src/domain/schemas/GitTrailerSchema.js (2)
GitTrailerSchema(71-71)GitTrailerSchema(71-71)
src/domain/entities/GitCommitMessage.js (4)
src/domain/errors/CommitMessageInvalidError.js (1)
CommitMessageInvalidError(4-12)src/domain/schemas/GitTrailerSchema.js (2)
GitTrailerSchema(71-71)GitTrailerSchema(71-71)src/domain/schemas/GitCommitMessageSchema.js (2)
GitCommitMessageSchema(12-16)GitCommitMessageSchema(12-16)src/domain/value-objects/GitTrailer.js (1)
GitTrailer(11-84)
src/domain/errors/TrailerValueInvalidError.js (1)
src/domain/errors/TrailerCodecError.js (1)
TrailerCodecError(4-15)
src/domain/value-objects/GitTrailer.js (4)
src/utils/zodValidator.js (1)
schema(28-28)src/domain/schemas/GitTrailerSchema.js (2)
GitTrailerSchema(71-71)GitTrailerSchema(71-71)src/domain/errors/TrailerValueInvalidError.js (1)
TrailerValueInvalidError(4-12)src/domain/errors/TrailerInvalidError.js (1)
TrailerInvalidError(4-12)
test/unit/domain/services/TrailerParser.test.js (3)
src/adapters/CodecBuilder.js (1)
parser(40-40)src/domain/services/TrailerParser.js (1)
TrailerParser(8-88)src/domain/errors/TrailerNoSeparatorError.js (1)
TrailerNoSeparatorError(4-12)
test/unit/domain/value-objects/GitTrailer.test.js (3)
src/domain/value-objects/GitTrailer.js (1)
GitTrailer(11-84)src/domain/errors/TrailerInvalidError.js (1)
TrailerInvalidError(4-12)src/domain/errors/TrailerValueInvalidError.js (1)
TrailerValueInvalidError(4-12)
src/domain/schemas/GitTrailerSchema.js (1)
src/domain/errors/TrailerInvalidError.js (1)
TrailerInvalidError(4-12)
test/unit/index.test.js (2)
src/adapters/CodecBuilder.js (2)
service(41-45)helpers(46-46)src/adapters/FacadeAdapter.js (2)
createMessageHelpers(65-82)formatBodySegment(47-53)
src/adapters/FacadeAdapter.js (2)
src/domain/errors/TrailerInvalidError.js (1)
TrailerInvalidError(4-12)src/domain/services/TrailerCodecService.js (1)
TrailerCodecService(15-159)
src/domain/services/TrailerCodecService.js (8)
src/domain/value-objects/GitTrailer.js (1)
GitTrailer(11-84)src/domain/schemas/GitTrailerSchema.js (2)
getDefaultTrailerSchemaBundle(74-74)getDefaultTrailerSchemaBundle(74-74)src/domain/services/helpers/MessageNormalizer.js (1)
MessageNormalizer(13-48)src/domain/services/helpers/TitleExtractor.js (2)
extractTitle(8-15)nextIndex(10-10)src/domain/services/helpers/BodyComposer.js (1)
composeBody(9-26)src/adapters/CodecBuilder.js (1)
schemaBundle(39-39)src/domain/services/TrailerParser.js (1)
TrailerParser(8-88)src/domain/entities/GitCommitMessage.js (1)
GitCommitMessage(21-114)
test/unit/domain/services/TrailerCodecService.test.js (6)
src/adapters/CodecBuilder.js (1)
service(41-45)src/domain/entities/GitCommitMessage.js (1)
GitCommitMessage(21-114)src/domain/errors/TrailerNoSeparatorError.js (1)
TrailerNoSeparatorError(4-12)src/domain/errors/TrailerValueInvalidError.js (1)
TrailerValueInvalidError(4-12)src/domain/errors/TrailerTooLargeError.js (1)
TrailerTooLargeError(4-12)src/domain/services/helpers/TitleExtractor.js (2)
title(9-9)nextIndex(10-10)
🪛 LanguageTool
TESTING.md
[style] ~16-~16: Consider using a different verb to strengthen your wording.
Context: ...ttier** with the configured settings to keep formatting consistent. Commit the forma...
(KEEP_PRESERVE)
README.md
[style] ~148-~148: Consider a different adjective to strengthen your wording.
Context: ...poseBody); see docs/SERVICE.md` for a deeper explanation of how to customize each st...
(DEEP_PROFOUND)
CODE_OF_CONDUCT.md
[style] ~20-~20: Try using a synonym here to strengthen your wording.
Context: ...vances * Trolling, insulting/derogatory comments, and personal or political attacks * Pu...
(COMMENT_REMARK)
[style] ~37-~37: ‘with regard to’ might be wordy. Consider a shorter alternative.
Context: ...s obligated to maintain confidentiality with regard to the reporter of an incident. Further de...
(EN_WORDINESS_PREMIUM_WITH_REGARD_TO)
🪛 markdownlint-cli2 (0.18.1)
CONTRIBUTING.md
25-25: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
ARCHITECTURE.md
21-21: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/SERVICE.md
12-12: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
13-13: Table pipe style
Expected: leading_and_trailing; Actual: no_leading_or_trailing; Missing leading pipe
(MD055, table-pipe-style)
13-13: Table pipe style
Expected: leading_and_trailing; Actual: no_leading_or_trailing; Missing trailing pipe
(MD055, table-pipe-style)
13-13: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
14-14: Spaces inside code span elements
(MD038, no-space-in-code)
14-14: Table pipe style
Expected: leading_and_trailing; Actual: trailing_only; Missing leading pipe
(MD055, table-pipe-style)
14-14: Table column count
Expected: 3; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
SECURITY.md
96-96: Bare URL used
(MD034, no-bare-urls)
CODE_OF_CONDUCT.md
37-37: Bare URL used
(MD034, no-bare-urls)
43-43: Bare URL used
(MD034, no-bare-urls)
47-47: Bare URL used
(MD034, no-bare-urls)
🪛 Shellcheck (0.11.0)
scripts/pre-commit.sh
[warning] 2-2: In POSIX sh, set option pipefail is undefined.
(SC3040)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: James Ross <james@flyingrobots.dev>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: James Ross <james@flyingrobots.dev>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: James Ross <james@flyingrobots.dev>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: James Ross <james@flyingrobots.dev>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: James Ross <james@flyingrobots.dev>
Getting ready to release v2.1.0
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.