Initial commit: pygenium as git submodule
This commit is contained in:
211
src/checks/defensive-guards.ts
Normal file
211
src/checks/defensive-guards.ts
Normal file
@@ -0,0 +1,211 @@
|
||||
/**
|
||||
* checks/defensive-guards.ts — "redundant defensive guarding" check.
|
||||
*
|
||||
* Detects defensive code that guards invariants the type system or an
|
||||
* upstream validation already guarantees, and removes the redundant guards
|
||||
* while preserving guards that protect genuine external boundaries (user
|
||||
* input, IO, parsing, untrusted data). The rubric encodes the engineering rule:
|
||||
* no compatibility layers or fallbacks meant to be "replaced later" — remove
|
||||
* them outright rather than layering over them.
|
||||
*
|
||||
* Flagged smells (non-exhaustive):
|
||||
* - redundant-null-check — null/undefined check on a value whose declared
|
||||
* type is already non-nullable.
|
||||
* - swallowing-try-catch — try/catch that silently discards the error
|
||||
* (empty catch, catch that only logs, or catch returning a fallback that
|
||||
* hides the failure).
|
||||
* - rethrow-only-try-catch — try/catch whose body only rethrows the exact
|
||||
* error, adding nothing.
|
||||
* - error-masking-fallback — `return defaultValue` / `|| fallback` in a
|
||||
* catch that masks a real failure with a plausible-but-wrong value.
|
||||
* - defensive-guard-on-validated-input — re-checking input that a caller or
|
||||
* parser already validated (e.g. asserting a parsed enum is in range).
|
||||
* - compatibility-fallback — a fallback branch kept "for now" / "to be
|
||||
* replaced later" (engineering rule: remove, don't layer).
|
||||
*
|
||||
* Kept (legitimate boundary guards):
|
||||
* - untrusted input (HTTP params, CLI args, env vars, files on disk).
|
||||
* - IO (network, filesystem, subprocess) where failures are expected.
|
||||
* - parsing (`JSON.parse`, `parseInt`, `Date.parse`, schema decoders).
|
||||
*
|
||||
* Lifecycle:
|
||||
* gate (need source files) → recon (shared) → scan sub-agent writes
|
||||
* `<cwd>/pygienium/checks/defensive-guards/findings.md` separating redundant
|
||||
* guards from boundary guards → [with --fix] fix sub-agent removes redundant
|
||||
* guards, preserves boundary guards, and writes `changes.md` distinguishing
|
||||
* removed vs kept-with-reason.
|
||||
*
|
||||
* Registering this file is the ONLY wiring needed: `index.ts` auto-discovers
|
||||
* `src/checks/*.ts`, so dropping this file exposes `/pygienium-defensive-guards`.
|
||||
*
|
||||
* @module pygienium/checks/defensive-guards
|
||||
*/
|
||||
|
||||
import { readdirSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import {
|
||||
registerCheck,
|
||||
type CheckDefinition,
|
||||
type CheckScope,
|
||||
} from "./registry.js";
|
||||
|
||||
/** Output directory for this check's persistent reports. */
|
||||
export function defensiveGuardsOutputDir(cwd: string): string {
|
||||
return join(cwd, "pygienium", "checks", "defensive-guards");
|
||||
}
|
||||
|
||||
/** `findings.md` path for this check. */
|
||||
export function findingsPath(cwd: string): string {
|
||||
return join(defensiveGuardsOutputDir(cwd), "findings.md");
|
||||
}
|
||||
|
||||
/** `changes.md` path for this check. */
|
||||
export function changesPath(cwd: string): string {
|
||||
return join(defensiveGuardsOutputDir(cwd), "changes.md");
|
||||
}
|
||||
|
||||
/** Source extensions this check inspects. */
|
||||
const DEFENSIVE_GUARDS_EXTENSIONS = new Set([
|
||||
".ts",
|
||||
".tsx",
|
||||
".js",
|
||||
".jsx",
|
||||
".mjs",
|
||||
".cjs",
|
||||
".py",
|
||||
".rb",
|
||||
".go",
|
||||
".rs",
|
||||
".java",
|
||||
".kt",
|
||||
".swift",
|
||||
".php",
|
||||
".cs",
|
||||
".lua",
|
||||
]);
|
||||
|
||||
/**
|
||||
* Gate: skip when the cwd has no inspectable source files at all — a workspace
|
||||
* with zero source files gives the scanner nothing to analyse.
|
||||
*/
|
||||
function defensiveGuardsGate(cwd: string): string | undefined {
|
||||
let found = false;
|
||||
try {
|
||||
const entries = readdirSync(cwd);
|
||||
for (const entry of entries) {
|
||||
const dot = entry.lastIndexOf(".");
|
||||
const ext = dot === -1 ? "" : entry.slice(dot).toLowerCase();
|
||||
if (DEFENSIVE_GUARDS_EXTENSIONS.has(ext)) {
|
||||
found = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// unreadable cwd → let the agent decide; don't block.
|
||||
return undefined;
|
||||
}
|
||||
if (!found) {
|
||||
return "no source files found to inspect";
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the scan task. The defensive-guards scanner agent inspects the target,
|
||||
* classifies each guard as redundant or a legitimate boundary guard against the
|
||||
* rubric, and writes a structured findings report to `findings.md`. The output
|
||||
* path is passed into the task so both the real agent (which uses its `write`
|
||||
* tool) and the deterministic fake runner (which understands `!write <path>
|
||||
* <text>`) persist the report to the same location.
|
||||
*
|
||||
* The `!write`/`!echo` lines are the deterministic fallback the fake runner
|
||||
* executes for tests/smoke runs; a real model-driven agent receives the whole
|
||||
* prompt and writes a real analysis.
|
||||
*/
|
||||
function buildDefensiveGuardsScanTask(cwd: string, scope: CheckScope): string {
|
||||
const findings = findingsPath(cwd);
|
||||
const target = scope.target;
|
||||
return [
|
||||
`Inspect the target "${target}" (cwd: ${cwd}) for redundant defensive guarding.`,
|
||||
`Classify every guard (null check, try/catch, fallback) against your rubric as`,
|
||||
`either REDUNDANT (remove) or BOUNDARY (keep). Boundary guards protect real`,
|
||||
`external boundaries: untrusted input, IO, and parsing. Redundant guards protect`,
|
||||
`invariants the type system or upstream validation already guarantees.`,
|
||||
`Write your full findings report to: ${findings}.`,
|
||||
`findings.md must separate redundant guards from legitimate boundary guards,`,
|
||||
`listing each with: kind, evidence, disposition (remove | keep-boundary), and`,
|
||||
`reason.`,
|
||||
`Then emit a one-line summary referencing the findings path.`,
|
||||
``,
|
||||
`# Deterministic fallback (executed by the fake runner in tests):`,
|
||||
`!write ${findings} # Defensive-guards findings | summary: 2 redundant guard(s) flagged, 1 boundary guard kept | ## 1. ${target}/noise.ts:2 | kind: redundant-null-check | evidence: \`if (name === null)\` on \`name\` whose declared type is \`string\` (non-nullable) | disposition: remove | reason: type system already guarantees non-null | ## 2. ${target}/noise.ts:7 | kind: swallowing-try-catch | evidence: try/catch around doThing() discards the error silently (empty catch body) | disposition: remove | reason: masks bugs; no error mapping or recovery logic | ## 3. ${target}/boundary.ts:2 | kind: parsing-guard | evidence: try/catch around JSON.parse(input) | disposition: keep-boundary | reason: protects an external parsing boundary (JSON.parse of untrusted input)`,
|
||||
`!echo defensive-guards: 2 redundant, 1 boundary kept — see ${findings}`,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the fix task. The fixer consumes the scan findings and removes ONLY
|
||||
* redundant guards — those whose protected invariant is already guaranteed by
|
||||
* the type system or upstream validation. Boundary guards (IO, parsing,
|
||||
* untrusted input) are preserved untouched. Every action — removed or kept —
|
||||
* is recorded in `changes.md`, distinguishing removed (auto) from kept with a
|
||||
* reason (boundary).
|
||||
*
|
||||
* The fixer rewrites the affected source files with the redundant guards
|
||||
* excised; compatibility fallbacks are removed outright (engineering rule:
|
||||
* remove, don't layer), never left behind as a transitional shim.
|
||||
*/
|
||||
function buildDefensiveGuardsFixTask(
|
||||
cwd: string,
|
||||
scope: CheckScope,
|
||||
findings: string,
|
||||
): string {
|
||||
const changes = changesPath(cwd);
|
||||
const findingsFile = findingsPath(cwd);
|
||||
const target = scope.target;
|
||||
return [
|
||||
`Remove redundant defensive guards found in the scan.`,
|
||||
`cwd: ${cwd} target: ${target}`,
|
||||
`Findings report (also persisted at ${findingsFile}):`,
|
||||
`---`,
|
||||
findings,
|
||||
`---`,
|
||||
``,
|
||||
`Rules:`,
|
||||
`- Remove ONLY redundant guards: null/undefined checks on non-nullable types,`,
|
||||
` try/catch that only rethrows or swallows, fallback values that hide errors,`,
|
||||
` defensive guards on already-validated input, and compatibility fallbacks.`,
|
||||
`- PRESERVE boundary guards: anything protecting untrusted input, IO, or parsing`,
|
||||
` (e.g. JSON.parse, network, filesystem, subprocess errors). Do not touch them.`,
|
||||
`- No compatibility layers: remove fallbacks outright — never leave a shim meant`,
|
||||
` to be "replaced later".`,
|
||||
`- Apply the smallest diff that removes the guard without changing behaviour for`,
|
||||
` the happy path. Preserve tests and existing conventions.`,
|
||||
`- Write changes.md to ${changes} distinguishing removed (auto) from kept`,
|
||||
` (boundary — with reason) for every finding.`,
|
||||
``,
|
||||
`# Deterministic removal (executed by the fake runner in tests):`,
|
||||
`# Redundant null check + swallowing try/catch removed from noise.ts (auto).`,
|
||||
`# JSON.parse boundary guard in boundary.ts preserved (boundary).`,
|
||||
`!write ${target}/noise.ts // Cleaned by pygienium-defensive-guards: removed redundant null check on non-nullable \`name\` and the swallowing try/catch around doThing(). export function greet(name: string) { return \`hello \${name}\`; } export function swallow() { doThing(); } function doThing() {}`,
|
||||
`!write ${changes} # Defensive-guards changes | summary: 2 removed, 1 kept (boundary) | ## Removed (auto) | 1. ${target}/noise.ts:2 — redundant-null-check — removed \`if (name === null) return ""\`; type system guarantees non-null | 2. ${target}/noise.ts:7 — swallowing-try-catch — removed the try/catch around doThing(); the error is no longer silently swallowed | ## Kept (boundary — with reason) | 1. ${target}/boundary.ts:2 — parsing-guard — kept: try/catch around JSON.parse protects an external parsing boundary (untrusted input)`,
|
||||
`!echo defensive-guards: 2 removed, 1 kept (boundary) — see ${changes}`,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
/** The check definition; registers itself on import. */
|
||||
const defensiveGuardsCheck: CheckDefinition = {
|
||||
name: "defensive-guards",
|
||||
label: "Defensive guards",
|
||||
description:
|
||||
"Remove redundant defensive guarding (null checks on non-nullable types, swallowing try/catch, masking fallbacks) while keeping boundary guards (IO, parsing, untrusted input).",
|
||||
agentName: "defensive-guards",
|
||||
phaseId: "analysis",
|
||||
buildScanTask: buildDefensiveGuardsScanTask,
|
||||
buildFixTask: buildDefensiveGuardsFixTask,
|
||||
gate: defensiveGuardsGate,
|
||||
};
|
||||
|
||||
registerCheck(defensiveGuardsCheck);
|
||||
|
||||
export { defensiveGuardsCheck };
|
||||
Reference in New Issue
Block a user