src/lib/services/holds.ts:42-53 hand-rolls its own token/access check (resolveOwningParentId + resolveShardForParent + tokenAllowsParent) instead of calling the shared requireAccessibleParent/requireAccessibleRecord helper every other permission-gated service function uses (e.g. documents.ts:132, records.ts:436,686).
CLAUDE.md states the audit log is "populated on every write/delete tool call, denied MCP attempt" — but because holds.ts doesn't go through the shared helper, a denied hold request never produces the _denied audit-log entry logDenial (permissions.ts:45-52) generates for every other permission-denied path. An admin filtering the audit log for denied attempts (per the PRD's requirement: "An admin can filter the audit log by agent identity and time range and see every action that agent took") would silently miss every denied hold attempt — a real gap in a documented guarantee, not a style preference.
Surfaced during the 2026-09-12 architecture audit (code-level scan of src/lib/services and src/lib/mcp).
Done when: a denied hold request appears in audit_log filtered by denial, the same as a denied write/delete/read already does, verified by a Tier A test.
Priority: P1 — a documented audit-completeness guarantee is silently broken for exactly one MCP tool. Added to 0.5.0 — Stability, Reliability, and QA alongside the milestone's other audit/reliability work (#58, #88).
src/lib/services/holds.ts:42-53hand-rolls its own token/access check (resolveOwningParentId+resolveShardForParent+tokenAllowsParent) instead of calling the sharedrequireAccessibleParent/requireAccessibleRecordhelper every other permission-gated service function uses (e.g.documents.ts:132,records.ts:436,686).CLAUDE.md states the audit log is "populated on every write/delete tool call, denied MCP attempt" — but because
holds.tsdoesn't go through the shared helper, a denied hold request never produces the_deniedaudit-log entrylogDenial(permissions.ts:45-52) generates for every other permission-denied path. An admin filtering the audit log for denied attempts (per the PRD's requirement: "An admin can filter the audit log by agent identity and time range and see every action that agent took") would silently miss every denied hold attempt — a real gap in a documented guarantee, not a style preference.Surfaced during the 2026-09-12 architecture audit (code-level scan of src/lib/services and src/lib/mcp).
holds.ts's hand-rolled access check withrequireAccessibleParent/requireAccessibleRecordhold_recordscall now produces a_deniedaudit-log entry, matching every other permission-denied pathhold_records, gets denied, and the audit log records the denialDone when: a denied hold request appears in
audit_logfiltered by denial, the same as a denied write/delete/read already does, verified by a Tier A test.Priority: P1 — a documented audit-completeness guarantee is silently broken for exactly one MCP tool. Added to 0.5.0 — Stability, Reliability, and QA alongside the milestone's other audit/reliability work (#58, #88).