Skip to content

feat: added skip-agents-md cli flag#517

Merged
magyargergo merged 7 commits into
abhigyanpatwari:mainfrom
gjcampbell:feat/skip-agent-md
Mar 28, 2026
Merged

feat: added skip-agents-md cli flag#517
magyargergo merged 7 commits into
abhigyanpatwari:mainfrom
gjcampbell:feat/skip-agent-md

Conversation

@gjcampbell

Copy link
Copy Markdown
Contributor

Summary

Adds a new gitnexus analyze --skip-agents-md flag to skip updating the GitNexus-managed sections in AGENTS.md and CLAUDE.md, allowing users to preserve manual/custom instructions in the gitnexus sections of those files.

Motivation / context

Some users maintain custom guidance in AGENTS.md / CLAUDE.md and don’t want gitnexus analyze to rewrite those files' GitNexus blocks on each run. This flag provides an explicit opt-out while keeping the default behavior unchanged. This addresses #510

Areas touched

  • gitnexus/ (CLI / core / MCP server)
  • gitnexus-web/ (Vite / React UI)
  • .github/ (workflows, actions)
  • eval/ or other tooling
  • Docs / agent config only (AGENTS.md, CLAUDE.md, .cursor/, llms.txt, etc.)

Scope & constraints

In scope

  • Add a new CLI flag gitnexus analyze --skip-agents-md.
  • When the flag is set, skip updating the GitNexus-managed sections in AGENTS.md and CLAUDE.md.
  • Plumb the option from CLI → AnalyzeOptions → generateAIContextFiles(...).
  • Update docs (README.md, gitnexus/README.md) to mention the new flag.
  • Add/adjust unit tests to cover the new behavior and CLI help output.

Explicitly out of scope / not done here

  • Changing the default behavior of gitnexus analyze (still updates AGENTS.md / CLAUDE.md unless the flag is provided).
  • Changing the format/markers/content of the GitNexus inserted blocks.
  • Any changes to embeddings, skill generation behavior, indexing pipeline logic, or MCP server behavior.
  • Any changes to gitnexus-web.

Implementation notes

  • Implemented as an optional AIContextOptions argument to generateAIContextFiles(...) (skipAgentsMd?: boolean).
  • The skip is applied by guarding the existing upsert calls for AGENTS.md and CLAUDE.md behind if (!options?.skipAgentsMd).
  • Commander wiring adds --skip-agents-md and maps it into AnalyzeOptions.skipAgentsMd.

Testing & verification

  • cd gitnexus && npm test
  • cd gitnexus && npm run test:integration (if core/indexing/MCP paths changed)
  • cd gitnexus && npx tsc --noEmit
  • cd gitnexus-web && npm test (if web changed)
  • cd gitnexus-web && npx tsc -b --noEmit (if web changed)
  • Manual / Playwright E2E (note environment — see gitnexus-web/e2e/)

If you didn’t actually run npm test / tsc yet, flip those [x] back to [ ] before opening.

Risk & rollout

  • Risk: Low. Behavior change is behind an opt-in flag; default behavior remains unchanged.
  • Breaking changes: None intended.
  • Rollout: No migration required. Users who want to preserve manual AGENTS.md / CLAUDE.md edits can re-run indexing with:
    • gitnexus analyze --skip-agents-md
  • Rollback: Revert the flag wiring and the generateAIContextFiles option gate; docs/tests revert cleanly.

Checklist

  • PR body meets repo minimum length (workflow may label short descriptions)
  • If AGENTS.md / overlays changed: headers, scope block, and changelog updated per project conventions
  • No secrets, tokens, or machine-specific paths committed

@vercel

vercel Bot commented Mar 26, 2026

Copy link
Copy Markdown

@gjcampbell is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

@magyargergo

Copy link
Copy Markdown
Collaborator

⚠️ Upcoming Prettier formatting — rebase instructions

