Add Bash script support#1217
Conversation
|
I tested this PR against a deployment repository with 101 Bash scripts and found a reproducible grammar ABI failure on common I opened a focused patch against the author's branch: Omar-Elwazeery#1 The patch vendors the official ABI-15 tree-sitter-bash 0.25.1 WASM with its MIT notice, keeps Bash imports exact-path, conservatively folds only a root-level deterministic script-directory idiom, and adds regressions. On a main-based integration branch it indexed all 101/101 |
1745053 to
c8a48a2
Compare
|
Rebased onto the latest main and force-pushed (c8a48a2). Bash support is still verified with targeted Bash tests and a successful build. The PR is mergeable; the only remaining blocker is review approval. |
There was a problem hiding this comment.
CodeGraph review
@Omar-Elwazeery — 1 finding on this change.
Overall risk: 🟠 Medium
This change adds Bash language support, including shebang detection, grammar loading, and a new extractor. The most important risk is that extensionless files are unconditionally classified as Bash for grammar loading, which could cause parsing failures for non‑Bash files.
Findings
| Severity | Finding | Where |
|---|---|---|
| 🟠 Medium | Overbroad language detection for extensionless files | src/extraction/index.ts:150 — see inline comment |
Worth double-checking
- detectLanguageForGrammarLoad — Verify that treating any extensionless file as Bash (line 150) does not cause incorrect grammar loading for non‑Bash files.
- collectIncludedFiles — Note that collectIncludedFiles has no test coverage; consider adding tests to ensure it correctly filters files, especially with the new Bash shebang logic.
Business rules (2 honored · 2 not applicable)
No rules violated or at risk.
QA checklist — 3 things to verify in the running product
- cli — Run
codegraph syncon a repository containing a Bash script with a proper shebang and confirm the script is indexed and appears in the language list. (Ensures the new Bash extraction and shebang detection work end‑to‑end.) - cli — Run
codegraph syncon a repository without any Bash scripts and verify the sync completes without errors. (Confirms that the added Bash support does not break existing sync behavior for other languages.) - api — Query
getSupportedLanguages()after sync and check that 'bash' is listed. (Validates that the language registry correctly includes Bash.)
Blast radius: 53 files affected beyond the diff · 161 symbols · 42 test files selected
Tests to run:
__tests__/adaptive-explore-sizing.test.ts__tests__/android-res-exclusion.test.ts__tests__/concurrent-locking.test.ts__tests__/config-secret-redaction.test.ts__tests__/context-ranking.test.ts__tests__/context.test.ts__tests__/db-reopen-on-replace.test.ts__tests__/dynamic-boundaries.test.ts__tests__/exclude-config.test.ts__tests__/explore-blast-radius.test.ts__tests__/explore-corroboration-ranking.test.ts__tests__/explore-nl-stopword-collision.test.ts__tests__/explore-output-budget.test.ts__tests__/explore-result-count.test.ts__tests__/explore-synth-constant-endpoints.test.ts__tests__/extension-mapping.test.ts__tests__/extraction.test.ts__tests__/frameworks.test.ts__tests__/frontload-hook.test.ts__tests__/grammar-wasm-bytes.test.ts
| function detectLanguageForGrammarLoad(filePath: string, overrides?: Record<string, Language>): Language { | ||
| const lang = detectLanguage(filePath, undefined, overrides); | ||
| return lang === 'unknown' && isPotentialBashShebangPath(filePath) ? 'bash' : lang; | ||
| } |
There was a problem hiding this comment.
🟠 Medium — Overbroad language detection for extensionless files
detectLanguageForGrammarLoad treats any extensionless file as Bash, potentially loading the Bash grammar for non‑Bash files and causing parse errors (src/extraction/index.ts line 150).
Summary
Fixes #1203.
.sh,.bash,.bats, and extensionless scripts with Bash/POSIX shell shebangs..batsimport resolution.Testing
npm test -- __tests__/extraction.test.ts __tests__/resolution.test.ts __tests__/sync.test.ts -t Bashnpm run buildnpm testcurrently times out in the full-suite parallel run at__tests__/mcp-daemon.test.ts > proxy survives the daemon dying mid-session and keeps serving (#662). I confirmed the same full-suite timeout occurs on a clean base after stashing this branch, and the exact test passes when run isolated both before and after this change.