Comprehension Gate

Comprehension Gate

You are performing a pre-merge comprehension review. The core insight this skill encodes: automated checks (linters, type checkers, tests) verify that code is correct in isolation. They cannot detect that a change's blast radius extends to a service in another repo, that an auth token has no TTL, that a cache key is shared across tenants, or that a "cleanup" just deleted the persistent state an AI agent was supposed to preserve.

This review exists because the person merging a change may not have the full system picture in their head — especially when that change was AI-generated.

Never say "looks good" as a default verdict. Every dimension requires an explicit finding. An absence of evidence is not evidence of absence — if you can't verify a dimension, say so.

Step 1: Gather Context

Run these in parallel before starting the analysis:

1. Get the diff — git diff HEAD for staged changes, or git diff main...HEAD for a branch. If neither is available, ask the user to paste the diff or specify which files changed.

2. Read module context files — for every directory touched by the diff, check for:

   • MODULE_MANIFEST.md — dependency map, shared resources, owner
   • BEHAVIORAL_CONTRACTS.md — interface guarantees for changed functions
   • DECISION_LOG.md — Warning entries that apply to changed code

3. Find dependents of changed interfaces — grep the broader codebase for callers of any function, class, or export that was modified. A function change can break callers in files the diff doesn't touch.

4. Check for shared resource patterns in the diff — grep the changed lines for: cache key patterns, database table names, queue/topic names, environment variable references, file paths that could be shared. These are the cross-service data flows.

5. Check if this appears to be AI-generated — look for: large uniform code blocks with no personal style, patterns common to AI output (comprehensive error handling on every line, unusually detailed comments, perfectly consistent formatting throughout), or if the commit/PR description says it was generated. Note this in the artifact — AI-generated code warrants explicit comprehension verification.

Report what context was found before starting analysis:

Context loaded:
• Diff: [N lines across N files]
• Module manifests found: [list] / none
• DECISION_LOG warnings applying to this change: [list] / none  
• Dependents of changed interfaces: [list] / none found
• AI-generated code: likely / unlikely / uncertain

Step 2: Seven-Dimension Analysis

Work through each dimension in order. For each, produce an explicit finding — even if the finding is "no issues found in this dimension." Vague reassurance is not a finding.

Read references/seven-dimensions-checklist.md for detailed guidance on each dimension.

Dimension 1: Credential and Secret Exposure

Look for: API keys, tokens, passwords, private keys, connection strings appearing in the diff. Look contextually, not just with regex — a variable named key that receives a value from config may be fine; the same variable assigned a string literal is not.

Also check: are credentials being logged? Written to a file? Sent over an unencrypted channel? Are environment variable names changed in a way that would silently break credential loading?

Dimension 2: Cross-Service Side Effects

Look for: writes to shared resources identified in MODULE_MANIFEST.md (shared Redis keys, shared DB tables, shared queues). Look for: new data flows introduced by this change (the diff calls a service that wasn't called before, or writes to a key that wasn't written before).

The cross-tenant incident pattern: does any write use a key or identifier that could be confused across tenants, users, or sessions? A cache key like result:{query_hash} without a tenant prefix is a classic cross-tenant leak vector.

New shared resource writes require isolation evidence. When the diff introduces a write to a shared resource that is net-new (present in the added lines but not in the removed lines — i.e., not a modification of an existing write), apply the isolation evidence standard:

A CLEAR verdict on a new shared resource write requires positive evidence of tenant isolation, not just the absence of visible problems.

Acceptable evidence (one of the following must be present):

1. A test in the diff or referenced PR that demonstrates tenant A cannot read tenant B's data through this write path
2. A code comment or review note explicitly documenting the isolation mechanism (e.g., "key is scoped by tenant_id always present from auth middleware")
3. A BEHAVIORAL_CONTRACTS.md "Isolation guarantee" field documenting the mechanism for this interface

If none of the above is present, issue REVIEW REQUIRED with this blocking question:

Dimension 2 — Cross-service side effects:
Line [N] introduces a new [cache write / shared DB write] at [key pattern / table].
No tenant isolation evidence was found in the diff.

Before CLEAR can be issued, provide one of:
  a) A test demonstrating tenant A cannot read tenant B's data through this path
  b) A code comment or contract entry documenting the isolation mechanism
  c) Confirmation that this write path is single-tenant by design (explain how)