PR #563 adds Prettier as the code formatter for the repo. When it merges, the bulk format commit will touch ~350 files (style-only: whitespace, quotes, trailing commas). Your branch will likely conflict.

After #563 merges, rebase your branch:

git fetch origin
git checkout <your-branch>
git rebase origin/main

# Conflicts will be formatting-only — accept your version:
git checkout --theirs .
git add .
git rebase --continue

# Then re-format your branch to match the new style:
npx prettier --write .
git add -A
git commit -m "style: apply prettier formatting"
git push --force-with-lease

New setup step: Run npm install at the repo root (not just in gitnexus/) to get prettier + activate the pre-commit hook. The hook auto-formats staged files on every commit going forward.

@gjcampbell

Copy link
Copy Markdown
Contributor Author

⚠️ Upcoming Prettier formatting — rebase instructions

PR #563 adds Prettier as the code formatter for the repo. When it merges, the bulk format commit will touch ~350 files (style-only: whitespace, quotes, trailing commas). Your branch will likely conflict.

After #563 merges, rebase your branch:

git fetch origin
git checkout <your-branch>
git rebase origin/main

# Conflicts will be formatting-only — accept your version:
git checkout --theirs .
git add .
git rebase --continue

# Then re-format your branch to match the new style:
npx prettier --write .
git add -A
git commit -m "style: apply prettier formatting"
git push --force-with-lease

New setup step: Run npm install at the repo root (not just in gitnexus/) to get prettier + activate the pre-commit hook. The hook auto-formats staged files on every commit going forward.

Done.

@magyargergo

Copy link
Copy Markdown
Collaborator

@claude review it

@github-actions

github-actions Bot commented Mar 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 2m 24s —— View job


PR Review: feat: added skip-agents-md cli flag

Tasks:

  • Gather context and read changed files
  • Review core logic (ai-context.ts)
  • Review CLI wiring (analyze.ts, index.ts)
  • Review tests
  • Review docs
  • Post final review

Overall this is a well-motivated, low-risk feature. The core concept and CLI wiring are clean. However, I found a test/implementation mismatch that will cause test failures, a missing error handling regression, and a scope creep issue that's the biggest concern.


🔴 Critical: Test/Implementation Mismatch

gitnexus/test/unit/ai-context.test.ts:102-103 expects the returned files array to contain skip-notification strings when skipAgentsMd: true:

expect(result.files).toContain('AGENTS.md (skipped via --skip-agents-md)');
expect(result.files).toContain('CLAUDE.md (skipped via --skip-agents-md)');

But gitnexus/src/cli/ai-context.ts:311-321 never pushes anything to createdFiles when the flag is set:

if (!options?.skipAgentsMd) {
  // pushes 'AGENTS.md (result)' and 'CLAUDE.md (result)'
}
// nothing pushed for the skip case

result.files won't contain those strings, so the test will fail. Either add the skip entries to createdFiles, or update the test to match the actual behavior (e.g. assert that result.files does NOT contain AGENTS/CLAUDE entries).

Fix this →


🔴 Critical: Error Handling Removed in analyze.ts

The original analyze.ts wrapped runFullAnalysis() in a try/catch that ensured cleanup on failure:

} catch (err: any) {
  clearInterval(elapsedTimer);
  process.removeListener('SIGINT', sigintHandler);
  console.log = origLog; // restore patched console
  bar.stop();
  console.error(`\n  Analysis failed: ${err.message}\n`);
  process.exitCode = 1;
  return;
}

The new inlined code has no error handling. If any step throws (e.g. runPipelineFromRepo, loadGraphToLbug, createFTSIndex), the process exits with:

  • A dangling setInterval (elapsedTimer)
  • console.log/warn/error still patched to barLog
  • The SIGINT handler still registered
  • The progress bar not stopped

Fix this →


🟡 Major: skipAgentsMd Not Wired Through the Shared Orchestrator

