Conversation
Wire the existing TrailerService into the awilix container as a singleton so downstream issues can resolve it from the cradle. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds two new methods: - addTrailers: amends HEAD to append AI-* trailers, deduplicating against existing trailers on the commit - buildCommitMessage: pure string builder for composing commit messages with a trailer block Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughIntroduces a TrailerService and adds two ITrailerService methods: Changes
Sequence DiagramsequenceDiagram
participant Caller as Client
participant TS as TrailerService
participant Git as Git Repository
Caller->>TS: addTrailers(newTrailers, cwd?)
activate TS
TS->>Git: query/read existing trailers (queryTrailers/readTrailers)
Git-->>TS: existingTrailers
TS->>TS: filterDuplicates(newTrailers, existingTrailers)
TS->>Git: read HEAD commit message
Git-->>TS: currentMessage
TS->>TS: buildCommitMessage(currentMessage, filteredTrailers)
TS->>TS: hasTrailerBlock(currentMessage)
TS-->>TS: amendedMessage
TS->>Git: amend HEAD with amendedMessage
Git-->>TS: success
deactivate TS
TS-->>Caller: void
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
There was a problem hiding this comment.
Pull request overview
Adds write support to the existing TrailerService so the tool can append AI-* trailers to the current commit, and introduces a pure helper for composing commit messages with trailer blocks. This builds on the DI registration work from GIT-65 and expands unit coverage around trailer handling.
Changes:
- Add
TrailerService.addTrailers()to amendHEADand append (deduped) trailers. - Add
TrailerService.buildCommitMessage()+ trailer-block detection to avoid double blank lines. - Expand DI + unit tests for the new service capabilities.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/infrastructure/services/TrailerService.ts |
Implements addTrailers and buildCommitMessage, plus trailer-block detection logic. |
src/domain/interfaces/ITrailerService.ts |
Extends the domain interface with the new write and message-building methods. |
src/infrastructure/di/types.ts |
Adds trailerService to the ICradle type. |
src/infrastructure/di/container.ts |
Registers TrailerService in the Awilix container as a singleton. |
tests/unit/infrastructure/services/TrailerService.test.ts |
Adds unit tests for buildCommitMessage and git-backed tests for addTrailers. |
tests/unit/infrastructure/di/container.test.ts |
Extends container tests to cover trailerService resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Read existing trailers to avoid duplicates | ||
| const existing = this.readTrailers('HEAD', cwd); | ||
| const existingKeys = new Set(existing.map(t => `${t.key}:${t.value}`)); | ||
| const newTrailers = trailers.filter(t => !existingKeys.has(`${t.key}:${t.value}`)); |
There was a problem hiding this comment.
addTrailers is documented as appending only AI-* trailers, but it currently accepts and appends any ITrailer keys. Since readTrailers() only returns AI-* trailers, passing non-AI trailers would bypass deduplication and can lead to duplicate non-AI trailers being appended on repeated calls. Consider filtering/validating inputs to AI_TRAILER_PREFIX (or isAiTrailer) and either ignoring or rejecting non-AI keys so behavior matches the docs and dedupe logic.
| // Read existing trailers to avoid duplicates | |
| const existing = this.readTrailers('HEAD', cwd); | |
| const existingKeys = new Set(existing.map(t => `${t.key}:${t.value}`)); | |
| const newTrailers = trailers.filter(t => !existingKeys.has(`${t.key}:${t.value}`)); | |
| // Only operate on AI-* trailers to match readTrailers/parseTrailerBlock behavior. | |
| const aiTrailers = trailers.filter(t => t.key.startsWith(AI_TRAILER_PREFIX)); | |
| if (aiTrailers.length === 0) return; | |
| // Read existing trailers to avoid duplicates | |
| const existing = this.readTrailers('HEAD', cwd); | |
| const existingKeys = new Set(existing.map(t => `${t.key}:${t.value}`)); | |
| const newTrailers = aiTrailers.filter( | |
| t => !existingKeys.has(`${t.key}:${t.value}`) | |
| ); |
There was a problem hiding this comment.
Fixed — added AI_TRAILER_PREFIX filter at the top of addTrailers so non-AI trailers are rejected before deduplication. See 8fcd93f.
|
|
||
| assert.equal(typeof trailerService.readTrailers, 'function'); | ||
| assert.equal(typeof trailerService.formatTrailers, 'function'); | ||
| assert.equal(typeof trailerService.queryTrailers, 'function'); |
There was a problem hiding this comment.
This test block claims to check the resolved TrailerService interface, but it only asserts the pre-existing methods. Since ITrailerService now includes addTrailers and buildCommitMessage, either add assertions for those methods too or rename the test to avoid going stale/misleading.
| assert.equal(typeof trailerService.queryTrailers, 'function'); | |
| assert.equal(typeof trailerService.queryTrailers, 'function'); | |
| assert.equal(typeof trailerService.addTrailers, 'function'); | |
| assert.equal(typeof trailerService.buildCommitMessage, 'function'); |
There was a problem hiding this comment.
Fixed — added assertions for addTrailers and buildCommitMessage to the container interface test. See 8fcd93f.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/unit/infrastructure/di/container.test.ts`:
- Around line 166-179: Update the DI unit test for trailerService to assert the
new methods on the resolved service: verify that container.cradle.trailerService
exposes addTrailers and buildCommitMessage as functions in the same style as
existing assertions for readTrailers, formatTrailers, and queryTrailers; keep
the existing singleton test unchanged so it still confirms
container.cradle.trailerService === container.cradle.trailerService.
- Filter addTrailers input to AI-* prefixed trailers only - Add addTrailers and buildCommitMessage to container interface test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
addTrailers(trailers, cwd)method — amends HEAD commit to append AI-* trailers, deduplicating against existing trailersbuildCommitMessage(message, trailers)method — pure string builder for composing commit messages with a trailer blockKey: Value) to avoid double blank linesDepends on #34 (GIT-65)
Closes GIT-66
Test plan
npm run pre-commitpasses (type-check + lint)addTrailersamends HEAD correctly, preserves existing trailers, deduplicatesbuildCommitMessagehandles: no trailers, existing trailers, body text, trailing whitespace🤖 Generated with Claude Code
Summary by CodeRabbit