Reference: /generate-isolation-tests can scaffold option (a) for this interface.

Dimension 3: Blast Radius

Using the dependents found in Step 1 plus any MODULE_MANIFEST.md data: trace forward. If this change modifies an interface, what calls that interface? What calls those callers? Where does the blast radius end?

Be specific. "Blast radius: UserService → AuthMiddleware → every authenticated endpoint" is a finding. "Could affect other services" is not.

Dimension 4: State and Persistence Mismatch (Kiro Pattern)

This is the pattern that destroyed Amazon's staging environment: an AI agent treated a persistent resource as ephemeral because nothing in the code made its persistence visible.

Look for: code that deletes, recreates, or resets something that might have persistent state (databases, cache stores, environment configs, cloud resources, secrets stores, queues with unprocessed messages). Look for: code that assumes a resource is empty when it might not be. Look for: "cleanup" operations that are irreversible.

If MODULE_MANIFEST.md or DECISION_LOG.md has Warning entries about persistent state, cross-check them against this diff explicitly.

Dimension 5: Token and Session TTL Management

Look for: token creation without expiry, session creation without TTL, cache writes without TTL, JWT signing without expiry claim, API key creation without rotation policy.

In automated/agent workflows this is especially critical: a token created by an agent that runs once may exist forever if no TTL is set, and may accumulate permissions as the system evolves.

Dimension 6: Implicit Assumptions

Look for: assumptions about ordering (this must run before that), timing (within N seconds), concurrency (only one caller at a time), load (works under N requests/second), data shape (this field will always be present), or system state (the cache will always be warm).

These assumptions are often correct in development and wrong in production under load. If an assumption is not enforced by the code (a lock, a schema constraint, a rate limit), it is a risk — note it.

Dimension 7: Comprehension Check

Could the person who is about to merge this change explain it clearly to an on-call engineer at 3 AM when it's causing an incident?

If the change was AI-generated: was it reviewed line by line, or was the output accepted because the tests passed? Tests passing is not comprehension.

Ask: does this change have a clear, specific description of what it does (not just what the ticket said to do)? Is there a COMPREHENSION_ARTIFACT.md from a previous review, or is this the first one?

Step 3: Write the Comprehension Artifact

Write the artifact to .claude/comprehension/COMPREHENSION_ARTIFACT.md at the project root. If the directory doesn't exist, create it. If a previous artifact exists for the current branch, read it first — you're updating it, not replacing it.

Use the template in references/comprehension-artifact-template.md.

The artifact must include:

• Change summary: what the code actually does, in plain language. Not the PR title.
• Findings table: one row per finding, with severity (HIGH / MEDIUM / LOW / INFO), dimension, and specific details (file:line where possible)
• Blast radius map: even if it's "limited to this service"
• Questions before merging: numbered, specific, blocking questions that must be answered before this is safe to merge
• Verdict: exactly one of:
  • CLEAR — no blocking findings; this is safe to merge
  • REVIEW REQUIRED — findings that need discussion but don't necessarily block merge
  • HOLD — blocking findings; do not merge until resolved

Specificity rule: Every finding must name a specific file, line, variable, or pattern. "Possible caching issue" is not a finding. "Line 47 writes the auth token to cache.set('token', value) with no TTL; this key is shared across all users" is a finding.

Step 4: Report

After writing the artifact, report:

Comprehension gate complete.

Verdict: [CLEAR / REVIEW REQUIRED / HOLD]
Findings: [N HIGH, N MEDIUM, N LOW, N INFO]
Artifact: .claude/comprehension/COMPREHENSION_ARTIFACT.md

[If HOLD]: Blocking issues:
  1. [specific issue]
  2. [specific issue]

[If any context was missing]: Note: No MODULE_MANIFEST.md found in [dirs]. 
Run /context-layer-generator on those modules for deeper analysis.

Arguments

/comprehension-gate — reviews staged changes (git diff HEAD)
/comprehension-gate branch — reviews the current branch against main (git diff main...HEAD)
/comprehension-gate <file> — reviews a specific file
/comprehension-gate --diff <pasted diff> — reviews a pasted diff