Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions project/ticket-131/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Ticket 131: Cache explicit GitHub API reads from repository monitors

- **ID**: ticket-131
- **Owner**: codex
- **Status**: IN_PROGRESS
- **Workflow state**: PUBLICATION
- **Created**: 2026-10-27
- **Authorization**: SESSION_EXECUTION_AUTHORIZATION — the user requested reusable caching of repeated API records across providers and investigation of missed call paths.

## Goal and scope

Reuse canonical Procache for explicit GitHub API reads from repository monitors while preserving cwd-dependent execution and zero-TTL bypass. Preserve unrelated primary integration changes.

## Acceptance criteria

- [x] AC-01: Explicit API and repository read commands reuse cache; implicit repository context and placeholders retain direct execution.
- [x] AC-02: Cache/monitor regressions and managed governance pass; failures never trigger duplicate transport calls.
- [ ] AC-03: Observe protected publication separately from local checks and runtime deployment.

Local verification: 42 scoped cache and monitor tests pass, with sixteen additional subtests, against the accepted canonical Procache source. Governance and scoped lint pass. No provider request is repeated after a cache/transport failure.

The five context regression methods also pass under unittest discovery, matching the deployed protected Monag test collector. Existing accepted Procache emits SQLite ResourceWarnings; this ticket does not hide or change that dependency behavior.
76 changes: 76 additions & 0 deletions project/ticket-131/intent.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
{
"schema": "new-project.intent/v3",
"ticket": "ticket-131",
"summary": "Cache explicit GitHub API reads from repository monitors",
"workstream": "application",
"classification": {
"kind": "SERVICE",
"priority": "P2",
"origin": "health"
},
"allowedPaths": [
"project/ticket-131/**",
"TODO.md",
"project/TICKETS.md",
"src/monag/cache.py",
"tests/test_procache_integration.py"
],
"forbiddenPaths": [
"project/ticket-*/user-*.md"
],
"stacks": [],
"dependsOn": [],
"conflictsWith": [],
"integrationTicket": null,
"delivery": {
"acceptedBaseSha": "a373f608ca484389ac820eed576043e153f7f87d",
"targetBranch": "main",
"outcome": "Reuse canonical Procache for explicit GitHub API reads from repository monitors while preserving cwd-dependent command execution and zero-TTL bypass",
"nonGoals": [
"Dependency pin changes",
"Caching writes or authorization observations",
"Changing other writers integration files"
],
"complexity": "S",
"estimatedMinutes": 30,
"budgets": {
"maxImplementationFiles": 2,
"maxAffectedComponents": 1,
"maxPublicInterfaceChanges": 0,
"maxRuntimeDependencies": 0
},
"architecture": {
"status": "accepted",
"decision": "Keep the existing canonical CachedReadCommand transport and admit only commands whose repository context is explicit",
"components": [
{
"name": "monitor-read-cache",
"paths": [
"src/monag/cache.py",
"tests/test_procache_integration.py"
]
}
],
"responsibilityChanges": false,
"interfaceChanges": [],
"dataChanges": [],
"ui": {
"impact": "none",
"states": [],
"evidence": []
},
"rollback": "Revert the exact implementation through protected delivery"
},
"runtimeDependencies": [],
"validation": [
{
"criterion": "AC-02",
"commands": [
"python3 -B -m pytest -q tests/test_procache_integration.py tests/test_monitor.py",
"./project/governance-check.sh"
],
"evidence": "Explicit API cache hits, cwd and placeholder bypass, zero-TTL and no duplicate failures"
}
]
}
}
30 changes: 25 additions & 5 deletions src/monag/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import math
import os
import re
from pathlib import Path
from typing import Sequence, Tuple, Optional

