docs: add supply-chain security and CI test quality roadmap items#464
Conversation
Expand the deferred Phase 3 items into a dedicated phase after TypeScript Migration with detailed descriptions for each sub-item: event-driven pipeline, unified engine strategy, subgraph export filtering, transitive confidence, query caching, config profiles, pagination standardization, and plugin system. Renumber subsequent phases 5-9 → 6-10 with all cross-references updated.
…7, 4.8) Add two new roadmap sections after TypeScript Migration (Phase 4): - 4.7 Supply-Chain Security & Audit: npm audit in CI, SBOM generation, SLSA provenance, security audit log - 4.8 CI Test Quality & Coverage Gates: coverage thresholds in CI, unified test workflow, setTimeout/sleep cleanup, dependency audit
|
Claude finished @carlos-alm's task —— View job PR Review: Supply-Chain Security & CI Test Quality Roadmap ItemsReviewing PR #464 that adds roadmap items 4.7 and 4.8 for supply-chain security and CI test quality. Todo List
Analysis SummaryThe PR adds two new roadmap items (4.7 and 4.8) to Phase 4, along with minor corrections:
Root Cause AnalysisProblem Being Solved: Addressing missing CI hygiene and security practices that should be foundational before advanced features. The current CI lacks coverage enforcement, security auditing, and has test quality issues (312 setTimeout instances creating potential flakiness). Why This Approach: Logical placement in Phase 4 after TypeScript migration but before runtime extensibility. These are foundational CI practices that don't depend on later infrastructure work but benefit from the clean architecture established in Phase 3. Risk Assessment: Very low risk. This is documentation-only planning that adds no immediate dependencies or breaking changes. The proposed items follow industry best practices for supply-chain security and test quality. Backlog Compliance
Critical ConcernsNone identified. This is well-structured documentation that:
Minor Observations
Final Recommendation
Approved — This enhances the roadmap with essential CI practices that should indeed be foundational work. |
Greptile SummaryThis PR adds two new Phase 4 sub-sections ( Key changes:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Phase 4 — TypeScript Migration"]
A --> A1["4.1–4.6 (existing)"]
A --> A2["4.7 — Supply-Chain Security & Audit\n(npm audit, SBOM, SLSA, audit log)"]
A --> A3["4.8 — CI Test Quality & Coverage Gates\n(coverage thresholds, unified test workflow, timer cleanup)"]
A2 -->|"deliverable 1 cross-referenced"| A3
B["Phase 5 — Runtime & Extensibility"]
B --> B1["5.1–5.8 (existing)"]
B --> B2["5.9 — Developer Experience & Onboarding\n(codegraph init, npx, editor configs, guided CLI)"]
C["Phase 9 — GitHub Integration & CI"]
C --> C1["9.1–9.4 (existing)"]
C --> C2["9.5 — Auto-generated Docstrings\n(added in correct position ✅)"]
C --> C3["⚠️ 10.6 — Auto-generated Docstrings\nDUPLICATE — not removed from Phase 10"]
style C3 fill:#ffcccc,stroke:#cc0000
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| **Depends on:** 6.4 (`risk_score`, `complexity_notes`), 6.5 (module summaries) | ||
|
|
||
| ### 9.6 -- Auto-generated Docstrings | ||
| ### 9.5 -- Auto-generated Docstrings |
There was a problem hiding this comment.
Section 9.5 misplaced after Phase 10
This section (renumbered from 9.6 → 9.5) still sits after all Phase 10 sections (10.1–10.5), making it appear to belong to Phase 10 rather than Phase 9. Since this PR already touches the section number, it would be a good opportunity to move it to its correct position — after section 9.4 (SARIF Output, line 1537) and before the Phase 10 header.
There was a problem hiding this comment.
Fixed — moved section 9.5 (Auto-generated Docstrings) to its correct position after section 9.4 (SARIF Output) and before the Phase 10 header.
Add Phase 5.9 — Developer Experience & Onboarding: - codegraph init interactive wizard with editor config auto-detection - Published benchmarks across reference repos - npx zero-install support - Pre-built editor configs for Cursor, VS Code, Zed - Guided CLI output with next-step suggestions
| ### 4.8 -- CI Test Quality & Coverage Gates | ||
|
|
||
| **Gaps:** | ||
|
|
||
| - No coverage thresholds enforced in CI (coverage report runs locally only) | ||
| - Embedding tests in separate workflow requiring HuggingFace token | ||
| - 312 `setTimeout`/`sleep` instances in tests — potential flakiness under load | ||
| - No dependency audit step in CI (see also [4.7](#47----supply-chain-security--audit)) | ||
|
|
||
| **Deliverables:** | ||
|
|
||
| 1. **Coverage gate** -- add `vitest --coverage` to CI with minimum threshold (e.g. 80% lines/branches); fail the pipeline when coverage drops below the threshold | ||
| 2. **Unified test workflow** -- merge embedding tests into the main CI workflow using a securely stored `HF_TOKEN` secret; eliminate the separate workflow | ||
| 3. **Timer cleanup** -- audit and reduce `setTimeout`/`sleep` usage in tests; replace with deterministic waits (event-based, polling with backoff, or `vi.useFakeTimers()`) to reduce flakiness | ||
| 4. **Dependency audit step** -- add `npm audit --omit=dev` to CI (shared with 4.7) | ||
|
|
||
| **Affected files:** `.github/workflows/ci.yml`, `vitest.config.js`, `tests/` |
There was a problem hiding this comment.
Duplicate npm audit deliverable creates ownership ambiguity
Section 4.7 deliverable 1 and section 4.8 deliverable 4 both describe the same npm audit --omit=dev CI step. While 4.8 acknowledges the overlap with "(shared with 4.7)", having the same deliverable tracked in two independent sections risks:
- It being implemented twice (wasted effort) or
- Neither team/PR treating it as their primary responsibility ("the other section will handle it")
Consider removing deliverable 4 from 4.8 entirely and replacing it with a plain cross-reference note, e.g.:
Dependency audit step is covered by 4.7 deliverable 1.
This makes 4.7 the clear owner and 4.8 a consumer of that work.
There was a problem hiding this comment.
Fixed — removed the duplicate npm audit deliverable from 4.8 and replaced it with a cross-reference to 4.7, making ownership unambiguous.
1 participant
Summary
npm auditin CI, SBOM generation (CycloneDX/SPDX), SLSA provenance attestation, security audit logsetTimeout/sleepcleanup (312 instances), dependency audit stepTest plan