mirror of
https://github.com/LerianStudio/ring
synced 2026-04-21 13:37:27 +00:00
fix(gate-validator): address CodeRabbit review on PR #356
- Add backup before overwriting hook in installer (consistent with existing backup pattern for agents/skills/commands) - Add TTL-based eviction for pendingValidations Map (prevents memory leak if tool.execute.after never fires on timeout/crash) - Fix tilde fallback: skip HOME-based candidate path when HOME is undefined instead of using literal '~' that Node can't resolve - Document non-atomic race window in restore-and-validate flow Requested-by: @jeff-music-city
This commit is contained in:
parent
0ae4b2966b
commit
0a4b06c0f8
2 changed files with 26 additions and 9 deletions
|
|
@ -480,6 +480,10 @@ fi
|
|||
GATE_VALIDATOR="$RING_ROOT/dev-team/hooks/validate-gate-progression.sh"
|
||||
if [[ -f "$GATE_VALIDATOR" ]]; then
|
||||
mkdir -p "$TARGET_ROOT/hooks"
|
||||
if [[ -f "$TARGET_ROOT/hooks/validate-gate-progression.sh" ]]; then
|
||||
mkdir -p "$BACKUP_DIR/hooks"
|
||||
cp "$TARGET_ROOT/hooks/validate-gate-progression.sh" "$BACKUP_DIR/hooks/validate-gate-progression.sh"
|
||||
fi
|
||||
cp "$GATE_VALIDATOR" "$TARGET_ROOT/hooks/validate-gate-progression.sh"
|
||||
chmod +x "$TARGET_ROOT/hooks/validate-gate-progression.sh"
|
||||
echo " Installed hooks/validate-gate-progression.sh"
|
||||
|
|
|
|||
|
|
@ -20,22 +20,32 @@ import { execSync } from "node:child_process"
|
|||
/** Per-call state: tracks files we need to validate after write */
|
||||
const pendingValidations = new Map<
|
||||
string,
|
||||
{ filePath: string; previousContent: string | null }
|
||||
{ filePath: string; previousContent: string | null; timestamp: number }
|
||||
>()
|
||||
|
||||
/** Max age for pending entries (5 minutes) — prevents leaks on tool timeouts */
|
||||
const PENDING_TTL_MS = 5 * 60 * 1000
|
||||
|
||||
/** Evict stale entries on each before hook call */
|
||||
function evictStale(): void {
|
||||
const now = Date.now()
|
||||
for (const [key, entry] of pendingValidations) {
|
||||
if (now - entry.timestamp > PENDING_TTL_MS) {
|
||||
pendingValidations.delete(key)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the validate-gate-progression.sh script path.
|
||||
*/
|
||||
function findValidatorScript(projectRoot: string): string | null {
|
||||
const candidates = [
|
||||
path.join(projectRoot, "dev-team", "hooks", "validate-gate-progression.sh"),
|
||||
path.join(
|
||||
process.env.HOME ?? "~",
|
||||
".config",
|
||||
"opencode",
|
||||
"hooks",
|
||||
"validate-gate-progression.sh",
|
||||
),
|
||||
// HOME-based path (skip if HOME undefined — literal "~" won't resolve)
|
||||
...(process.env.HOME
|
||||
? [path.join(process.env.HOME, ".config", "opencode", "hooks", "validate-gate-progression.sh")]
|
||||
: []),
|
||||
path.join(projectRoot, ".ring", "hooks", "validate-gate-progression.sh"),
|
||||
]
|
||||
|
||||
|
|
@ -111,7 +121,8 @@ export function createToolExecuteBefore(projectRoot: string) {
|
|||
}
|
||||
}
|
||||
|
||||
pendingValidations.set(input.callID, { filePath: absPath, previousContent })
|
||||
evictStale()
|
||||
pendingValidations.set(input.callID, { filePath: absPath, previousContent, timestamp: Date.now() })
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -147,6 +158,8 @@ export function createToolExecuteAfter(projectRoot: string) {
|
|||
|
||||
// Temporarily restore previous state so the script's regression check
|
||||
// compares against the ORIGINAL state, not the already-written content.
|
||||
// NOTE: Non-atomic window between restore and re-write. Acceptable in
|
||||
// single-agent usage; concurrent file access could read reverted state.
|
||||
if (pending.previousContent !== null) {
|
||||
fs.writeFileSync(pending.filePath, pending.previousContent, "utf-8")
|
||||
} else {
|
||||
|
|
|
|||
Loading…
Reference in a new issue