Expand All @@ -27,9 +28,6 @@ def get_cache_runner(
if not PROCACHE_AVAILABLE or os.environ.get("MONAG_DISABLE_PROCACHE") == "1":
return None

if _runner is not None and ttl is None and cache_path is None:
return _runner

path = Path(cache_path) if cache_path else _cache_path
if path is None:
cache_root = Path(os.environ.get("XDG_CACHE_HOME", Path.home() / ".cache"))
Expand All @@ -45,6 +43,13 @@ def get_cache_runner(
except (TypeError, ValueError, OverflowError):
effective_ttl = 30.0

# Zero disables caching, including a runner already initialized by an
# earlier monitor refresh. Let the caller execute directly once.
if effective_ttl == 0:
return None
if _runner is not None and ttl is None and cache_path is None:
return _runner

try:
cache = SQLiteResponseCache(path, namespace="github-user")
runner = CachedReadCommand(cache, ttl=effective_ttl)
Expand Down Expand Up @@ -80,8 +85,10 @@ def run_cached_gh(
if runner is None or not runner.is_read(args):
return None

# If cwd is set and --repo is not explicitly specified, direct execution is safer
if cwd is not None and "--repo" not in args:
# CachedReadCommand does not accept cwd. gh api endpoints are explicit,
# except placeholders that gh resolves from the working repository.
# Repository subcommands need an explicit --repo/-R value instead.
if cwd is not None and not _has_explicit_context(args):
return None

try:
Expand All @@ -95,3 +102,16 @@ def run_cached_gh(
# The provider may already have executed. Returning None would cause
# monitor.command to repeat the request and bypass cooldown/failure.
return "", f"gh cache: {type(error).__name__}"


def _has_explicit_context(args: Sequence[str]) -> bool:
if len(args) > 1 and args[1] == "api":
return not any(re.search(r"\{(?:owner|repo|branch)\}", item) for item in args[2:])
for index, item in enumerate(args):
if item in {"--repo", "-R"}:
return index + 1 < len(args) and bool(args[index + 1]) and not args[index + 1].startswith("-")
if item.startswith("--repo=") and item.removeprefix("--repo="):
return True
if item.startswith("-R") and len(item) > 2:
return True
return False
70 changes: 70 additions & 0 deletions tests/test_procache_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
import json
import os
import subprocess
import tempfile
import unittest
from pathlib import Path
from unittest import mock

import pytest
Expand Down Expand Up @@ -202,3 +205,70 @@ def test_cli_main_entrypoint_execution():
assert res.returncode == 0
assert "monag" in res.stdout



class CacheContextTests(unittest.TestCase):
"""Also collected by the protected Monag unittest discovery profile."""

def setUp(self):
reset_cache_runner()
temporary = tempfile.TemporaryDirectory()
self.addCleanup(temporary.cleanup)
self.root = Path(temporary.name)
environment = mock.patch.dict(os.environ, {
"XDG_CACHE_HOME": str(self.root), "MONAG_GITHUB_READ_TTL": "60",
"MONAG_DISABLE_PROCACHE": "0",
})
environment.start()
self.addCleanup(environment.stop)
self.addCleanup(reset_cache_runner)

def test_explicit_api_endpoint_reuses_cache_with_cwd(self):
args = ["gh", "api", "repos/semcod/example/issues/42"]
with mock.patch("subprocess.run", return_value=subprocess.CompletedProcess(args, 0, '{"id":42}', "")) as transport:
first = command(args, cwd=self.root)
second = command(args, cwd=self.root / "other")
self.assertEqual(first, ('{"id":42}', None))
self.assertEqual(second, first)
self.assertEqual(transport.call_count, 1)

def test_context_dependent_commands_bypass_cache(self):
from monag.cache import run_cached_gh
for args in (
["gh", "pr", "list"],
["gh", "api", "repos/{owner}/{repo}/issues"],
["gh", "api", "repos/semcod/example/branches/{branch}"],
["gh", "api", "graphql", "-f", 'query=query { repository(owner: "{owner}", name: "{repo}") { id } }'],
):
with self.subTest(args=args), mock.patch("subprocess.run") as transport:
self.assertIsNone(run_cached_gh(args, cwd=self.root))
transport.assert_not_called()

def test_explicit_repository_options_cache_with_cwd(self):
for repo_option in (["--repo", "semcod/example"], ["--repo=semcod/example"],
["-R", "semcod/example"], ["-Rsemcod/example"]):
args = ["gh", "pr", "list", *repo_option]
with self.subTest(option=repo_option), mock.patch(
"subprocess.run", return_value=subprocess.CompletedProcess(args, 0, "[]", ""),
) as transport:
self.assertEqual(command(args, cwd=self.root), ("[]", None))
self.assertEqual(command(args, cwd=self.root), ("[]", None))
self.assertEqual(transport.call_count, 1)

def test_zero_ttl_disables_an_initialized_runner_and_executes_once(self):
self.assertIsNotNone(get_cache_runner())
with mock.patch.dict(os.environ, {"MONAG_GITHUB_READ_TTL": "0"}):
args = ["gh", "api", "repos/semcod/example"]
self.assertIsNone(get_cache_runner())
with mock.patch("subprocess.run", return_value=subprocess.CompletedProcess(args, 0, "fresh", "")) as transport:
self.assertEqual(command(args, cwd=self.root), ("fresh", None))
self.assertEqual(transport.call_count, 1)
self.assertEqual(transport.call_args.kwargs["cwd"], self.root)

def test_cached_api_transport_failure_is_not_repeated_by_monitor(self):
args = ["gh", "api", "repos/semcod/example"]
with mock.patch("subprocess.run", side_effect=subprocess.TimeoutExpired(args, 8)) as transport:
stdout, error = command(args, cwd=self.root)
self.assertEqual(stdout, "")
self.assertIn("TimeoutExpired", error)
self.assertEqual(transport.call_count, 1)
Loading