refactor(generate): unify shared config file writes behind a declared-ownership gateway - #2187
Merged
Merged
Conversation
…tracts The generation step graph's writesSharedFile (~40 hand-listed paths) and dependsOn edges are now derived from the processor registry's settable paths plus a single SHARED_WRITE_FEATURE_ORDER precedence declaration in shared-file-derive.ts, so adding a tool or shared path no longer requires editing generate.ts. A registry-derived contract test runs the real generation pipeline per feature in canonical order (project and global scope) and fails when any step deletes a key path another feature wrote to a shared config file (JSON/JSONC/YAML/TOML).
…tract Pre-seed every derived shared file with a sentinel key before running the per-feature generation passes, so the contract also fails when a read-modify-write drops keys the user put in the file, not only keys another feature wrote.
Replace the four per-file helper modules (claudecode-settings-gateway, hermes-config, shared/takt-config, opencode-config's parser) with a single shared-config gateway (src/features/shared/shared-config-gateway.ts) that owns the format codecs (YAML/JSON/JSONC, one empty-file rule, one prototype-pollution pass), the named conflict policies (replace-owned-keys, deep-merge with snapshot keys), and a per-file SHARED_CONFIG_OWNERSHIP declaration executed by applySharedConfigPatch. Writers now state their patch and declared feature; the gateway performs the merge and rejects writes outside the declared ownership, and a test keeps each declaration in lock-step with the registry-derived writer set. takt's hand-rolled provider_profiles/provider_options merging collapses into the declared deep-merge policy, making the provider_options sibling-clobber class (329c9bf) structurally impossible; hermes' hooks/mcp/permissions/subagents writers and claudecode's hooks writer share the same declared policies instead of re-implementing merges. Also surfaces and fixes an undeclared cross-feature writer found during the migration: the subagents feature read-modify-writes ~/.hermes/config.yaml (plugins.enabled) but declared no shared write, so no ordering edge protected it. It now declares the path via getExtraSharedWritePaths, has a position in SHARED_WRITE_FEATURE_ORDER, and is exercised by the cross-feature write contract test.
…ndary Every registry-derived shared file must now either carry a SHARED_CONFIG_OWNERSHIP declaration or sit on the explicit GATEWAY_PENDING_SHARED_FILES list, so a new shared file can no longer appear without a conscious ownership decision, and the pending list can only shrink (a stale entry fails the companion test).
Contributor
|
Thank you for your contribution! Unfortunately, this PR has 1585 added lines, which exceeds the limit of 1000 lines for external contributors. Please split your changes into smaller PRs. See CONTRIBUTING.md for details. |
Owner
|
@saitota Thank you! |
This was referenced Jul 8, 2026
Merged
hirossyi73
pushed a commit
to hirossyi73/rulesync
that referenced
this pull request
Jul 10, 2026
… derivation Follow-up cleanups from PR dyoshikawa#2187 (shared-config gateway refactor): - [mid] deriveSharedFileWriters() no longer pre-filters registry entries by SHARED_WRITE_FEATURE_ORDER. That filter made the ordering guard in deriveSharedWriteSteps() dead code and the accompanying test vacuous: a future feature that started returning a shared path would be silently dropped from the write order instead of forcing an explicit precedence decision. All features are now considered; single-writer files are still excluded (features < 2), so the derived writer set is unchanged (snapshot test confirms), while the guard is now genuinely reachable. The test now asserts the property directly rather than relying only on the throw. - [low] Wrap the getExtraSharedWritePaths() call in try/catch, mirroring the sibling getSettablePaths() call, so a future tool whose implementation throws cannot break module load of generate.ts. - [low] Add a test pinning every custom policyFunction name in SHARED_CONFIG_OWNERSHIP to a real exported function, so a rename cannot silently stale the declaration. Closes dyoshikawa#2189 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 13, 2026
Merged
dyoshikawa
pushed a commit
that referenced
this pull request
Jul 14, 2026
Migrate the codexcli, grokcli, vibe, and reasonix shared-file writers (hooks/mcp/permissions) onto SHARED_CONFIG_OWNERSHIP + applySharedConfigPatch, adding a toml codec to the gateway alongside the existing yaml/json/jsonc ones. Blocks whose value depends on existing entries (codexcli-mcp per-server tool-approval state, codexcli-permissions override keys, reasonix sandbox/agent shallow merges) are recomputed in the tool class and applied as a whole owned key, matching the hermes plugins idiom. CODEXCLI_OVERRIDE_KEYS moves to constants/codexcli-paths.ts so the gateway declaration and the tool class can both import it without a cycle. GATEWAY_PENDING_SHARED_FILES is now empty, so the pending-list boundary tests are removed along with it. Refs #2187, #2223
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.
Background
#2081/#2109/#2115 settled the write order (declared, registry-verified) and gave
.claude/settings.jsonan ownership gateway. This PR covers the remaining pieces:GENERATION_STEP_GRAPHre-declares ~40 registry-known paths by hand, each shared file has its own merge helper re-spelling preserve/overwrite semantics (where the takt 329c9bf and amp 8e69d7e bugs came from), and no test guards cross-feature key preservation.Changes
writesSharedFile/dependsOnfrom the registry plus a singleSHARED_WRITE_FEATURE_ORDERlist; the hand-maintained graph is gone.shared-config-gateway.ts: format codecs, declared conflict policies (SHARED_CONFIG_OWNERSHIP), andapplySharedConfigPatch, which rejects writes outside declared ownership. takt's hand-rolledprovider_optionsmerge becomes the declared deep-merge.subagentswrites~/.hermes/config.yamlwith no declared shared write — now declared and ordered. Not-yet-migrated shared files sit on an enforced, shrink-only pending list.No change to generated output (one parse-error message reworded).
Test Plan
pnpm run check+ full vitest green (6,880 tests).Refs #2081, #2109, #2115