analyze-worker.ts (used by the MCP server's analyze path) still calls runFullAnalysis() from core/run-analyze.ts. That module's AnalyzeOptions interface does not include skipAgentsMd, and generateAIContextFiles() is called without it at run-analyze.ts:315. The new flag only works via the direct CLI path, not via the web UI or any programmatic caller using runFullAnalysis.

If this is intentional (flag only for CLI use), it should be documented. Otherwise, run-analyze.ts's AnalyzeOptions and the generateAIContextFiles call there need to be updated too.


🟡 Significant Scope Creep: analyze.ts Refactored Beyond the Flag

The PR description explicitly lists as out of scope: "Any changes to embeddings, skill generation behavior, indexing pipeline logic". But analyze.ts was rewritten to inline ~250 lines of pipeline logic previously delegated to runFullAnalysis().

This creates:

  • Duplicate logic: run-analyze.ts and analyze.ts now contain parallel implementations of the same pipeline
  • Divergence risk: Any future fix to the shared orchestrator won't apply to the CLI path
  • Dead weight: The --skills flag handling was simplified but also moved out of runFullAnalysis, meaning the shared orchestrator's pipelineResult return path may be unused

The flag itself only required ~15 lines of change (ai-context.ts + analyze.ts option passing + index.ts). The analyze.ts refactor should be a separate PR.


🟠 Minor: AIContextOptions Not Exported

gitnexus/src/cli/ai-context.ts:27-29: AIContextOptions is declared but not exported. Since generateAIContextFiles is exported and accepts options?: AIContextOptions, callers outside the module need to know the shape. Exporting it (or using an inline type) avoids any future friction.


✅ What's Good

  • The core guard pattern (if (!options?.skipAgentsMd)) is clean and correct in intent
  • Commander wiring in index.ts:27 is minimal and follows existing patterns
  • Docs updated in both README.md and gitnexus/README.md
  • The test covers the happy path (files unchanged, custom content preserved) well once the files assertion is fixed

Summary: The flag logic itself is sound, but two test+implementation mismatches and a missing error-handling regression need to be fixed before merge. The analyze.ts refactor should ideally be separated or at minimum double-checked against the original runFullAnalysis contract.

@gjcampbell

Copy link
Copy Markdown
Contributor Author

Resolved issues.

@gjcampbell

Copy link
Copy Markdown
Contributor Author

@claude review?

@magyargergo

Copy link
Copy Markdown
Collaborator

It's only enabled for some contributors.

@magyargergo

Copy link
Copy Markdown
Collaborator

@claude is this pr ready for release?

@github-actions

github-actions Bot commented Mar 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 1m 0s —— View job


PR Release Readiness Assessment

Tasks:

  • Gather context from previous review
  • Read changed files
  • Check if critical issues were resolved
  • Post final verdict

Yes — this PR is ready to merge. All issues from the previous review have been resolved.

Previous Issues: Status

Issue Status
🔴 Test/Implementation mismatch (result.files not containing skip strings) ✅ Fixed — else branch now pushes 'AGENTS.md (skipped via --skip-agents-md)' and 'CLAUDE.md (skipped via --skip-agents-md)' to createdFiles (ai-context.ts:321-324)
🔴 Error handling removed from analyze.ts ✅ Fixed — runFullAnalysis() is now called inside a proper try/catch (analyze.ts:172-288) with full cleanup: clearInterval, removeListener, console restore, bar.stop(), process.exitCode = 1
🟡 skipAgentsMd not wired through shared orchestrator ✅ Fixed — AnalyzeOptions in run-analyze.ts now includes skipAgentsMd?: boolean (line 50), and it's correctly passed to generateAIContextFiles at line 330
🟡 Scope creep / inlined pipeline logic in analyze.ts ✅ Resolved — analyze.ts now correctly delegates to runFullAnalysis() from run-analyze.ts; no duplicate pipeline logic
🟠 AIContextOptions not exported ✅ Fixed — export interface AIContextOptions at ai-context.ts:27

Code Looks Clean

  • ai-context.ts: Guard pattern is correct; skip path pushes the expected notification strings; AIContextOptions exported
  • analyze.ts: Properly delegates to runFullAnalysis(), passes skipAgentsMd, full error handling with cleanup
  • run-analyze.ts: skipAgentsMd in AnalyzeOptions and plumbed through to generateAIContextFiles — both CLI and MCP/worker paths now respect the flag
  • index.ts: --skip-agents-md CLI option wired correctly (line 27)
  • Tests: Test assertions now match the actual behavior; both the files content assertions and the file-preservation assertions are correct
  • Docs: Both README.md and gitnexus/README.md updated with the new flag

No Remaining Concerns

The implementation is minimal, correct, and well-tested. Default behavior is unchanged. The flag is purely opt-in and low risk.

@github-actions

Copy link
Copy Markdown
Contributor

CI Report

All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Tests success unit tests, 3 platforms
✅ E2E success gitnexus-web changes only

Test Results

Tests Passed Failed Skipped Duration
4717 4671 0 46 182s

✅ All 4671 tests passed

46 test(s) skipped — expand for details
  • buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature
  • Swift constructor-inferred type resolution > detects User and Repo classes, both with save methods
  • Swift constructor-inferred type resolution > resolves user.save() to Models/User.swift via constructor-inferred type
  • Swift constructor-inferred type resolution > resolves repo.save() to Models/Repo.swift via constructor-inferred type
  • Swift constructor-inferred type resolution > emits exactly 2 save() CALLS edges (one per receiver type)
  • Swift self resolution > detects User and Repo classes, each with a save function
  • Swift self resolution > resolves self.save() inside User.process to User.save, not Repo.save
  • Swift parent resolution > detects BaseModel and User classes plus Serializable protocol
  • Swift parent resolution > emits EXTENDS edge: User → BaseModel
  • Swift parent resolution > emits IMPLEMENTS edge: User → Serializable (protocol conformance)
  • Swift cross-file User.init() inference > resolves user.save() via User.init(name:) inference
  • Swift cross-file User.init() inference > resolves user.greet() via User.init(name:) inference
  • Swift return type inference > detects User class and getUser function
  • Swift return type inference > detects save function on User (Swift class methods are Function nodes)
  • Swift return type inference > resolves user.save() to User#save via return type of getUser() -> User
  • Swift return-type inference via function return type > resolves user.save() to User#save via return type of getUser()
  • Swift return-type inference via function return type > user.save() does NOT resolve to Repo#save
  • Swift return-type inference via function return type > resolves repo.save() to Repo#save via return type of getRepo()
  • Swift implicit imports (cross-file visibility) > detects UserService class in Models.swift
  • Swift implicit imports (cross-file visibility) > resolves UserService() constructor call across files (no explicit import)
  • Swift implicit imports (cross-file visibility) > resolves service.fetchUser() member call across files
  • Swift implicit imports (cross-file visibility) > creates IMPORTS edges between files in the same module
  • Swift extension deduplication > detects Product class
  • Swift extension deduplication > resolves Product() constructor despite extension creating duplicate class node
  • Swift extension deduplication > resolves product.save() to Product.swift (primary definition)
  • Swift constructor call fallback (no new keyword) > resolves OCRService() as constructor call across files
  • Swift constructor call fallback (no new keyword) > resolves ocr.recognize() member call via constructor-inferred type
  • Swift export visibility (internal vs private) > resolves PublicService() constructor across files
  • Swift export visibility (internal vs private) > resolves internalHelper() across files (internal = module-scoped)
  • Swift if let / guard let binding resolution > detects User and Repo classes
  • Swift if let / guard let binding resolution > resolves user.save() inside if-let to User#save
  • Swift if let / guard let binding resolution > resolves repo.save() inside guard-let to Repo#save
  • Swift if let / guard let binding resolution > user.save() in if-let does NOT resolve to Repo#save
  • Swift await / try expression unwrapping > resolves user.save() via await fetchUser() return type
  • Swift await / try expression unwrapping > resolves repo.save() via try parseRepo() return type
  • Swift await / try expression unwrapping > detects fetchUser and parseRepo as functions
  • Swift for-in loop element type inference > detects User and Repo classes
  • Swift for-in loop element type inference > creates implicit import edges between files
  • Swift field-type resolution > detects classes and their properties
  • Swift field-type resolution > emits HAS_PROPERTY edges from class to field
  • Swift field-type resolution > resolves field-chain call user.address.save() → Address#save
  • Swift field-type resolution > emits ACCESSES edges for field reads in chains
  • Swift field-type resolution > populates field metadata (visibility, declaredType) on Property nodes
  • Swift call-result binding > resolves call-result-bound method call user.save() → User#save
  • Swift call-result binding > getUser() is present as a defined function
  • Swift call-result binding > emits processUser -> getUser CALLS edge for let-assigned free function call

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 70.32% 12607/17926 70.32% = 0.0 🟢 ██████████████░░░░░░
Branches 59.69% 8287/13882 59.69% = 0.0 🟢 ███████████░░░░░░░░░
Functions 74.88% 1103/1473 74.88% = 0.0 🟢 ██████████████░░░░░░
Lines 72.51% 11492/15847 72.51% = 0.0 🟢 ██████████████░░░░░░

📋 View full run · Generated by CI

@magyargergo magyargergo merged commit b03413d into abhigyanpatwari:main Mar 28, 2026
11 of 12 checks passed
icodebuster pushed a commit to icodebuster/GitNexus that referenced this pull request Mar 31, 2026
* main: (114 commits)
  feat(csharp): C# MethodExtractor config (abhigyanpatwari#582)
  docs: add gitnexus-shared build step before gitnexus-web (abhigyanpatwari#585)
  chore: add enterprise offering section to README, ignore local_docs/ (abhigyanpatwari#579)
  fix(eval): exclude litellm 1.82.7 and 1.82.8 due to compatibility issues (abhigyanpatwari#580)
  feat(java,kotlin): MethodExtractor abstraction with per-language configs (abhigyanpatwari#576)
  feat: added skip-agents-md cli flag (abhigyanpatwari#517)
  feat(wiki): Azure OpenAI support for wiki command (abhigyanpatwari#562)
  refactor: reduce explicit any types (abhigyanpatwari#566)
  feat(java): method references, worker overload disambiguation, interface dispatch (abhigyanpatwari#540)
  feat: configure eslint with unused import removal (abhigyanpatwari#564)
  feat: configure prettier with pre-commit hook (abhigyanpatwari#563)
  feat: unify web and cli ingestion pipeline (abhigyanpatwari#536)
  fix/opencode mcp gitnexus timeout (abhigyanpatwari#363)
  chore: bump version to 1.4.10, update CHANGELOG
  fix: resolve tree-sitter peer dependency conflicts (abhigyanpatwari#538)
  chore: bump version to 1.4.9, add CHANGELOG.md
  refactor: Phase 8 & 9 — Field Types and Return-Type Binding (abhigyanpatwari#494)
  feat: add COBOL language support with regex extraction pipeline (abhigyanpatwari#498)
  fix: close remaining Dart language support gaps (abhigyanpatwari#524)
  refactor: split global BUILT_IN_NAMES into per-language provider fields (abhigyanpatwari#523)
  ...

# Conflicts:
#	gitnexus/src/core/wiki/llm-client.ts
@abhigyanpatwari abhigyanpatwari mentioned this pull request Apr 1, 2026
3 tasks
motolese pushed a commit to motolese/datamoto-gitnexus that referenced this pull request Apr 23, 2026
* feat: added skip-agents-md cli flag

* fix: apply prettier formatting

* feat: added skip-agents-md cli flag

* fix: apply prettier formatting

* feat: add skipAgentsMd option to skip AGENTS.md and CLAUDE.md updates

* fixed bad merge
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.

2 participants