Fix/dev review findings - #1
Merged
Merged
Conversation
PostToolUse does not honour permissionDecision — that field belongs to PreToolUse. Emitting it meant the invisible-character warning either never reached Claude or the payload was rejected as schema-invalid, while the tests only asserted the absence of updatedToolOutput. - the documented per-event field sets now live in one place (DOCUMENTED_FIELDS) and render_outcome renders per event: PostToolUse carries additionalContext only - a clean PostToolUse result prints nothing; a malformed payload or internal fault uses the documented exit 2 + stderr channel with no JSON on stdout - the PostToolUse scan is bounded by HookConfig.max_posttooluse_texts (default 64) - new TestEventContract asserts the emitted field set is a subset of the documented set for every outcome kind — the check that would have caught this
hook-config printed a bare 'aiitg hook pretooluse'. A hook command that cannot start is a NON-BLOCKING error in Claude Code, so the document would reach the model unscanned while the user believes it was checked — the worst failure mode for a gateway like this. - hook-config resolves the executable with shutil.which, quotes it, and warns on stderr when it is not on PATH (with --exe to override) - aiitg hook doctor self-checks the executable, quarantine and cache directories, the fail-closed default and the enforced-format count; exits 1 on any failure, --json available
The as-built section gains the four post-review changes (event-aware rendering, no verdict on PostToolUse, absolute hook command, bounded scan) and a new Known gaps section listing what is deliberately left open: no prune command for the cache/quarantine dirs, one audit line per repeated read, an unreachable decision-is-None branch, and the WebFetch updatedToolOutput path.
Three ways the cache could lie about a document: - the key was only (path, mtime_ns, size), so an aiitg upgrade or a --mode change kept serving yesterday's verdict for an unchanged file. Entries now carry a namespace of version | mode | detector set | policy rules and a mismatch is a miss - a structurally incomplete entry (valid JSON, missing keys) passed the check and then blew up while being rebuilt, which the guard turned into a DENY for a clean document. HookCache.get now validates the required keys and types and returns a miss instead - allow decisions cached the document's text even though only a quarantine decision needs it PostToolUse findings are also recorded through the shipped AuditLog now that --audit is honoured (POL-TOOL-001, action allow, counts in the note): that event runs after the tool, so it reports rather than enforces.
…ng in doctor aiitg hook posttooluse accepted --mode, --cache-dir, --quarantine-dir, --max-file-bytes and --queue, none of which can apply to an event that runs after the tool: a flag that does nothing is the same failure class as a hook that silently does not run. - posttooluse keeps only --audit and --fail-open - hook doctor now checks that a PreToolUse hook calling 'aiitg hook pretooluse' is actually present in the project or user .claude/settings.json (or --settings PATH), and that the command in it resolves — a wired-but-unstartable hook is FAIL, not a silent bypass
…contribution policies - README: install as a tool (uv tool install / pipx) because the hook needs aiitg on PATH, a note that the decision cache and audit log are local state rather than a trust boundary, a link to the design document, and the accurate test counts - SECURITY.md: in-scope (silent non-enforcement, detection/policy bypass, crash-to-allow) and out-of-scope (incomplete detection, an attacker with local user access, approved content) - CONTRIBUTING.md: gates, English-only rule, conventional commits, generated fixtures, and the design rules the hook relies on
The static badge went stale twice (105 advertised while the suite was at 152). The number is now checked: the badge and the 'make test # N tests' comment must equal the collected test count, so a commit that adds tests without updating them fails locally and in CI.
0.1.0 was never released, so the changelog starts at 0.2.0 documenting the M3 hook adapter, the security fixes and the project hygiene. Adds [project.urls] and, importantly, replaces the author email in pyproject.toml with the public noreply identity — the previous value was a personal address baked into the published package metadata.
Numbers 10-15 cover the fixes made after the plan was written: PostToolUse flags that could not apply, doctor not checking wiring, cached verdicts unbound to the build, damage flipping a verdict, allow decisions storing document text, and repository hygiene. The known-gaps list is rewritten so only genuinely open items remain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.