refactor(generate): enforce shared-file write order via a declarative step graph - #2081
Merged
Conversation
…teps and topological sort Replace the implicit ordering convention (prose comments stating MCP must run before Rules, and Permissions after Ignore) with declarative GenerationStep entries carrying writesSharedFile/dependsOn, then derive the run order with resolveExecutionOrder. A shared file written by two steps with no declared order, an unknown dependency, or a cycle now throws at runtime instead of silently corrupting kilo.jsonc / opencode.json / .claude/settings.json.
Address review: the skills step previously ran outside resolveExecutionOrder, so a future shared-file write by skills would bypass the data-loss assertion. Run generateSkillsCore inside the step's run() and have rules dependsOn skills, so skills now obeys the same ordering invariant as every other step.
The prior change only covered the two example pairs from the issue (mcp/rules, ignore/permissions). In reality hooks also writes .claude/settings.json, and permissions also writes kilo.jsonc/opencode.json, so the assertSharedFilesOrdered guard had gaps: reordering permissions or hooks would not be caught. Declare every writer of each shared file (claude-settings, kilo-opencode-config) with writesSharedFile/dependsOn so the guard covers all three writers per file. Rename the mcp-instructions-config token to kilo-opencode-config since permissions writes there too.
Zed's .zed/settings.json (and global ~/.config/zed/settings.json) is read-modify-written by three features: ignore (private_files), mcp (context_servers), and permissions. Declare the shared zed-settings token on all three so assertSharedFilesOrdered enforces their order too, closing the same reorder-safety gap that was just fixed for claude-settings and kilo-opencode-config.
…er tools Eight more tools read-modify-write a single config file from multiple features: qwencode/augmentcode/hermesagent (mcp+hooks+permissions), kiro (hooks+permissions), and amp/codexcli/grokcli/vibe (mcp+permissions). They were safe only by the incidental mcp->hooks->permissions run order, outside the assertSharedFilesOrdered guard. Declare a per-tool shared-file token on each writer and add hooks dependsOn mcp so every writer pair of every shared file is now totally ordered and machine-checked.
codexcli hooks writes [features] hooks=true and vibe hooks writes enable_experimental_hooks=true into the same config.toml that mcp and permissions already share, making them three-writer files. The hooks step was missing codexcli-config and vibe-config, so the hooks<->mcp/permissions order on those files was not machine-checked. Add both tokens to the hooks step.
Owner
|
@saitota Thank you! |
This was referenced Jul 1, 2026
Merged
saitota
added a commit
to saitota/rulesync
that referenced
this pull request
Jul 2, 2026
…rce declarations PR dyoshikawa#2081 declared each step's shared-file writers by hand as opaque tokens, checked only for internal graph consistency; a missing writer passed CI silently and was caught only in review (e.g. codexcli/vibe config.toml in ab61e38). Derive the multi-writer set from PROCESSOR_REGISTRY x getSettablePaths (plus a getExtraSharedWritePaths contract for writes not visible as settable paths), and assert it matches the step graph, so a missing declaration fails CI instead of review. Refs dyoshikawa#2081
saitota
added a commit
to saitota/rulesync
that referenced
this pull request
Jul 2, 2026
…rce declarations PR dyoshikawa#2081 declared each step's shared-file writers by hand as opaque tokens, checked only for internal graph consistency; a missing writer passed CI silently and was caught only in review (e.g. codexcli/vibe config.toml in ab61e38). Derive the multi-writer set from PROCESSOR_REGISTRY x getSettablePaths (plus a getExtraSharedWritePaths contract for writes not visible as settable paths), and assert it matches the step graph, so a missing declaration fails CI instead of review. Refs dyoshikawa#2081
saitota
added a commit
to saitota/rulesync
that referenced
this pull request
Jul 2, 2026
…rce declarations GENERATION_STEP_GRAPH declared each step's shared-file writers by hand; a missing writer passed CI silently and was caught only in review (e.g. codexcli/vibe config.toml in ab61e38). Derive the multi-writer set from PROCESSOR_REGISTRY x getSettablePaths across both project and global scope, plus a getExtraSharedWritePaths contract for writes not visible as settable paths, and assert it matches the step graph so a missing declaration fails CI instead of review. Deriving surfaced three drifted declarations, now fixed: .takt/config.yaml was unregistered; permissions was wrongly listed as a kilo.json writer (it writes kilo.jsonc); and the global-only .config/devin/config.json shared write was covered only by chance. Refs dyoshikawa#2081
This was referenced Jul 3, 2026
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.
Description
The write order of features sharing one config file lived only in two prose comments in
generate(). Reordering theawait generate*Core(...)lines, or switching toPromise.all, would silently drop another feature's keys. PR #1858 pinned the two known pairs with tests; this makes the constraint explicit in data.Changes
writesSharedFile/dependsOn;resolveExecutionOrdertopologically sorts them and throws on an unordered shared-file writer pair, an unknown dependency, or a cycle — so the array can be reordered freely and a silent data-loss trap becomes a startup error.Test Plan
resolveExecutionOrder; existing test(generate): add regression tests for execution-order constraints on shared output files #1858 regression tests preserved.pnpm cicheckgreen (6553 tests).Refs #1858