refactor(lib): harden shared-file-writer derivation against drift - #2204
Merged
dyoshikawa merged 2 commits intoJul 10, 2026
Conversation
Address the two `mid` follow-ups from PR #2109 review (#2110): mid#1: SHARED_WRITE_FEATURE_ORDER is hand-maintained, so a feature that later starts writing a shared config file could be silently forgotten. Add an explicit NON_SHARED_WRITE_FEATURES exclusion list (commands, skills — each writes its own dedicated per-item artifact) plus a test asserting the two lists partition ALL_FEATURES exactly. Adding a new Feature now fails the test until it is consciously classified as a shared writer or an excluded one. mid#2: settablePathsForScope early-returned from the getSettablePaths catch, which skipped the subsequent getExtraSharedWritePaths collection. A tool whose global getSettablePaths throws while it also has a global-only extra shared path would silently drop that path. Collect extra paths independently of getSettablePaths success (still fail-open per hook), and cover it with a test. Closes #2110 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address a review low: settablePathsForScope became public in this PR, so switch its positional (cls, global) args to a single object argument per the project coding guideline for multi-arg functions.
Owner
Author
|
@dyoshikawa Thank you! |
Merged
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
Addresses the two
midfollow-ups from the PR #2109 review captured in #2110, hardeningsrc/lib/shared-file-derive.tsagainst future drift. Refs #2081.Changes
mid #1 — exhaustiveness for the shared-write feature classification
SHARED_WRITE_FEATURE_ORDERis hand-maintained. Ifcommands/subagents/skillslater start writing shared config files and someone forgets to account for them, the drift this derivation is meant to prevent could silently re-appear.NON_SHARED_WRITE_FEATURESexclusion list (commands,skills), each documented with why it is excluded (both write their own dedicated per-item artifacts — command files / skill directories — never a file another feature also writes).SHARED_WRITE_FEATURE_ORDER ∪ NON_SHARED_WRITE_FEATURESpartitionsALL_FEATURESexactly. Adding a newFeaturenow fails the test until it is consciously classified as a shared writer or an excluded one. (A duplicate makes the classified list longer thanALL_FEATURES, so the single partition assertion also rejects overlap — no redundant case.)mid #2 —
getExtraSharedWritePathsno longer swallowed by a throwinggetSettablePathssettablePathsForScopeearly-returned from thegetSettablePathscatch, which skipped the subsequentgetExtraSharedWritePathscollection. Currently harmless with existing tools, but a tool whose globalgetSettablePathsthrows while it also has a global-only extra shared path would silently drop that path.getSettablePathsleavessettableundefined and falls through to the extra-path collection instead of returning early. Both hooks remain fail-open individually (the derivation runs at module load, so a throwing tool must not take down everygenerate).settablePathsForScopeand added a test that a throwinggetSettablePathsstill yields the tool'sgetExtraSharedWritePathsentries.Notes
lowitems (Add support for sst/opencode #3 base-class default forgetExtraSharedWritePaths, Add support for Openhands CLI #4 type-guard for thesettableshape, Add support for opencode-ai/opencode #5 test-case-subsumption comment) and item Add support for gemini-cli #6 (confirm the intended Kilokilo.json/kilo.jsoncsplit) are out of scope for this PR and left for the maintainer.Full
pnpm cicheckis green (typecheck, lint, 6926 tests, content checks).Closes #2110