Skip to content

refactor: consolidate agent-config path + JSON read/write helpers - #231

Merged
getappz merged 3 commits into
masterfrom
devin/1784314856-dedup-config-json-utils
Jul 17, 2026
Merged

refactor: consolidate agent-config path + JSON read/write helpers#231
getappz merged 3 commits into
masterfrom
devin/1784314856-dedup-config-json-utils

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

The agent-config code (init.rs, components.rs, uninstall.rs) had repeated the same three inline idioms all over the place. This PR extracts them into shared helpers and routes the call sites through them — no behavior change other than reads becoming JSONC-tolerant (see below).

1. Config path constructionhome().join(".claude").join("settings.json"), ~/.claude.json, ~/.claude/rules, and the ~/.config/opencode/{opencode.jsonc,rules} triple were hand-built at ~30 sites. New helpers in paths.rs:

claude_dir() / claude_settings_path() / claude_rules_dir() / claude_json_path()
opencode_dir() / opencode_config_path() / opencode_rules_dir()

2. Read-config-as-object — every wire_*/merge_* did:

let mut v: Value = fs::read_to_string(&p).ok()
    .and_then(|s| serde_json::from_str(&s).ok())
    .unwrap_or_else(|| json!({}));
if !v.is_object() { v = json!({}); }

Collapsed to jsonc::read_json_object(&p, || json!({})), which reads, falls back to default() on missing/unreadable/invalid/non-object, and guarantees an object.

3. Write-pretty-JSON-configfs::write(&p, serde_json::to_string_pretty(&v).unwrap() + "\n") (8+ copies, mixed unwrap() / unwrap_or_default()) → jsonc::write_json_pretty(&p, &v), keeping the exact 2-space-indent + trailing-\n byte format in one place.

Net: -142 / +156 lines, but the +156 is mostly the four new documented helpers and their tests; the duplicated call sites shrink substantially.

