feat: persona-driven skill architecture — journey engine, PR gate, new skills - #46
feat: persona-driven skill architecture — journey engine, PR gate, new skills#46aanishs wants to merge 3 commits into
Conversation
Add plain-english.json with human-readable descriptions and why_it_matters for all HIPAA-mapped NIST controls. New /comply-explain skill looks up any control by ID, CFR section, or keyword. Enriches comply-db control --json with plain English context. Updates comply-assess and comply-auto to show why_it_matters during interviews. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix journey skip/complete identical status, normalize control ID casing, error on invalid ID in --json mode, extract findControlInCatalog helper (DRY), merge dual DB connections, reject unknown config keys, split getDbPath read/write, revert premature comply-auto deprecation, add journey + config + invalid-ID tests, fix skill counts to 14, add plain-english.test.ts to test script. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…w skills - /comply v3.0: rewritten as journey engine with persona detection, Framework Advisor (3 questions), milestone routing, and persona-aware recommendations for founders, engineers, and compliance officers - bin/comply-check: new PR gate CLI that checks diffs for PHI patterns, sensitive data, removed security controls, and BAA-requiring dependencies - nist/baa-vendors.json: curated list of 13 services requiring BAAs - /comply-policy: new skill for generating personalized policy documents - /comply-deal: new skill for answering prospect security questionnaires - /soc2 v3.0: 3 domain-specific questions (trust criteria, audit type, driver) - /gdpr v3.0: 4 domain-specific questions (role, locations, lawful basis, DPO) - /pci-dss v3.0: 3 domain-specific questions (card handling, CDE scope, SAQ) - /comply-auto v3.0: deprecated, points to /comply Autopilot mode - Updated all comply-auto references across skill templates - Tests updated: 14 → 16 skills (added comply-deal, comply-policy) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR introduces a persona-driven Three correctness issues exist in
Confidence Score: 4/5The journey engine, skills, comply-db additions, and test updates are safe to merge; bin/comply-check has three P1 issues that should be fixed before relying on it in CI. Three P1 findings all live in the newly introduced bin/comply-check binary and do not affect any existing functionality. They would make the gate miss PHI leakage and security-control removals, and silently succeed when its own check-loading fails — important to fix before CI integration. bin/comply-check requires attention on all three P1 issues before the gate is wired into CI pipelines. Important Files Changed
Sequence DiagramsequenceDiagram
participant CI as CI Pipeline
participant CC as comply-check
participant Git as git diff
participant CR as checks-registry.ts
CI->>CC: bun comply-check --diff HEAD~1
CC->>CR: loadCodeChecks() via regex parse
alt zero checks loaded (order mismatch)
CR-->>CC: checks = []
CC-->>CI: WARNING + exit 0 ❌ (should be exit 2)
else checks loaded OK
CR-->>CC: code_grep checks[]
end
CC->>Git: getDiffFiles() --diff-filter=ACMR
Git-->>CC: added/modified files, addedLines only
Note over CC,Git: ❌ deleted lines (-) never collected
CC->>CC: runCodeChecks(addedLines, checks)
CC->>CC: checkNewDependencies(package.json addedLines)
CC->>CC: filter by severity threshold
Note over CC: ❌ phi-identifiers (INFO) < default warn (HIGH)
alt gatedFindings.length > 0
CC-->>CI: findings + exit 1 (blocks PR)
else
CC-->>CI: clean + exit 0
end
Reviews (1): Last reviewed commit: "feat: persona-driven skill architecture ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Parse added lines (lines starting with +, excluding +++ header) | ||
| const addedLines: { num: number; text: string }[] = []; | ||
| let lineNum = 0; | ||
| for (const line of diffOutput.split('\n')) { | ||
| const hunkMatch = line.match(/^@@ -\d+(?:,\d+)? \+(\d+)/); | ||
| if (hunkMatch) { | ||
| lineNum = parseInt(hunkMatch[1]) - 1; | ||
| continue; | ||
| } | ||
| if (line.startsWith('+') && !line.startsWith('+++')) { | ||
| lineNum++; | ||
| addedLines.push({ num: lineNum, text: line.slice(1) }); | ||
| } else if (!line.startsWith('-')) { | ||
| lineNum++; | ||
| } |
There was a problem hiding this comment.
getDiffFiles() filters to --diff-filter=ACMR and only parses lines starting with + (added). Lines starting with - are never collected, so runCodeChecks never sees them. The PR description lists "removed security controls" as a checked feature, but deleting an ensureEncrypted() call, a requireMFA() guard, or any audit-logging statement is completely invisible to the gate — it will exit 0 as if nothing was removed.
| function loadCodeChecks(): CodeCheck[] { | ||
| const registryPath = path.join(import.meta.dir, '..', 'frameworks', 'checks-registry.ts'); | ||
| const content = fs.readFileSync(registryPath, 'utf-8'); | ||
|
|
||
| // Extract code_grep checks by parsing the array literals | ||
| const checks: CodeCheck[] = []; | ||
| const checkRegex = /\{\s*id:\s*"([^"]+)"[\s\S]*?category:\s*"([^"]+)"[\s\S]*?description:\s*"([^"]+)"[\s\S]*?type:\s*"code_grep"[\s\S]*?pattern:\s*"([^"]+)"[\s\S]*?severity_default:\s*"([^"]+)"/g; | ||
|
|
||
| let match; | ||
| while ((match = checkRegex.exec(content)) !== null) { | ||
| checks.push({ | ||
| id: match[1], | ||
| category: match[2], | ||
| description: match[3], | ||
| pattern: match[4], | ||
| severity: match[5], | ||
| }); | ||
| } | ||
| return checks; |
There was a problem hiding this comment.
loadCodeChecks() parses checks-registry.ts with a regex that requires object properties in a fixed order: id → category → description → type:"code_grep" → pattern → severity_default. If any future code_grep check is written with fields in a different order, the regex produces zero matches. The tool prints a warning but continues and exits 0 — the gate passes every PR as if it were clean, with no way to distinguish this from a real clean run in CI.
The checks.length === 0 guard should exit 2 instead:
| function loadCodeChecks(): CodeCheck[] { | |
| const registryPath = path.join(import.meta.dir, '..', 'frameworks', 'checks-registry.ts'); | |
| const content = fs.readFileSync(registryPath, 'utf-8'); | |
| // Extract code_grep checks by parsing the array literals | |
| const checks: CodeCheck[] = []; | |
| const checkRegex = /\{\s*id:\s*"([^"]+)"[\s\S]*?category:\s*"([^"]+)"[\s\S]*?description:\s*"([^"]+)"[\s\S]*?type:\s*"code_grep"[\s\S]*?pattern:\s*"([^"]+)"[\s\S]*?severity_default:\s*"([^"]+)"/g; | |
| let match; | |
| while ((match = checkRegex.exec(content)) !== null) { | |
| checks.push({ | |
| id: match[1], | |
| category: match[2], | |
| description: match[3], | |
| pattern: match[4], | |
| severity: match[5], | |
| }); | |
| } | |
| return checks; | |
| if (checks.length === 0) { | |
| console.error('ERROR: No code_grep checks loaded from checks-registry.ts. Cannot run gate.'); | |
| process.exit(2); | |
| } |
|
|
||
| const SEVERITY_ORDER: Record<string, number> = { | ||
| 'INFO': 0, 'LOW': 1, 'MEDIUM': 2, 'HIGH': 3, 'CRITICAL': 4 | ||
| }; | ||
| const THRESHOLD: Record<string, string> = { | ||
| 'info': 'INFO', 'warn': 'HIGH', 'block': 'CRITICAL' | ||
| }; | ||
| const thresholdSeverity = THRESHOLD[severity] || 'HIGH'; |
There was a problem hiding this comment.
phi-identifiers and health-data-fields both have severity_default: "INFO" (order value 0). The default --severity is warn, which maps to a threshold of HIGH (order value 3). Those checks are added to allFindings but are filtered out of gatedFindings, so adding ssn, dob, mrn, or patient names to source code exits 0 with the default invocation. For a HIPAA compliance gate, the two most basic PHI-detection checks should block by default.
| // Read full file content for context | ||
| let content = ''; | ||
| try { | ||
| content = fs.readFileSync(name, 'utf-8'); | ||
| } catch { | ||
| // File might not exist (deleted) | ||
| } | ||
|
|
||
| if (addedLines.length > 0) { | ||
| files.push({ path: name, content, addedLines }); | ||
| } | ||
| } |
There was a problem hiding this comment.
DiffFile.content loaded but never used
fs.readFileSync(name, 'utf-8') reads the entire current file into DiffFile.content on every changed file, but neither runCodeChecks nor checkNewDependencies ever accesses file.content — both iterate only over file.addedLines. On a large PR that touches generated files or lock files this is unnecessary I/O. Consider removing the field or deferring the read until a check actually needs full-file context.
1 participant
Summary
Test plan
bun test test/architecture.test.ts— skill count updated 14 → 16, new skills in expected arraybun test test/skill-validation.test.ts— template and generated file counts updatedbun run gen:skill-docs— all 16 skills regenerate cleanlybin/comply-check --diff HEAD~1 --json— returns findings JSON with severity classification/complyin Claude Code, verify persona prompt and journey routing/comply-deal, verify questionnaire parsing flow/comply-policy, verify policy generation prompt🤖 Generated with Claude Code