Skip to content

fix(memory): improve extraction, indexing, and scanner reliability - #90

Merged
mulhamna merged 5 commits into
mainfrom
feat/memory-fixes-and-omp-support
Sep 20, 2026
Merged

mulhamna merged 5 commits into
mainfrom
feat/memory-fixes-and-omp-support

Conversation

@badrus123

@badrus123 badrus123 commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes correctness and stability issues across ForgeGuard's memory subsystem, scanner, duplication detection, coverage handling, MCP schema, and command runner.

OMP integration has been removed from this PR. It remains out of scope until suiflex/kurir provides native OMP skill-directory, hook-file, and hook-registration support.

Changes

  • Memory extraction
    • Correctly identify exported Rust items with attributes or doc comments and keep signatures free of decorators.
    • Extract exported JavaScript/TypeScript arrow functions and function expressions.
    • Normalize Python raw route strings and qualified superclass names.
  • Memory store and indexing
    • Add a 5-second SQLite busy timeout.
    • Preserve schema metadata correctly and rebuild on schema mismatch.
    • Refresh stale indexes across commits, remove deleted or newly unindexable files, and preserve imported artifact metadata.
    • Fix receiver-scoped caller/callee counts and module-name normalization.
  • Queries and tracing
    • Ignore write keywords inside Cypher string literals and avoid splitting clauses inside quoted text.
    • Keep inbound and outbound trace branches separate during bidirectional traversal.
  • Core scanner and gate
    • Scan secrets before returning on syntax errors.
    • Respect maximum file size in semantic duplicate checks.
    • Add accurate duplicate finding ranges and support arrow-function clone scopes.
    • Normalize duplicate baseline evidence and merge duplicate LCOV records.
  • Runner and CLI reliability
    • Terminate timed-out Unix process groups portably and fail closed when SBOM commands produce no valid JSON.
    • Require both stdin and stdout terminals before launching the init wizard.
    • Use typed MCP detail/direction arguments and keep JSON stdout valid when MCP registration fails.
    • Correct the Codex stop-hook block payload.

User-visible result

  • Code-memory queries handle decorated Rust symbols, JavaScript/TypeScript arrows, Python routes, quoted Cypher values, and bidirectional traces correctly.
  • Concurrent memory access waits briefly instead of failing immediately on SQLite contention.
  • Timed-out checks do not leave descendant processes running.
  • OMP support is intentionally deferred to a follow-up after native kurir support is released.

Verification

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo test --locked --workspace
  • cargo build --locked --workspace --release
  • sh tests/install_test.sh
  • sh tests/wizard_test.sh
  • forgeguard gate --changed --output compact
  • Required guardener / forgeguard check

… support

- Memory extract: fix Rust is_exported and signature attribute skipping; resolve JS/TS arrow functions and export statements; strip Python raw route prefixes; isolate superclass name in class inheritance.
- Memory store: configure SQLite busy_timeout to 5s; fix schema_version retention in reset; copy commit/last_indexed metadata on artifact import; fix receiver-scoped callees_of filtering; improve module_stem extension stripping.
- Memory cypher: allow legitimate identifiers (delete, select, create) in string literals without triggering read-only keyword rejection; quote-aware find_keyword.
- Memory trace: prevent sideways walks in Direction::Both by tracking branch direction during BFS.
- Harnesses: add AgentTarget::Omp with rules, skills (.agents/skills), hooks (.agents/hooks.json), and MCP support; update Codex block hook wire format; add typed DetailArg/DirectionArg enums for MCP tools.
- Core gate: run secret scanning prior to syntax error early return; populate end_line on duplicate block findings and include arrow_function in clone scope; normalize baseline evidence for FG-DRY rules.
- Tests: add regression tests for memory extraction, cypher identifier safety, OMP project initialization, and CLI updates.
@badrus123
badrus123 requested a review from a team September 20, 2026 13:29
@suiflex-bot suiflex-bot Bot added commit: feat maintainer area: cli ForgeGuard CLI crate (crates/forgeguard-cli) area: core Core engine crate (crates/forgeguard-core) area: tests Integration test scripts (tests/) area: ci GitHub Actions workflows and templates labels Sep 20, 2026
@mulhamna

Copy link
Copy Markdown
Member

Hey @badrus123, thanks for putting this together!

The bugfixes across the memory subsystem & core scanner look really solid:

  • AST extraction in Rust (skipping attributes/doc comments in signature and is_exported) and TS arrow function exports address real edge cases nicely.
  • Cypher parser fix stripping string literals before checking forbidden write keywords avoids false positives on common identifier names like delete or select.
  • Adding the 5s busy_timeout to SQLite and killing process groups on runner timeout are great stability improvements.

That said, I have a few major architectural concerns regarding the OMP harness integration:

  1. Bypassing the Kurir Abstraction:
    Across our repos, all harness handling (resolving skill directories, hook file discovery, and hook mutation) is standardized and delegated to suiflex/kurir. In kurir 0.2.0, skills_dir(Harness::Omp) and hook_file(Harness::Omp) are not yet implemented.
  2. Borrowing Harness::Codex in init.rs:
    In install_omp, calling skill_directory(Harness::Codex, scope) to borrow Codex's skill directory bypasses the Kurir contract and introduces brittle coupling.
  3. Collision on .agents/hooks.json:
    .agents/hooks.json is currently owned by Antigravity. Forcing OMP hooks into the same file via install_antigravity_simple_hook means lifecycle events (Stop, PreInvocation) will overwrite each other whenever both harnesses are configured or when running --agent all.

Suggested Path Forward:

  • Split this PR:
    1. PR 1 (Core & Memory Fixes): Extract the memory extraction, Cypher parser, SQLite timeout, runner timeout, and duplication fixes. This part is clean, well-tested, and ready to merge immediately.
    2. OMP Support: Standardize OMP in suiflex/kurir first (implementing proper skills_dir, hook_file, and hook registration). Once a new kurir release is out, we can bump the dependency in ForgeGuard and consume OMP natively without borrowing other harnesses or conflicting on hook files.

What do you think?

@suiflex-bot suiflex-bot Bot added commit: refactor and removed area: ci GitHub Actions workflows and templates labels Sep 20, 2026
@badrus123 badrus123 changed the title feat(memory): fix AST extraction, cypher parsing, and add OMP harness support fix(memory): improve extraction, indexing, and scanner reliability Sep 20, 2026
@mulhamna mulhamna self-assigned this Sep 20, 2026
@mulhamna
mulhamna merged commit 0e70391 into main Sep 20, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli ForgeGuard CLI crate (crates/forgeguard-cli) area: core Core engine crate (crates/forgeguard-core) area: tests Integration test scripts (tests/) commit: chore commit: feat commit: fix commit: refactor maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants