From 89dfc85abec257c1d1aa9882c53c567360250baf Mon Sep 17 00:00:00 2001 From: Bill Thornton Date: Thu, 2 Jul 2026 15:07:58 -0700 Subject: [PATCH] fix(linear): converge snapshot cache reader/writer keys (TAP-4588) The Linear snapshot payload cache almost never self-hit: the reader (tapps_linear_snapshot_get) and the writer (auto-populate hook + agents' own get/put) computed different keys for the same open-issue slice. The sentinel/gate layer already collapsed aliases (TAP-1374) but the payload key did not. Root cause + fix: - state alias: add _canonical_state() mapping ""/None/"open"/open-bucket members to one canonical "open" token; _resolve_cache_key canonicalizes the filename segment AND the hashed state field. Closed buckets (completed/canceled) stay isolated. Mirrored in the two bash key snippets, the PowerShell variant, and all 3 regenerated committed hooks. - limit in hash: dropped from the key entirely; enforced at read time via a superset-limit fallback in snapshot_get (stored>=requested serves, truncated, served_from_superset=true; smaller stored limit MISSES). snapshot_put now records the stored limit. - poisoning: snapshot_get returns a miss (not a false empty hit) for an auto_populated payload with issues==[]; the auto-populate hook skips the write when the request state was an alias/invalid AND the result is empty. Tests: +7 behavioral tests (canonicalization hit, superset truncation, smaller-can't-serve-larger, closed-bucket isolation, poisoning guard, manual-empty-still-hits). Updated the assertions that encoded the pre-fix multi-key/limit-sensitive behavior to the new convergence contract (TAP-1374 cross-alias unlock preserved via convergence). Full tapps-mcp unit suite green (5952 passed). Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/hooks/tapps-post-linear-list.sh | 27 ++- .../hooks/tapps-post-linear-snapshot-get.sh | 25 ++- .claude/hooks/tapps-pre-linear-list.sh | 25 ++- .../pipeline/platform_hook_templates.py | 62 ++++-- .../src/tapps_mcp/server_linear_tools.py | 94 ++++++++- .../tests/unit/test_linear_cache_gate.py | 12 +- .../tests/unit/test_linear_list_gateway.py | 29 ++- .../tests/unit/test_server_linear_tools.py | 188 +++++++++++++++++- 8 files changed, 408 insertions(+), 54 deletions(-) diff --git a/.claude/hooks/tapps-post-linear-list.sh b/.claude/hooks/tapps-post-linear-list.sh index bae3b636..667c5418 100755 --- a/.claude/hooks/tapps-post-linear-list.sh +++ b/.claude/hooks/tapps-post-linear-list.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # tapps-mcp-hook-version: 3.12.48 -# tapps-mcp-hook-content-sha: 21da2082 +# tapps-mcp-hook-content-sha: 643764c9 # TappsMCP PostToolUse hook — Linear list_issues auto-populate (TAP-1412) # After a successful mcp__plugin_linear_linear__list_issues call, write the # response into .tapps-mcp-cache/linear-snapshots/.json so the next @@ -32,15 +32,24 @@ except Exception: limit = 50 if not team or not project: sys.exit(0) +# TAP-4588: canonicalize the open-bucket alias and drop limit from the hash so +# this writer's key matches server _resolve_cache_key / the reader. +OPEN_BUCKET = ('backlog', 'unstarted', 'started', 'triage') +def _canon_state(s): + s_lc = (s or '').strip().lower() + if s_lc == '' or s_lc == 'open' or s_lc in OPEN_BUCKET: + return 'open' + return s_lc +canon = _canon_state(state) filt = {k: v for k, v in sorted({ - 'state': state, 'label': label, 'limit': limit, + 'state': canon, 'label': label, }.items()) if v not in (None, '')} payload = json.dumps(filt, sort_keys=True, default=str).encode('utf-8') fhash = hashlib.sha256(payload).hexdigest()[:16] key = '__'.join([ team.replace('/', '_') or '_', project.replace('/', '_') or '_', - (state or 'any').replace('/', '_'), + (canon.replace('/', '_') or 'any'), fhash, ]) resp = d.get('tool_response') or d.get('toolResponse') or {} @@ -69,8 +78,17 @@ def _find_issues(o): return r return None issues = _find_issues(resp) or [] -# TTL aligned with server-side _ttl_for_state defaults (5 min open, 1 h closed). +# TAP-4588 poisoning guard: list_issues(state='open') (a tapps-mcp alias, not a +# real Linear state) returns [] — caching that empty list under the canonical +# 'open' key would make a later get falsely report 0 issues. Skip the write +# when the raw request state was an alias/invalid AND the result is empty. +VALID_LINEAR_STATES = ( + 'backlog', 'unstarted', 'started', 'triage', 'completed', 'canceled' +) state_lc = state.lower() +if not issues and state_lc and state_lc not in VALID_LINEAR_STATES: + sys.exit(0) +# TTL aligned with server-side _ttl_for_state defaults (5 min open, 1 h closed). ttl = 3600 if state_lc in ('completed', 'canceled') else 300 now = time.time() out = { @@ -81,6 +99,7 @@ out = { 'team': team, 'project': project, 'auto_populated': True, + 'limit': limit, } root = os.environ.get('TAPPS_PROJECT_ROOT') or os.getcwd() cache_dir = os.path.join(root, '.tapps-mcp-cache', 'linear-snapshots') diff --git a/.claude/hooks/tapps-post-linear-snapshot-get.sh b/.claude/hooks/tapps-post-linear-snapshot-get.sh index 0a3ae295..a29fd3ed 100755 --- a/.claude/hooks/tapps-post-linear-snapshot-get.sh +++ b/.claude/hooks/tapps-post-linear-snapshot-get.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # tapps-mcp-hook-version: 3.12.48 -# tapps-mcp-hook-content-sha: b071ddb7 +# tapps-mcp-hook-content-sha: 5a2c3acd # TappsMCP PostToolUse hook — Linear cache-gate sentinel writer (TAP-1224) # Writes a per-(team, project, state, label, limit) sentinel on BOTH # cached=true and cached=false responses from tapps_linear_snapshot_get. @@ -33,27 +33,36 @@ except Exception: limit = 50 # Open-bucket alias: tapps-mcp's TTL bucket 'open' covers backlog, unstarted, # started, triage. The skill tells agents to snapshot_get(state='open') and -# then list_issues with a concrete state. Without alias support the keys -# differ and the gate self-trips (TAP-1374). Fix: derive a bucket alias and -# emit additional sentinels for it. Same logic on both sides. +# then list_issues with a concrete state. TAP-4588: canonicalize any open +# alias ('' / 'open' / bucket member) to ONE token so the payload key and the +# sentinel key converge — matching server _canonical_state. limit is dropped +# from the hash (enforced at read time via the superset fallback). Same logic +# on both sides — see server_linear_tools._resolve_cache_key. OPEN_BUCKET = ('backlog', 'unstarted', 'started', 'triage') state_lc = state.lower() +def _canon_state(s): + s_lc = (s or '').strip().lower() + if s_lc == '' or s_lc == 'open' or s_lc in OPEN_BUCKET: + return 'open' + return s_lc def _key_for(state_part: str) -> str: + canon = _canon_state(state_part) filt = {k: v for k, v in sorted({ - 'state': state_part, 'label': label, 'limit': limit, + 'state': canon, 'label': label, }.items()) if v not in (None, '')} payload = json.dumps(filt, sort_keys=True, default=str).encode('utf-8') fhash = hashlib.sha256(payload).hexdigest()[:16] parts = [ (team.replace('/', '_') or '_'), (project.replace('/', '_') or '_'), - ((state_part or 'any').replace('/', '_')), + (canon.replace('/', '_') or 'any'), fhash, ] return '__'.join(parts) key = _key_for(state) -# Bucket alias keys: when state is 'open' (a tapps-mcp alias), '' (any), or -# any open-bucket member, every other open-bucket member should resolve. +# With canonicalization every open-bucket alias resolves to the same key, so +# the alias set is a singleton ({key}). We still emit the bucket variants and +# de-dup so the set matches the Python _alias_keys contract byte-for-byte. alias_keys = [] if not team or not project: key = '' diff --git a/.claude/hooks/tapps-pre-linear-list.sh b/.claude/hooks/tapps-pre-linear-list.sh index 7de9da0e..e37edfb6 100755 --- a/.claude/hooks/tapps-pre-linear-list.sh +++ b/.claude/hooks/tapps-pre-linear-list.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # tapps-mcp-hook-version: 3.12.48 -# tapps-mcp-hook-content-sha: adb4be12 +# tapps-mcp-hook-content-sha: 69a96eb2 # TappsMCP PreToolUse hook — Linear cache-first read gate (TAP-1224) # Gates raw mcp__plugin_linear_linear__list_issues calls behind a recent # tapps_linear_snapshot_get sentinel for the same (team, project, state, @@ -36,27 +36,36 @@ except Exception: limit = 50 # Open-bucket alias: tapps-mcp's TTL bucket 'open' covers backlog, unstarted, # started, triage. The skill tells agents to snapshot_get(state='open') and -# then list_issues with a concrete state. Without alias support the keys -# differ and the gate self-trips (TAP-1374). Fix: derive a bucket alias and -# emit additional sentinels for it. Same logic on both sides. +# then list_issues with a concrete state. TAP-4588: canonicalize any open +# alias ('' / 'open' / bucket member) to ONE token so the payload key and the +# sentinel key converge — matching server _canonical_state. limit is dropped +# from the hash (enforced at read time via the superset fallback). Same logic +# on both sides — see server_linear_tools._resolve_cache_key. OPEN_BUCKET = ('backlog', 'unstarted', 'started', 'triage') state_lc = state.lower() +def _canon_state(s): + s_lc = (s or '').strip().lower() + if s_lc == '' or s_lc == 'open' or s_lc in OPEN_BUCKET: + return 'open' + return s_lc def _key_for(state_part: str) -> str: + canon = _canon_state(state_part) filt = {k: v for k, v in sorted({ - 'state': state_part, 'label': label, 'limit': limit, + 'state': canon, 'label': label, }.items()) if v not in (None, '')} payload = json.dumps(filt, sort_keys=True, default=str).encode('utf-8') fhash = hashlib.sha256(payload).hexdigest()[:16] parts = [ (team.replace('/', '_') or '_'), (project.replace('/', '_') or '_'), - ((state_part or 'any').replace('/', '_')), + (canon.replace('/', '_') or 'any'), fhash, ] return '__'.join(parts) key = _key_for(state) -# Bucket alias keys: when state is 'open' (a tapps-mcp alias), '' (any), or -# any open-bucket member, every other open-bucket member should resolve. +# With canonicalization every open-bucket alias resolves to the same key, so +# the alias set is a singleton ({key}). We still emit the bucket variants and +# de-dup so the set matches the Python _alias_keys contract byte-for-byte. alias_keys = [] if not team or not project: key = '' diff --git a/packages/tapps-mcp/src/tapps_mcp/pipeline/platform_hook_templates.py b/packages/tapps-mcp/src/tapps_mcp/pipeline/platform_hook_templates.py index 73c40113..c0adab62 100644 --- a/packages/tapps-mcp/src/tapps_mcp/pipeline/platform_hook_templates.py +++ b/packages/tapps-mcp/src/tapps_mcp/pipeline/platform_hook_templates.py @@ -2428,27 +2428,36 @@ def _memory_auto_recall_script_cursor_ps( limit = 50 # Open-bucket alias: tapps-mcp's TTL bucket 'open' covers backlog, unstarted, # started, triage. The skill tells agents to snapshot_get(state='open') and -# then list_issues with a concrete state. Without alias support the keys -# differ and the gate self-trips (TAP-1374). Fix: derive a bucket alias and -# emit additional sentinels for it. Same logic on both sides. +# then list_issues with a concrete state. TAP-4588: canonicalize any open +# alias ('' / 'open' / bucket member) to ONE token so the payload key and the +# sentinel key converge — matching server _canonical_state. limit is dropped +# from the hash (enforced at read time via the superset fallback). Same logic +# on both sides — see server_linear_tools._resolve_cache_key. OPEN_BUCKET = ('backlog', 'unstarted', 'started', 'triage') state_lc = state.lower() +def _canon_state(s): + s_lc = (s or '').strip().lower() + if s_lc == '' or s_lc == 'open' or s_lc in OPEN_BUCKET: + return 'open' + return s_lc def _key_for(state_part: str) -> str: + canon = _canon_state(state_part) filt = {k: v for k, v in sorted({ - 'state': state_part, 'label': label, 'limit': limit, + 'state': canon, 'label': label, }.items()) if v not in (None, '')} payload = json.dumps(filt, sort_keys=True, default=str).encode('utf-8') fhash = hashlib.sha256(payload).hexdigest()[:16] parts = [ (team.replace('/', '_') or '_'), (project.replace('/', '_') or '_'), - ((state_part or 'any').replace('/', '_')), + (canon.replace('/', '_') or 'any'), fhash, ] return '__'.join(parts) key = _key_for(state) -# Bucket alias keys: when state is 'open' (a tapps-mcp alias), '' (any), or -# any open-bucket member, every other open-bucket member should resolve. +# With canonicalization every open-bucket alias resolves to the same key, so +# the alias set is a singleton ({key}). We still emit the bucket variants and +# de-dup so the set matches the Python _alias_keys contract byte-for-byte. alias_keys = [] if not team or not project: key = '' @@ -2658,15 +2667,24 @@ def _key_for(state_part: str) -> str: limit = 50 if not team or not project: sys.exit(0) +# TAP-4588: canonicalize the open-bucket alias and drop limit from the hash so +# this writer's key matches server _resolve_cache_key / the reader. +OPEN_BUCKET = ('backlog', 'unstarted', 'started', 'triage') +def _canon_state(s): + s_lc = (s or '').strip().lower() + if s_lc == '' or s_lc == 'open' or s_lc in OPEN_BUCKET: + return 'open' + return s_lc +canon = _canon_state(state) filt = {k: v for k, v in sorted({ - 'state': state, 'label': label, 'limit': limit, + 'state': canon, 'label': label, }.items()) if v not in (None, '')} payload = json.dumps(filt, sort_keys=True, default=str).encode('utf-8') fhash = hashlib.sha256(payload).hexdigest()[:16] key = '__'.join([ team.replace('/', '_') or '_', project.replace('/', '_') or '_', - (state or 'any').replace('/', '_'), + (canon.replace('/', '_') or 'any'), fhash, ]) resp = d.get('tool_response') or d.get('toolResponse') or {} @@ -2695,8 +2713,17 @@ def _find_issues(o): return r return None issues = _find_issues(resp) or [] -# TTL aligned with server-side _ttl_for_state defaults (5 min open, 1 h closed). +# TAP-4588 poisoning guard: list_issues(state='open') (a tapps-mcp alias, not a +# real Linear state) returns [] — caching that empty list under the canonical +# 'open' key would make a later get falsely report 0 issues. Skip the write +# when the raw request state was an alias/invalid AND the result is empty. +VALID_LINEAR_STATES = ( + 'backlog', 'unstarted', 'started', 'triage', 'completed', 'canceled' +) state_lc = state.lower() +if not issues and state_lc and state_lc not in VALID_LINEAR_STATES: + sys.exit(0) +# TTL aligned with server-side _ttl_for_state defaults (5 min open, 1 h closed). ttl = 3600 if state_lc in ('completed', 'canceled') else 300 now = time.time() out = { @@ -2707,6 +2734,7 @@ def _find_issues(o): 'team': team, 'project': project, 'auto_populated': True, + 'limit': limit, } root = os.environ.get('TAPPS_PROJECT_ROOT') or os.getcwd() cache_dir = os.path.join(root, '.tapps-mcp-cache', 'linear-snapshots') @@ -2787,17 +2815,25 @@ def _find_issues(o): } $key = '' if ($team -and $project) { + # TAP-4588: canonicalize the open-bucket alias and drop limit from the hash + # so the PowerShell key matches server _resolve_cache_key / the reader. + $stateLc = $state.Trim().ToLower() + $openBucket = @('backlog', 'unstarted', 'started', 'triage') + if ($stateLc -eq '' -or $stateLc -eq 'open' -or $openBucket -contains $stateLc) { + $canon = 'open' + } else { + $canon = $stateLc + } $filtObj = [ordered]@{} - if ($state) { $filtObj['state'] = $state } + if ($canon) { $filtObj['state'] = $canon } if ($label) { $filtObj['label'] = $label } - if ($limit) { $filtObj['limit'] = $limit } $payload = ($filtObj | ConvertTo-Json -Compress) $sha = [System.Security.Cryptography.SHA256]::Create() $bytes = [System.Text.Encoding]::UTF8.GetBytes($payload) $hash = [BitConverter]::ToString($sha.ComputeHash($bytes)).Replace('-', '').ToLower().Substring(0, 16) $teamPart = if ($team) { $team.Replace('/', '_') } else { '_' } $projPart = if ($project) { $project.Replace('/', '_') } else { '_' } - $statePart = if ($state) { $state.Replace('/', '_') } else { 'any' } + $statePart = if ($canon) { $canon.Replace('/', '_') } else { 'any' } $key = "${teamPart}__${projPart}__${statePart}__${hash}" } """ diff --git a/packages/tapps-mcp/src/tapps_mcp/server_linear_tools.py b/packages/tapps-mcp/src/tapps_mcp/server_linear_tools.py index 2f5fa4cd..407b8d96 100644 --- a/packages/tapps-mcp/src/tapps_mcp/server_linear_tools.py +++ b/packages/tapps-mcp/src/tapps_mcp/server_linear_tools.py @@ -116,8 +116,40 @@ def _cache_dir(project_root: Path) -> Path: return cache_dir +# Canonical token for the whole open-issue slice (TAP-4588). Any open-bucket +# alias — ""/None, the tapps-mcp TTL alias "open", and every _OPEN_STATE_BUCKETS +# member — collapses to this ONE token so the payload key converges regardless +# of which alias the caller used. Mirrors the sentinel-collapse contract +# (TAP-1374) at the payload layer. +_CANONICAL_OPEN_STATE = "open" + + +def _canonical_state(state: str | None) -> str: + """Canonicalize a Linear ``state`` for cache-key construction (TAP-4588). + + Collapses every open-bucket alias — ``""``/``None``, the tapps-mcp TTL alias + ``"open"``, and each :data:`_OPEN_STATE_BUCKETS` member — to the single token + :data:`_CANONICAL_OPEN_STATE` so a ``get`` for the open slice hits a write + made under any of those aliases. Closed buckets + (``completed``/``canceled``) and any other named state are returned + lower-cased and unchanged, keeping them isolated from the open bucket and + from each other. + """ + state_lc = (state or "").strip().lower() + if state_lc == "" or state_lc == _CANONICAL_OPEN_STATE or state_lc in _OPEN_STATE_BUCKETS: + return _CANONICAL_OPEN_STATE + return state_lc + + def _filter_hash(**kwargs: Any) -> str: - """Stable hash of filter kwargs for cache-key construction.""" + """Stable hash of filter kwargs for cache-key construction. + + ``limit`` is deliberately NOT part of the hash (TAP-4588): limit is + enforced at read time via the superset fallback in + :func:`tapps_linear_snapshot_get`, so a stored ``limit=150`` snapshot can + serve a ``limit=50`` read from the same key. Callers pass only the fields + that define the *slice identity* (``state``, ``label``). + """ normalized = {k: v for k, v in sorted(kwargs.items()) if v not in (None, "")} payload = json.dumps(normalized, sort_keys=True, default=str).encode("utf-8") return hashlib.sha256(payload).hexdigest()[:16] @@ -126,7 +158,12 @@ def _filter_hash(**kwargs: Any) -> str: def _cache_key( team: str, project: str, state: str | None, filter_hash: str ) -> str: - """Build the cache-file stem from slice identifiers.""" + """Build the cache-file stem from slice identifiers. + + ``state`` must already be canonicalized via :func:`_canonical_state` by the + caller (:func:`_resolve_cache_key`) so the filename segment matches the + hashed ``state`` field. + """ parts = [ team.replace("/", "_") or "_", project.replace("/", "_") or "_", @@ -282,9 +319,17 @@ def _cache_invalidate_prefix(cache_dir: Path, prefix: str) -> int: def _resolve_cache_key( team: str, project: str, state: str, label: str, limit: int ) -> str: - """Build the canonical cache key used by both _get and _put.""" - fhash = _filter_hash(state=state, label=label, limit=limit) - return _cache_key(team, project, state or None, fhash) + """Build the canonical cache key used by both _get and _put. + + State is canonicalized (TAP-4588) so every open-bucket alias resolves to + one key, and ``limit`` is excluded from the key entirely — it is enforced + at read time by the superset fallback in :func:`tapps_linear_snapshot_get`. + ``limit`` is still accepted for signature compatibility with the bash hooks + and the sentinel gateway, but does not affect the key. + """ + canon = _canonical_state(state) + fhash = _filter_hash(state=canon, label=label) + return _cache_key(team, project, canon, fhash) async def tapps_linear_snapshot_get( @@ -353,6 +398,38 @@ async def tapps_linear_snapshot_get( key = _resolve_cache_key(team, project, state, label, limit) cached = _cache_read(cache_dir, key) + + # TAP-4588: superset-limit + poisoning guards. The key no longer embeds + # ``limit``, so an exact-key hit may carry a snapshot stored under a + # different limit; only a stored ``limit >= requested`` can serve the read + # (a smaller stored limit is an incomplete slice and must MISS). Also + # reject an auto-populated empty payload as a false empty hit: it most + # likely came from list_issues(state="") returning []. + served_from_superset = False + if cached is not None: + stored_limit_raw = cached.get("limit") + auto_populated = bool(cached.get("auto_populated")) + issue_list: list[dict[str, Any]] = cached.get("issues", []) or [] + + if auto_populated and not issue_list: + # Poisoning guard: an empty auto-populated payload is not a + # confident hit — undo the hit bookkeeping and fall through to MISS. + _snapshot_stats["hits"] -= 1 + _snapshot_stats["misses"] += 1 + cached = None + elif stored_limit_raw is not None: + try: + stored_limit = int(stored_limit_raw) + except (TypeError, ValueError): + stored_limit = limit + if stored_limit < limit: + # Smaller stored slice cannot satisfy a larger request. + _snapshot_stats["hits"] -= 1 + _snapshot_stats["misses"] += 1 + cached = None + elif stored_limit > limit: + served_from_superset = True + elapsed_ms = (time.perf_counter_ns() - start_ns) // 1_000_000 if cached is None: @@ -372,6 +449,9 @@ async def tapps_linear_snapshot_get( now = time.time() cached_at = float(cached.get("cached_at", 0)) issues: list[dict[str, Any]] = cached.get("issues", []) + if served_from_superset: + # Truncate the broader snapshot down to what the caller asked for. + issues = issues[:limit] if projection == "compact": issues = [_compact_issue(i) for i in issues] return success_response( @@ -388,6 +468,7 @@ async def tapps_linear_snapshot_get( "team": team, "project": project, "state": state or None, + "served_from_superset": served_from_superset, }, ) @@ -491,6 +572,9 @@ async def tapps_linear_snapshot_put( "state": state or None, "team": team, "project": project, + # TAP-4588: record the stored limit so snapshot_get's superset fallback + # can decide whether this snapshot can serve a smaller-limit read. + "limit": limit, } _cache_write(cache_dir, key, payload) diff --git a/packages/tapps-mcp/tests/unit/test_linear_cache_gate.py b/packages/tapps-mcp/tests/unit/test_linear_cache_gate.py index 71033ea5..81d32d26 100644 --- a/packages/tapps-mcp/tests/unit/test_linear_cache_gate.py +++ b/packages/tapps-mcp/tests/unit/test_linear_cache_gate.py @@ -221,12 +221,14 @@ def test_post_snapshot_writes_sentinel_on_hit(self, tmp_path: Path) -> None: cwd=tmp_path, ) assert rc == 0 - # TAP-1374: state='open' is a tapps-mcp bucket alias; we now also - # write sentinels for each open-bucket member (backlog, unstarted, - # started, triage) plus the empty-state alias so concrete - # list_issues calls don't self-trip the gate. + # TAP-4588: state='open' and every open-bucket member now canonicalize + # to ONE key (was: a distinct sentinel per alias, TAP-1374). The + # cross-alias unlock contract is unchanged — it is now satisfied by + # convergence rather than by emitting N sentinels (see + # TestAliasUnlock). So a single canonical 'open' sentinel is written. sentinels = list((tmp_path / ".tapps-mcp").glob(".linear-snapshot-sentinel-*")) - assert len(sentinels) >= 5 + assert len(sentinels) == 1 + assert "__open__" in sentinels[0].name assert all(int(s.read_text().strip()) > 0 for s in sentinels) def test_post_snapshot_writes_sentinel_on_miss(self, tmp_path: Path) -> None: diff --git a/packages/tapps-mcp/tests/unit/test_linear_list_gateway.py b/packages/tapps-mcp/tests/unit/test_linear_list_gateway.py index 98ddeb5a..f866444d 100644 --- a/packages/tapps-mcp/tests/unit/test_linear_list_gateway.py +++ b/packages/tapps-mcp/tests/unit/test_linear_list_gateway.py @@ -98,14 +98,22 @@ def test_returns_false_for_negative_age(self, tmp_path: Path) -> None: class TestAliasKeys: - def test_open_state_returns_all_open_bucket_variants(self) -> None: + def test_open_state_collapses_to_single_canonical_key(self) -> None: + # TAP-4588: every open-bucket alias now canonicalizes to ONE key, so + # the alias set is a singleton (was: 5 distinct variants under TAP-1374, + # which the payload cache could not converge on). + from tapps_mcp.server_linear_tools import _resolve_cache_key + keys = _alias_keys("T", "P", "open", "", 50) - assert len(keys) >= 5 # open + backlog + unstarted + started + triage + assert len(keys) == 1 + assert keys == [_resolve_cache_key("T", "P", "open", "", 50)] + + def test_open_bucket_member_collapses_to_open(self) -> None: + """state="backlog" resolves to the same canonical 'open' key.""" + from tapps_mcp.server_linear_tools import _resolve_cache_key - def test_open_bucket_member_returns_aliases(self) -> None: - """state="backlog" should return alias keys including "open".""" keys = _alias_keys("T", "P", "backlog", "", 50) - assert len(keys) >= 2 + assert keys == [_resolve_cache_key("T", "P", "open", "", 50)] def test_non_open_state_returns_empty(self) -> None: keys = _alias_keys("T", "P", "completed", "", 50) @@ -271,14 +279,19 @@ def test_alias_hit_passes_gate(self, tmp_path: Path) -> None: result = gate_linear_list(tmp_path, "T", "P", "backlog", "", 50) assert result is None - def test_different_limit_misses_gate(self, tmp_path: Path) -> None: - """A sentinel for limit=50 does NOT satisfy a limit=100 check.""" + def test_different_limit_shares_gate(self, tmp_path: Path) -> None: + """TAP-4588: limit is no longer part of the key, so a sentinel for + limit=50 DOES satisfy a limit=100 check for the same slice. + + Limit does not change slice identity; a snapshot_get for the slice + authorizes a list at any limit (payload-side superset truncation still + enforces smaller-can't-serve-larger for the returned issues).""" from tapps_mcp.server_linear_tools import _resolve_cache_key key50 = _resolve_cache_key("T", "P", "backlog", "", 50) _write_sentinel(tmp_path, key50, age_s=0.0) result = gate_linear_list(tmp_path, "T", "P", "backlog", "", 100) - assert result is not None + assert result is None # --------------------------------------------------------------------------- diff --git a/packages/tapps-mcp/tests/unit/test_server_linear_tools.py b/packages/tapps-mcp/tests/unit/test_server_linear_tools.py index 6eed09c7..5ee29476 100644 --- a/packages/tapps-mcp/tests/unit/test_server_linear_tools.py +++ b/packages/tapps-mcp/tests/unit/test_server_linear_tools.py @@ -88,13 +88,38 @@ def test_resolve_cache_key_is_deterministic() -> None: assert a == b -def test_resolve_cache_key_differs_on_any_input() -> None: +def test_resolve_cache_key_differs_on_identity_inputs() -> None: + # team, project, and label still change the key (slice identity). base = _resolve_cache_key("T", "P", "backlog", "", 50) assert _resolve_cache_key("OTHER", "P", "backlog", "", 50) != base assert _resolve_cache_key("T", "OTHER", "backlog", "", 50) != base - assert _resolve_cache_key("T", "P", "unstarted", "", 50) != base assert _resolve_cache_key("T", "P", "backlog", "bug", 50) != base - assert _resolve_cache_key("T", "P", "backlog", "", 100) != base + + +def test_resolve_cache_key_collapses_open_aliases() -> None: + # TAP-4588: every open-bucket alias resolves to ONE canonical key so the + # payload cache self-hits regardless of which alias the caller used. + base = _resolve_cache_key("T", "P", "backlog", "", 50) + for alias in ("", "open", "unstarted", "started", "triage", "BACKLOG"): + assert _resolve_cache_key("T", "P", alias, "", 50) == base + + +def test_resolve_cache_key_ignores_limit() -> None: + # TAP-4588: limit is enforced at read time (superset fallback), not baked + # into the key — so a larger-limit write and a smaller-limit read share it. + base = _resolve_cache_key("T", "P", "backlog", "", 50) + assert _resolve_cache_key("T", "P", "backlog", "", 100) == base + assert _resolve_cache_key("T", "P", "backlog", "", 25) == base + + +def test_resolve_cache_key_isolates_closed_buckets() -> None: + # TAP-4588: completed / canceled stay distinct from open and each other. + open_key = _resolve_cache_key("T", "P", "open", "", 50) + completed = _resolve_cache_key("T", "P", "completed", "", 50) + canceled = _resolve_cache_key("T", "P", "canceled", "", 50) + assert completed != open_key + assert canceled != open_key + assert completed != canceled def test_ttl_for_state_picks_correct_bucket() -> None: @@ -566,3 +591,160 @@ async def test_get_compact_byte_budget_50_issues( assert len(issues_json) < 48_000, ( f"Compact 50-issue payload too large: {len(issues_json)} bytes (limit 48,000)" ) + + +# --------------------------------------------------------------------------- +# TAP-4588: reader/writer key convergence (canonicalization, superset limit, +# poisoning guard). These prove the payload cache actually self-hits. +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_get_open_hits_put_under_concrete_open_state( + tmp_path: Path, mock_load_settings: Any +) -> None: + """get(state='open') HITS a put made under state='' / 'started'. + + Before TAP-4588 each alias wrote a distinct file, so this missed. + """ + stored = json.dumps([{"id": "LIN-1", "title": "open work"}]) + # Writer uses the empty / concrete-open aliases the auto-populate hook and + # skills actually produce. + put_empty = await tapps_linear_snapshot_put( + team="T", project="P", issues_json=stored, state="" + ) + assert put_empty["data"]["stored"] is True + + got = await tapps_linear_snapshot_get(team="T", project="P", state="open") + assert got["data"]["cached"] is True + assert got["data"]["issues"] == [{"id": "LIN-1", "title": "open work"}] + + # And the reverse: a put under a concrete open state also serves get('open'). + put_started = await tapps_linear_snapshot_put( + team="T", project="P", issues_json=stored, state="started" + ) + assert put_started["data"]["cache_key"] == got["data"]["cache_key"] + + +@pytest.mark.asyncio +async def test_get_smaller_limit_hits_larger_cached_superset( + tmp_path: Path, mock_load_settings: Any +) -> None: + """get(limit=50) HITS a cached limit=150 snapshot, truncated.""" + issues = [{"id": f"LIN-{i}"} for i in range(150)] + put = await tapps_linear_snapshot_put( + team="T", project="P", issues_json=json.dumps(issues), state="open", limit=150 + ) + assert put["data"]["stored"] is True + + got = await tapps_linear_snapshot_get( + team="T", project="P", state="open", limit=50 + ) + assert got["data"]["cached"] is True + assert got["data"]["served_from_superset"] is True + assert len(got["data"]["issues"]) == 50 + assert got["data"]["issues"][0]["id"] == "LIN-0" + + +@pytest.mark.asyncio +async def test_get_larger_limit_misses_smaller_cached( + tmp_path: Path, mock_load_settings: Any +) -> None: + """get(limit=150) MISSES a cached limit=50 snapshot (can't serve larger).""" + issues = [{"id": f"LIN-{i}"} for i in range(50)] + await tapps_linear_snapshot_put( + team="T", project="P", issues_json=json.dumps(issues), state="open", limit=50 + ) + + got = await tapps_linear_snapshot_get( + team="T", project="P", state="open", limit=150 + ) + assert got["data"]["cached"] is False + + +@pytest.mark.asyncio +async def test_get_exact_limit_hits_not_flagged_superset( + tmp_path: Path, mock_load_settings: Any +) -> None: + """Equal stored/requested limit hits without the superset flag.""" + issues = [{"id": f"LIN-{i}"} for i in range(50)] + await tapps_linear_snapshot_put( + team="T", project="P", issues_json=json.dumps(issues), state="open", limit=50 + ) + got = await tapps_linear_snapshot_get( + team="T", project="P", state="open", limit=50 + ) + assert got["data"]["cached"] is True + assert got["data"]["served_from_superset"] is False + + +@pytest.mark.asyncio +async def test_closed_buckets_isolated_from_open( + tmp_path: Path, mock_load_settings: Any +) -> None: + """A completed-state snapshot never satisfies an open-slice get.""" + closed = json.dumps([{"id": "LIN-done", "title": "done"}]) + await tapps_linear_snapshot_put( + team="T", project="P", issues_json=closed, state="completed" + ) + # get for the open slice must MISS — different canonical key. + got_open = await tapps_linear_snapshot_get(team="T", project="P", state="open") + assert got_open["data"]["cached"] is False + # completed and canceled are isolated from each other too. + got_canceled = await tapps_linear_snapshot_get( + team="T", project="P", state="canceled" + ) + assert got_canceled["data"]["cached"] is False + # but the completed get itself hits. + got_completed = await tapps_linear_snapshot_get( + team="T", project="P", state="completed" + ) + assert got_completed["data"]["cached"] is True + + +@pytest.mark.asyncio +async def test_auto_populated_empty_payload_is_miss_not_false_hit( + tmp_path: Path, mock_load_settings: Any +) -> None: + """An auto_populated payload with issues==[] returns a miss, not 0 issues. + + Guards against poisoning: list_issues(state='open') returns [] (invalid + Linear state); the hook may cache that empty list. A later get must not + report a confident empty hit off it. + """ + cache_dir = _cache_dir(tmp_path) + key = _resolve_cache_key("T", "P", "open", "", 50) + _cache_write( + cache_dir, + key, + { + "issues": [], + "expires_at": time.time() + 600, + "cached_at": time.time() - 1, + "state": "open", + "team": "T", + "project": "P", + "auto_populated": True, + "limit": 50, + }, + ) + got = await tapps_linear_snapshot_get(team="T", project="P", state="open") + assert got["data"]["cached"] is False + + +@pytest.mark.asyncio +async def test_manual_empty_put_is_still_a_hit( + tmp_path: Path, mock_load_settings: Any +) -> None: + """A manual (non-auto) empty put is a legitimate empty hit. + + The poisoning guard only distrusts auto_populated empties — an agent that + explicitly put([]) for a genuinely-empty slice should still self-hit. + """ + put = await tapps_linear_snapshot_put( + team="T", project="P", issues_json="[]", state="completed" + ) + assert put["data"]["stored"] is True + got = await tapps_linear_snapshot_get(team="T", project="P", state="completed") + assert got["data"]["cached"] is True + assert got["data"]["issues"] == []