Data-driven artifact engine for agent integrations (fixes #29, groundwork for #50) - #63
Open
pradeepmouli wants to merge 42 commits into
Open
Data-driven artifact engine for agent integrations (fixes #29, groundwork for #50)#63pradeepmouli wants to merge 42 commits into
pradeepmouli wants to merge 42 commits into
Conversation
#50) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjmvwHuwV5r7ZeZpJLp5oR
Both halves of fork issue #65 (agent-target config + hook scripts) now share one bundled+user-override template mechanism, landing as a single upstream PR per user direction. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjmvwHuwV5r7ZeZpJLp5oR
…ct design Replaces the earlier three-parallel-mechanisms draft with a single InstallArtifact model: bundled + user-override tree under integrations/<name>/, where a plain file's tree position IS its destination path, and a <file>.toml sidecar is a patch descriptor (strategy + metadata) for merging into an existing file rather than overwriting it. Five merge strategies (json_key_path, json_array_append, toml_section, marker_delimited, overwrite) cover every case found across config_targets.rs, hooks.rs, and install.rs's CLAUDE.md/rules/reindex-command writers. Adds a narrow subprocess "resolver" escape hatch (pipeline-plugin's JSON-IPC shape, any language) for the few cases that can't be static data (VS Code's profile-dependent path, Zed's settings.json embed), explicitly excluded from future capture/promote tooling to preserve invertibility of the declarative path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjmvwHuwV5r7ZeZpJLp5oR
…ration - One manifest per integration (config.toml, alongside content files), not a sidecar-per-artifact tree -- matches infigraph-pipeline-plugin's own plugin.toml precedent more closely. - Content grouped by kind (hooks/, rules/) rather than mirroring destination paths or sitting flat -- consistent across all integrations regardless of current file count per kind. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjmvwHuwV5r7ZeZpJLp5oR
hooks/ and rules/ weren't an invented organizational scheme, they happened to already match .claude/hooks/ and .cursor|.windsurf/rules/ -- makes that correspondence explicit as the actual rule, and fixes the one place it wasn't followed (reindex-command.md now lives under commands/, mirroring the real ~/.claude/commands/ subfolder). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjmvwHuwV5r7ZeZpJLp5oR
The instructional text every agent gets told about infigraph (today's write_claude_md_instructions block, echoed in spirit by write_editor_rules for Cursor/Windsurf) shouldn't be copied into each integration's directory -- content_file can now reach into ../shared/, so editing the instructions once updates every integration that references it, and user-level override of shared/agents.md affects all of them in one edit too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjmvwHuwV5r7ZeZpJLp5oR
…xceptions Replaces per-hook/per-agent [[artifact]] declarations (key_path, idempotency_key, self_heal_field) with a simpler rule: a bundled .json/.toml file at a path mirroring a real destination deep-merges automatically (array entries owned/replaced via an "infigraph" naming convention instead of declared idempotency keys); anything else mirrored overwrites. config.toml is now only needed for the two genuine exceptions -- CLAUDE.md's marker-delimited insertion and VS Code/Zed's resolver-based paths -- so most integrations need no manifest at all. claude-code/settings.json becomes a single literal fragment of the full desired hooks structure instead of ten separate declared entries. Matcher self-healing becomes automatic (not opt-in) as a side effect of always fully replacing owned array entries on every apply. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjmvwHuwV5r7ZeZpJLp5oR
…located Replaces the "marker_delimited OR resolver" framing with the actual underlying rule: a manifest entry is needed whenever content isn't physically at its own mirrored local path, regardless of what write strategy applies once it's found. Resolves a previously-hedged open question: Cursor and Windsurf's rules content is shared with CLAUDE.md's instructional text, so they need a minimal config.toml too (plain overwrite, not marker_delimited) -- for the same reason CLAUDE.md does, not a different one. Named the forward-looking implication: a future shared hook or command would need the same treatment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjmvwHuwV5r7ZeZpJLp5oR
Converts /infigraph-reindex from a Claude-Code-only commands/ file into shared/skills/infigraph-reindex/SKILL.md (Agent Skills format), registered via config.toml artifact entries in Claude Code and Codex -- the two integrations whose skill discovery paths are actually verified. Other skills-adopting integrations are flagged as needing the same per-tool path verification before their entries are written, rather than guessed. Also tightens the "Category-level shared sourcing" out-of-scope bullet: hooks still lack a second cross-tool execution target, but the earlier "hooks are confirmed Claude-Code-specific" phrasing overclaimed given real prior art for an optional hooks:/triggers: SKILL.md frontmatter field.
Corrects an inconsistency: the spec previously grouped .json and .toml under one automatic "deep-merge, no manifest" convention, but the Research table already called for a distinct toml_section (raw-text section splice) strategy for Codex, since a full parse-merge-reserialize risks stripping comments/formatting from a real hand-maintained TOML file. A surgical splice needs an explicit section key path that can't be inferred from extension+path alone, so toml_section is always manifest-declared -- adds this as a third reason a manifest is required, gives Codex's config.toml its own MCP artifact (independent of the reindex-skill work), and updates the directory layout, uninstall symmetry, and test sections to match.
Twelve-task plan for a new, fully self-contained crates/infigraph-cli/src/artifacts/ module: manifest parsing, template substitution, the five artifact strategies (json_deep_merge, overwrite, marker_delimited, toml_section, json_key_path) plus their uninstall inverses, convention-based classification, build.rs bundled-resource embedding, two-tier discovery, and InstallStep grouping. Purely additive -- touches no existing code, unused until the follow-up migration plan wires cmd_install/cmd_uninstall onto it. Task 12 fixes a real gap found while writing this plan: the first pass required every artifact to have a static manifest path, which the design spec's own Zed example (resolver-only, no path) already contradicts, and never actually invoked the resolver subprocess at apply time.
User asked to make it one big plan instead of two -- renamed the plan file
and updated its header to cover the full feature (engine + integration
content + install/uninstall rewiring), with Tasks 13+ now appended into
the same document rather than a separate follow-up plan.
While reconciling the merged plan's discovery logic against real bundled
file paths for Task 13+, found and fixed:
- Convention-based discovery (Pass 2) never stripped the leading
<integration>/ directory segment from a bundled file's own path before
using it as the destination path relative to $HOME -- every bundled file
would have landed at e.g. ~/claude-code/.claude.json instead of
~/.claude.json. Added strip_integration_prefix and fixed every affected
test's expected path.
- The design spec's own Claude Code directory-layout diagram was
inconsistent with every other integration's entry: settings.json and
hooks/ sat directly under claude-code/ with no .claude/ nesting, unlike
Cursor/Windsurf/Gemini CLI/etc., which all show their full real nested
destination path. Fixed to nest under claude-code/.claude/, and kept the
infigraph- filename prefix on hook scripts so the array-ownership
substring marker still matches them.
- Task 12's resolver support wrongly required every resolver artifact to
have no content_file, which breaks VS Code (resolver determines only the
*path*; content is a static local fragment). Relaxed that, and made
template-format inference (JSON vs TOML) derive from strategy instead of
the (possibly absent) path, so a resolver-only artifact's local
content_file still gets {{mcp_path}} substituted correctly.
…install Completes the merged single-plan implementation doc. Tasks 13-17 populate crates/infigraph-cli/resources/integrations/ with real, verified content for all 13 integrations (five simple JSON agents; Claude Code's 10 hooks + settings.json + CLAUDE.md + shared reindex skill; Cursor/Windsurf pulling shared/agents.md as rules; Codex's toml_section MCP registration; VS Code/Zed resolver scripts), each with fixture tests exercising the real compiled BUNDLED_INTEGRATIONS registry rather than synthetic fixtures. Tasks 18-19 rewire cmd_install/cmd_uninstall onto the artifact engine. Task 20 deletes config_targets.rs, the hardcoded hook-install functions, and the hardcoded docs/rules/reindex writers it all replaces. Task 21 is final full-workspace verification plus a manual install/uninstall smoke test against a disposable $HOME. Every integration's destination path/shape was cross-checked against both the old (pre-#29-fix) hardcoded config_targets.rs values and the design spec's independently re-verified research table before being written in.
…d content infigraph-enforce.sh's grep/rg detection currently blocks any Bash command containing those tool names, including when piping another command's output for filtering (cmd 2>&1 | grep -iE "error") -- which this repo's own CLAUDE.md explicitly allows. Caught live this session via a mobile Claude Code screenshot; the fix existed only in a local, untracked copy of the hook, never in hooks.rs on any branch. A pure "verbatim copy" in Task 14 would have carried the bug into the bundled default forever. Task 14 Step 3 now writes infigraph-enforce.sh with the fix folded in (sed-stripped piped-grep exemption, deliberately avoiding \b since macOS's BSD sed silently no-ops on it rather than erroring -- verified against both GNU and BSD sed live), everything else in the script unchanged. Added a regression test proving the bundled content carries the fix.
/codex:adversarial-review --base upstream/main flagged four high-severity issues in the fully-written plan, all confirmed real and fixed before any code was written: 1. apply_marker_delimited/remove_marker_delimited (Task 5) treated a present-but-unmatched start marker as "block extends to EOF", silently destroying trailing user content in a damaged CLAUDE.md. Now refuses rather than guessing, with regression tests proving the file is left byte-for-byte untouched. 2. apply_toml_section/remove_toml_section (Task 6) used a raw substring search for the table header, letting a comment mentioning the header text or a differently-named header sharing our header as a prefix (e.g. [mcp_servers.infigraph_extra]) get corrupted or deleted. Now uses a line-based match via find_toml_section_bounds, with two collision regression tests. 3. cmd_uninstall (Task 19) recursively removed all of ~/.infigraph/, which -- now that this PR adds ~/.infigraph/integrations/ as a persistent user-override directory -- would silently destroy real user customization work on every uninstall. Extracted into clean_infigraph_cache_dir(home: &Path), scoped to just models/ and update_check.json, with a regression test proving integrations/ survives. 4. run_resolver_from_script (Task 12) always spawned resolver scripts via shebang-based direct execution, which only works on Unix -- both bundled .py resolvers (VS Code, Zed; Task 17) would fail to start on Windows despite existing specifically to handle Windows profile paths. Added resolver_invocation, launching .py scripts via an explicit python3/python interpreter call on every platform, with a shebang-less regression test. All four fixes include updated test counts at every downstream checkpoint step that cited exact numbers.
…itHub Copilot CLI
…e shared reindex skill
…tions it replaces Removes config_targets.rs (already superseded by the artifact engine's discovery/apply pipeline) and hooks.rs's 10 hardcoded hook-script constants plus the 8 install_*_hook functions that wrote them -- Claude Code's hooks are now bundled artifacts under resources/integrations/ claude-code/.claude/hooks/ and installed through run_install like every other integration. hooks.rs keeps only allowed_tools, install_claude_allowlist, and uninstall_claude_allowlist (the Claude Code permission allowlist genuinely sits outside the artifact mechanism), plus a simplified uninstall_hooks that now only strips infigraph-tagged entries from settings.json (file removal is handled by remove_resolved_artifact). Also removes install.rs's write_claude_md_instructions/write_editor_rules/write_reindex_command, dead since Task 18 rewired cmd_install onto the artifact engine. reinstall_hooks() (called from cmd_update after a self-update binary swap) now calls run_install directly instead of the deleted install_*_hook functions, so a self-update correctly re-applies the new binary's bundled hooks through the same artifact pipeline as a fresh install.
…structured-deny and indexable-Read fixes apply_overwrite never set the executable bit, so every hook script installed through the artifact engine would silently never fire (hook scripts run via shebang through the user's shell, which requires it -- unlike every other overwrite-strategy artifact). apply_resolved_artifact now chmods 0o755 any overwrite-strategy artifact whose target path has a `hooks` segment, with a regression test. Also ports two enforce.sh fixes developed independently on the fork's feat/hardening branch, carried back here since both are genuine hook-correctness improvements with no dependency on anything fork-only: - deny() helper emitting the real structured PreToolUse `permissionDecision: "deny"` contract, replacing the old `exit 2` + raw `additionalContext` mechanism. - Read is now only blocked for files infigraph can actually index -- an extensions allowlist, a `.gitignore` check via `git check-ignore`, and an excluded-dirs check (.infigraph, .claude, node_modules, __pycache__, .tox, .git) -- rather than blocking every Read unconditionally. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjmvwHuwV5r7ZeZpJLp5oR
…ooks key
Found via an end-to-end install/uninstall run against a scratch $HOME on
feat/hardening (not just the unit suite), then confirmed the same bug
exists here: after `infigraph uninstall`, .claude/settings.json ended up
as `{"hooks": null}` instead of a clean `{}`.
uninstall_hooks() read `settings["hooks"].get_mut(event)` for each of the
6 hook event names. serde_json's Index (not IndexMut-via-get_mut) silently
inserts a null entry for a missing key as a side effect of indexing --
and by the time this legacy cleanup pass runs, run_uninstall (the artifact
engine's own settings.json removal, which runs first in cmd_uninstall) has
typically already deleted the "hooks" key entirely once every event array
it owned went empty. So the very act of checking "is there anything left
to clean up here" reintroduced a corrupt "hooks": null into a file that
had just been correctly emptied. The function also wrote the file back
unconditionally regardless of whether anything actually changed.
Fixed by using settings.get_mut("hooks") (Option, no insert side effect)
and only writing the file back when a hook entry was actually removed.
Added two regression tests exercising uninstall_hooks directly: one for
the corruption case (empty settings.json, "hooks" key absent), one
confirming its real remaining job -- stripping infigraph hooks a user
added outside the artifact mechanism -- still works.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjmvwHuwV5r7ZeZpJLp5oR
… MCP config Comparing this branch against upstream PR #56 ("fix: correct OpenCode and Copilot MCP configs", open on intuit/infigraph, addressing part of issue #29) found this branch's bundled github-copilot-cli fragment was missing two fields PR #56's author confirmed necessary through actual manual testing against real Copilot CLI: "type": "local" (without it, Copilot CLI doesn't recognize this as a local MCP server) and "tools": ["*"] (grants tool access). The existing regression test only asserted command/args, so the gap went uncaught since Task 13 bundled this fragment generically without that field-level detail. OpenCode's bundled fragment already matches PR #56's schema and path exactly, so no change needed there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjmvwHuwV5r7ZeZpJLp5oR
Ported from upstream PR #57 (open on intuit/infigraph). Directly relevant to this branch: every agent config the artifact engine installs invokes infigraph-mcp with exactly `--mcp` alone (no `--ui`), so every user of `infigraph install` was hitting this leak. ui_enabled was derived from `args.iter().any(|a| a == "--ui" || a.starts_with("--ui=") || a == "--mcp")` -- the `|| a == "--mcp"` clause meant bare `--mcp` (no `--ui`) still enabled the UI keep-alive path, putting the process into a `loop { sleep(3600s) }` after stdin EOF instead of exiting. Over repeated MCP client restarts this accumulates orphaned worker processes. Extracted the check into `ui_enabled_from(args: &[String]) -> bool` (unit-testable in isolation) and removed the `--mcp` clause. Verified via the new unit tests and PR #57's manual repro on feat/hardening: `infigraph-mcp --mcp < /dev/null &` now exits on stdin EOF instead of hanging (same fix, same file region, verified separately there). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjmvwHuwV5r7ZeZpJLp5oR
…ipeline plugins Replace .py-extension-sniffing (resolver = ["./script.py"]) with an explicit, pipeline-plugin-style command array where the manifest author declares the interpreter: resolver = ["python3", "./script.py"]. The array's last element is the script; everything before it is a literal prefix prepended verbatim at invocation time. The only remaining magic is normalizing a "python"/"python3" prefix to whichever binary exists under that name for the current OS. ResolverSpec::extra_args is renamed to command_prefix to match. Bundled vscode/zed manifests and docs/AGENT_INTEGRATION_PLUGINS.md are updated to the new format. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QRZkpP9WC2nDpBDcbsbvmA
…ace manual_snippet
Two fixes from an adversarial Codex review of the artifact-engine branch:
- overwrite strategy now records a content hash at install time
(~/.infigraph/installed-files.json) and checks it before deleting on
uninstall. A file whose content no longer matches -- the user edited a
hook script, a Cursor/Windsurf rules file, or a shared skill doc after
install -- is left in place instead of silently removed.
remove_resolved_artifact now returns RemoveOutcome::{Removed,
NotPresent, PreservedModified} instead of a bare bool; cmd_uninstall
reports preserved paths. The other four strategies don't need this --
they already scope removal to a specific key/section/marker pair, so a
user's edits elsewhere in the same file are untouched by construction.
- ApplyOutcome::Skipped's manual_snippet was computed (e.g. by
apply_json_deep_merge on a JSONC parse failure) but discarded by
run_install's `Skipped { reason, .. }` match arm, so a user whose Zed
settings.json has comments/trailing-commas got a skip reason with no
way to actually complete the registration. InstallReport.skipped now
carries the snippet through to cmd_install's output.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QRZkpP9WC2nDpBDcbsbvmA
docs/superpowers/plans and docs/superpowers/specs are working documents from this fork's own planning tooling, not something meant to land in intuit/infigraph's docs tree. Dropped from this PR branch only; they stay in feat/agent-target-templates and feat/hardening on the fork.
pradeepmouli
requested review from
WinterQuant,
johnintuit,
murari316 and
sandeep-mewara
as code owners
August 19, 2026 00:57
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
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
Replaces the hardcoded per-agent install logic (
config_targets.rs'sAGENT_TARGETSarray, ~10install_*_hookfunctions, and the ad-hocwrite_claude_md_instructions/write_editor_rules/write_reindex_commandfunctions) with a data-driven artifact engine:crates/infigraph-cli/resources/integrations/<agent>/(bundled into the binary viabuild.rs), optionally overridden per-file at~/.infigraph/integrations/<agent>/— same layout, user wins..jsonfiles convention-inferjson_deep_merge, everything else non-.tomlconvention-infersoverwrite..tomldestinations always need an explicitconfig.tomlmanifest entry (toml_section+key_path), since a full parse/reserialize would risk stripping comments from a hand-maintained file like Codex's~/.codex/config.toml.json_deep_merge,overwrite,marker_delimited,toml_section,json_key_path.infigraph-pipeline-plugin'scommandfield:resolver = ["python3", "./resolve-x-path.py"], full command array, author-controlled interpreter.cmd_install/cmd_uninstallare now thin wrappers arounddiscover_artifacts+apply_resolved_artifact/remove_resolved_artifact; uninstall symmetry is derived from each artifact's own shape rather than hardcoded per-agent keys.docs/AGENT_INTEGRATION_PLUGINS.md, written in the same format asGRAMMAR_PLUGINS.md/docs/PIPELINE_PLUGINS.md.Fixes #29 (wrong MCP config shape for OpenCode/Zed/Copilot CLI/Windsurf/Kiro/VS Code — each now gets its own correct schema and path instead of one hardcoded
mcpServersshape). Lays groundwork for #50 (invasive default install): artifacts are now classified via anInstallStepenum (McpRegistration/DocsAndRules/Hooks/Models), grouping whatcmd_installwrites so a future--modeflag has something to filter on — no such flag ships in this PR.Also folds in two small, independently-verified fixes found along the way (not scoped to this PR but touching the same install path): the
--mcpflag alone triggering the web UI keep-alive loop, and a missingtype/toolsfield pair in the bundled GitHub Copilot CLI MCP config.Safety fixes from an adversarial review pass
Before opening this PR, I ran an adversarial review against the full diff and verified two findings against the actual source before fixing them:
overwritestrategy (hook scripts, Cursor/Windsurf rule files, shared skill docs) removed whatever was at its target path unconditionally — no different from the pre-existinghooks.rsbehavior for hook scripts, but now covering a much larger surface (rule/skill files a user is far more likely to hand-edit).overwritenow records a content hash at install time (~/.infigraph/installed-files.json) and checks it before deleting; a changed file is left in place and reported instead of removed. The other four strategies don't need this — they already scope removal to a specific key/section/marker, so user edits elsewhere in the same file are untouched by construction.Skippedoutcome'smanual_snippetwas computed but discarded.apply_json_deep_merge/apply_json_key_pathcompute the exact fragment to add by hand when a target file fails to parse (e.g. Zed'ssettings.jsonhas comments or trailing commas), butrun_install'sSkipped { reason, .. }match arm dropped it, so the user got a skip reason with no way to actually complete the registration.InstallReport.skippednow carries the snippet through tocmd_install's output.(A third finding from the same review — an OpenCode config schema claim — was checked against OpenCode's live docs and found to be incorrect; the shipped
mcp.<name>shape is current.)Test plan
cargo test --all,cargo fmt --all -- --check,cargo clippy --all-targets -- -D warningsall greeninfigraph-clicover the artifact engine end-to-end: bundled-fixture application per agent, uninstall symmetry, resolver dispatch (VS Code/Zed), and a new regression test asserting a hand-editedoverwritefile survives uninstall while an unmodified one is removedinfigraph install/infigraph uninstallagainst a scratch$HOME