Test plan

  • cargo test (581 unit tests pass; new jsonc helper tests added. The caveman_cli integration test is flaky under compile load on both master and this branch — broken pipe writing to a stubbed CLI's stdin — unrelated to this change.)
  • cargo clippy --locked --workspace --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedantic
  • cargo fmt --check

Notes for reviewers

  • Risk areas / edge cases: read_json_object reads via the existing read_jsonc, so ~/.claude/settings.json, ~/.codex/hooks.json, and .cursor/hooks.json reads now tolerate JSONC (comments / trailing commas) instead of silently falling back to an empty object on serde_json::from_str failure. JSONC is a strict superset of JSON (see pure_json_passthrough test), so any previously-parsing input parses identically; this only adds tolerance and is aligned with the reason jsonc.rs exists (avoid clobbering a user's comment-bearing config).
  • Backwards compatibility: on-disk output format is unchanged (pretty JSON + trailing newline). merge_json/merge_opencode_mcp signatures changed from &PathBuf to &Path (satisfies clippy::ptr_arg; callers coerce automatically).

Link to Devin session: https://app.devin.ai/sessions/19a419ad0c9e458e8fcfeef00b82d066
Requested by: @getappz

Summary by CodeRabbit

  • Bug Fixes

    • Improved reliability when reading and updating Claude Code and OpenCode configuration files.
    • Preserved support for JSONC files containing comments and trailing commas.
    • Standardized configuration and rules locations to reduce path-related issues.
    • Improved cleanup when removing integrations and related configuration entries.
  • Refactor

    • Centralized configuration file handling for more consistent behavior across setup, synchronization, and removal workflows.
  • Tests

    • Added coverage for JSONC parsing, fallback behavior, and formatted configuration output.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@getappz getappz self-assigned this Jul 17, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR centralizes Claude and OpenCode configuration paths and JSONC object handling. Component, initialization, and uninstall flows now use shared readers and pretty writers, with tests covering fallback behavior, JSONC parsing, and newline-terminated output.

Changes

Configuration persistence standardization

Layer / File(s) Summary
Shared JSONC and path helpers
src/jsonc.rs, src/paths.rs
Adds object-validating JSONC reads, pretty JSON writes, and derived Claude/OpenCode paths with test coverage.
Component configuration updates
src/components.rs
Updates component checks, merges, rule targets, skill synchronization, and mode writes to use centralized paths and JSON helpers.
Initialization and hook wiring
src/init.rs
Standardizes configuration loading and persistence across Claude, Cursor, Codex, and OpenCode wiring flows.
Configuration cleanup
src/uninstall.rs
Uses centralized paths and shared JSON writing for Claude, OpenCode, and MCP cleanup.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Suggested reviewers: getappz

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: consolidating agent-config path handling and JSON read/write helpers.
Description check ✅ Passed The description matches the template well, with a clear summary, completed test plan, and reviewer notes on risks and compatibility.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch devin/1784314856-dedup-config-json-utils

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/uninstall.rs`:
- Around line 105-111: Update clean_opencode to replace the serde_json::from_str
read of opencode_config_path() with the shared JSONC-aware object reader.
Preserve the existing cleanup behavior while allowing comments and trailing
commas in opencode.jsonc so instructions are removed successfully.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7e07467c-9665-4cd5-986c-330d12bbac87

📥 Commits

Reviewing files that changed from the base of the PR and between a9d3095 and 1c13651.

📒 Files selected for processing (5)
  • src/components.rs
  • src/init.rs
  • src/jsonc.rs
  • src/paths.rs
  • src/uninstall.rs

Comment thread src/uninstall.rs
Comment on lines 105 to +111
fn clean_opencode(dry_run: bool) {
let rules_dir = home().join(".config").join("opencode").join("rules");
let rules_dir = opencode_rules_dir();
for f in &["exa.md", "git.md", "lean-ctx.md"] {
remove_file(&rules_dir.join(f), dry_run);
}

let config_path = home()
.join(".config")
.join("opencode")
.join("opencode.jsonc");
let config_path = opencode_config_path();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use the JSONC-aware reader for OpenCode cleanup.

opencode_config_path() targets opencode.jsonc, but the subsequent read still uses serde_json::from_str (Lines 113–117). Any valid config containing comments or trailing commas will fail parsing, so uninstall silently leaves its instructions behind. Replace this read with the shared JSONC object reader.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/uninstall.rs` around lines 105 - 111, Update clean_opencode to replace
the serde_json::from_str read of opencode_config_path() with the shared
JSONC-aware object reader. Preserve the existing cleanup behavior while allowing
comments and trailing commas in opencode.jsonc so instructions are removed
successfully.

@getappz
getappz enabled auto-merge (squash) July 17, 2026 19:28
@getappz
getappz merged commit aa672a3 into master Jul 17, 2026
16 checks passed
@getappz
getappz deleted the devin/1784314856-dedup-config-json-utils branch July 17, 2026 20:00
getappz added a commit that referenced this pull request Jul 21, 2026
…, #233) (#290)

* skill CLI: add skill crate dep, migrate serde_yaml→serde_yml, wire skill subcommand

Agentflare-Agent: 1
Agentflare-Branch: skill-detect-worktree

* skill CLI: wire skill subcommand into command dispatch (#233)

Agentflare-Agent: claude-code_2-1-216_agent
Agentflare-Branch: skill-detect-worktree

* skill-detect: intent classifier, MCP skill_detect tool, session/prompt injection hooks (#231)

Agentflare-Agent: claude-code_2-1-216_agent
Agentflare-Branch: skill-detect-worktree

* skill CLI: use crate::paths::home() for testability, add install/registry tests (#233)

Agentflare-Agent: claude-code_2-1-216_agent
Agentflare-Branch: skill-detect-worktree

* skill-detect: semantic re-rank via MiniLM ONNX pipeline, per-turn skill injection, clippy cleanup (#231)

Agentflare-Agent: claude-code_2-1-216_agent
Agentflare-Branch: skill-detect-worktree

* fix(skill-registry): drop unsound serde_yml, run cargo fmt

serde_yml has an unsound RUSTSEC-2025-0068 advisory (segfault risk in its
Serializer) and its upstream project is archived; cargo-deny flags the
0.0.13 this crate's unconstrained "0.0" requirement resolved to. Switched
to serde_yaml_ng, a maintained drop-in fork, for frontmatter parsing.
Also applies the cargo fmt fix CI flagged on types.rs.

Agentflare-Agent: claude-code_2-1-216_agent
Agentflare-Branch: item-231-233-skill-detect
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant