fix: align dotgitignore facet paths with actual directory structure#535
Merged
nrslib merged 1 commit intonrslib:mainfrom Mar 20, 2026
Merged
fix: align dotgitignore facet paths with actual directory structure#535nrslib merged 1 commit intonrslib:mainfrom
nrslib merged 1 commit intonrslib:mainfrom
Conversation
The `.takt/.gitignore` template listed facet types (personas, policies,
etc.) without the `facets/` prefix, so files ejected to
`.takt/facets/{type}/` were not tracked by git.
Add the `facets/` prefix to each allowlist entry and update the
integration test to derive facet directories from `getProjectFacetDir`
and `VALID_FACET_TYPES` instead of hardcoded values.
Closes nrslib#513
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
kenfdev
commented
Mar 20, 2026
Contributor
Author
kenfdev
left a comment
kenfdev
left a comment
There was a problem hiding this comment.
少しテスト方法を変えてしまっているのですが、「こうしてほしい！」があればご指摘お願いします！ 🙏
Comment on lines
+14
to
+15
| import { getProjectPiecesDir, getProjectFacetDir } from '../infra/config/paths.js'; | ||
| import { VALID_FACET_TYPES, parseFacetType } from '../features/config/ejectBuiltin.js'; |
Contributor
Author
There was a problem hiding this comment.
ここまでやるのか、という議論はあるかと思います（元々のテストのやり方を変更しているため）
ただし、元々の問題もキャッチできるようにするためアプリ側のロジックから持ってくるようにしました
Comment on lines
-61
to
-62
| mkdirSync(join(testDir, '.takt', facet), { recursive: true }); | ||
| writeFileSync(join(testDir, '.takt', facet, 'test.md'), `# ${facet}`); |
Contributor
Author
There was a problem hiding this comment.
ここに単純に facets/ を追加するテストを追加するだけ（piecesは別階層ですが）でも良かったのですが、もう少し踏み込んだテストにしています。
Owner
|
ありがとうございます！ |
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
.takt/.gitignoretemplate listed facet types (personas, policies, etc.) without thefacets/prefix, so files ejected to.takt/facets/{type}/were not tracked by git.Add the
facets/prefix to each allowlist entry and update the integration test to derive facet directories fromgetProjectFacetDirandVALID_FACET_TYPESinstead of hardcoded values.Closes #513
Takt Review Summary
Overall Verdict: APPROVE
Summary
All five reviewers unanimously approved this change. The fix correctly aligns
.takt/.gitignorefacet path patterns with the actual directory structure (.takt/facets/{type}/instead of.takt/{type}/), and the tests now derive expected paths from canonical source-of-truth functions (getProjectFacetDir,VALID_FACET_TYPES), eliminating hardcoded values and preventing this class of bug from recurring.Review Results
parseFacetType()!non-null assertion safe in test contextCurrent Iteration Findings (new)
None
Carry-over Findings (persists)
None
Resolved Findings (resolved)
None
Improvement Suggestions
Rejection Gate
neworpersists. APPROVE is valid.