feat(robomp): added local issue indexing and commit-search tool support
- Added `ROBOMP_ISSUE_INDEX_SYNC_SECONDS` configuration and lifecycle-managed issue indexing through startup/shutdown hooks. - Added issue/PR index tables with FTS5 triggers plus upsert and keyword/filter search helpers for indexed records. - Added GitHub backend/proxy support for `IssueIndexEntry` and issue-index page retrieval, including webhook ingestion and periodic watermark-driven sync. - Updated `gh_search_issues` to prefer local index queries when synchronized and added `search_commits` host tool with query modes and validation.
This commit is contained in:
@@ -97,6 +97,10 @@ def _baseline_env(tmp_path: Path) -> dict[str, str]:
|
||||
# cache flip `ROBOMP_NATIVES_CACHE_ENABLED=true` explicitly.
|
||||
"ROBOMP_NATIVES_CACHE_ROOT": str(tmp_path / "natives-cache"),
|
||||
"ROBOMP_NATIVES_CACHE_ENABLED": "false",
|
||||
# Same reasoning for the issue-index reconciler: its first tick would
|
||||
# spin connect-retries against the .invalid proxy URL inside server
|
||||
# tests. Tests that want it construct IssueIndexSync directly.
|
||||
"ROBOMP_ISSUE_INDEX_SYNC_SECONDS": "0",
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import json
|
||||
import subprocess
|
||||
import threading
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
@@ -14,7 +15,7 @@ from omp_rpc import HostToolContext, RpcCommandError
|
||||
|
||||
from robomp import host_tools
|
||||
from robomp.db import Database
|
||||
from robomp.github_client import GitHubClient, IssueInfo, RepoInfo
|
||||
from robomp.github_client import GitHubClient, IssueIndexEntry, IssueInfo, RepoInfo
|
||||
from robomp.host_tools import AbortController, ToolBindings, build
|
||||
from robomp.sandbox import LocalGitTransport, Workspace
|
||||
|
||||
@@ -937,6 +938,98 @@ def test_gh_search_issues_rejects_repo_qualifier_and_empty_query(db: Database, t
|
||||
_stop_loop(loop, t)
|
||||
|
||||
|
||||
def test_gh_search_issues_serves_from_local_index_once_synced(db: Database, tmp_path: Path) -> None:
|
||||
"""With a sync watermark present the tool answers from SQLite: qualifiers
|
||||
become filters, merged PRs render as `merged`, and NO GitHub call happens."""
|
||||
|
||||
def handler(_request: httpx.Request) -> httpx.Response:
|
||||
raise AssertionError("local-index search must not call GitHub")
|
||||
|
||||
bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(handler))
|
||||
db.set_issue_index_watermark("octo/widget", "2026-07-01T00:00:00Z")
|
||||
db.upsert_issue_index(
|
||||
IssueIndexEntry(
|
||||
repo="octo/widget",
|
||||
number=31,
|
||||
is_pull_request=True,
|
||||
title="fix: resize crash",
|
||||
body="handles narrow terminals",
|
||||
state="closed",
|
||||
state_reason="",
|
||||
merged_at="2026-06-02T00:00:00Z",
|
||||
author="bot",
|
||||
labels=(),
|
||||
comments=1,
|
||||
created_at="2026-06-02T00:00:00Z",
|
||||
updated_at="2026-06-02T00:00:00Z",
|
||||
html_url="https://example/pull/31",
|
||||
)
|
||||
)
|
||||
db.upsert_issue_index(
|
||||
IssueIndexEntry(
|
||||
repo="octo/widget",
|
||||
number=30,
|
||||
is_pull_request=False,
|
||||
title="resize crash report",
|
||||
body="",
|
||||
state="closed",
|
||||
state_reason="not_planned",
|
||||
merged_at="",
|
||||
author="bob",
|
||||
labels=("wontfix",),
|
||||
comments=3,
|
||||
created_at="2026-05-01T00:00:00Z",
|
||||
updated_at="2026-06-01T00:00:00Z",
|
||||
html_url="https://example/30",
|
||||
)
|
||||
)
|
||||
try:
|
||||
tool = next(x for x in build(bindings) if x.name == "gh_search_issues")
|
||||
result = tool.execute({"query": "resize crash"}, _ctx())
|
||||
pr_only = tool.execute({"query": "resize crash is:merged"}, _ctx())
|
||||
finally:
|
||||
_stop_loop(loop, t)
|
||||
assert "#30 (issue, closed (not_planned))" in result
|
||||
assert "#31 (PR, merged)" in result
|
||||
assert "#31" in pr_only and "#30" not in pr_only
|
||||
|
||||
|
||||
def _git_repo_with_commits(bindings) -> None:
|
||||
"""Turn the stub workspace repo_dir into a git repo with two commits."""
|
||||
repo = str(bindings.workspace.repo_dir)
|
||||
ident = ["-c", "user.name=t", "-c", "user.email=t@example.invalid"]
|
||||
subprocess.run(["git", "init", "-q", "-b", "main", repo], check=True)
|
||||
Path(repo, "a.txt").write_text("plain start\n", encoding="utf-8")
|
||||
subprocess.run(["git", "-C", repo, "add", "."], check=True)
|
||||
subprocess.run(["git", "-C", repo, *ident, "commit", "-q", "-m", "feat: initial import"], check=True)
|
||||
Path(repo, "a.txt").write_text("plain start\nsplitPathAndSel guard\n", encoding="utf-8")
|
||||
subprocess.run(["git", "-C", repo, "add", "."], check=True)
|
||||
subprocess.run(
|
||||
["git", "-C", repo, *ident, "commit", "-q", "-m", "fix(tools): colon selector literal paths"],
|
||||
check=True,
|
||||
)
|
||||
|
||||
|
||||
def test_search_commits_message_and_patch_modes(db: Database, tmp_path: Path) -> None:
|
||||
"""message mode greps commit messages; patch mode pickaxes diff content.
|
||||
Without an origin ref the search falls back to HEAD instead of failing."""
|
||||
bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(lambda r: httpx.Response(500)))
|
||||
_git_repo_with_commits(bindings)
|
||||
try:
|
||||
tool = next(x for x in build(bindings) if x.name == "search_commits")
|
||||
by_message = tool.execute({"query": "colon selector"}, _ctx())
|
||||
by_patch = tool.execute({"query": "splitPathAndSel", "mode": "patch"}, _ctx())
|
||||
none = tool.execute({"query": "nonexistent-topic"}, _ctx())
|
||||
with pytest.raises(RpcCommandError):
|
||||
tool.execute({"query": "x", "mode": "bogus"}, _ctx())
|
||||
finally:
|
||||
_stop_loop(loop, t)
|
||||
assert "fix(tools): colon selector literal paths" in by_message
|
||||
assert "feat: initial import" not in by_message
|
||||
assert "fix(tools): colon selector literal paths" in by_patch
|
||||
assert none.startswith("No commits")
|
||||
|
||||
|
||||
def test_classify_issue_rejects_bug_without_priority(db: Database, tmp_path: Path) -> None:
|
||||
bindings, loop, t = _bindings(db, tmp_path, httpx.MockTransport(lambda r: httpx.Response(500)))
|
||||
try:
|
||||
|
||||
@@ -0,0 +1,189 @@
|
||||
"""Local issue index: query parsing, webhook ingest, FTS search, reconcile sync."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from robomp.db import Database
|
||||
from robomp.github_client import IssueIndexEntry
|
||||
from robomp.issue_index import IssueIndexSync, ingest_webhook_payload, parse_search_query
|
||||
|
||||
|
||||
def _entry(number: int, **overrides) -> IssueIndexEntry:
|
||||
base = {
|
||||
"repo": "octo/widget",
|
||||
"number": number,
|
||||
"is_pull_request": False,
|
||||
"title": f"issue {number}",
|
||||
"body": "",
|
||||
"state": "open",
|
||||
"state_reason": "",
|
||||
"merged_at": "",
|
||||
"author": "alice",
|
||||
"labels": (),
|
||||
"comments": 0,
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"updated_at": "2026-01-01T00:00:00Z",
|
||||
"html_url": f"https://example/{number}",
|
||||
}
|
||||
base.update(overrides)
|
||||
return IssueIndexEntry(**base)
|
||||
|
||||
|
||||
# ---- parse_search_query ----
|
||||
|
||||
|
||||
def test_parse_search_query_extracts_supported_qualifiers() -> None:
|
||||
parsed = parse_search_query("colon selector is:pr is:merged label:bug author:@alice in:title")
|
||||
assert parsed.keywords == ("colon", "selector") # `in:title` dropped, not fed to FTS
|
||||
assert parsed.is_pr is True
|
||||
assert parsed.merged is True
|
||||
assert parsed.label == "bug"
|
||||
assert parsed.author == "alice"
|
||||
|
||||
|
||||
def test_parse_search_query_state_and_issue_kind() -> None:
|
||||
parsed = parse_search_query("is:issue is:closed crash")
|
||||
assert parsed.is_pr is False
|
||||
assert parsed.state == "closed"
|
||||
assert parsed.keywords == ("crash",)
|
||||
|
||||
|
||||
# ---- db index: upsert + search ----
|
||||
|
||||
|
||||
def test_search_issue_index_matches_body_text_and_ranks(db: Database) -> None:
|
||||
db.upsert_issue_index(_entry(1, title="TUI crash on resize", body="stack trace mentions overlay"))
|
||||
db.upsert_issue_index(_entry(2, title="unrelated docs typo", body="readme wording"))
|
||||
found = db.search_issue_index("octo/widget", keywords=("resize", "crash"))
|
||||
assert [e.number for e in found] == [1]
|
||||
# body-only terms also hit
|
||||
found = db.search_issue_index("octo/widget", keywords=("overlay",))
|
||||
assert [e.number for e in found] == [1]
|
||||
|
||||
|
||||
def test_search_issue_index_filters(db: Database) -> None:
|
||||
db.upsert_issue_index(
|
||||
_entry(1, title="fix crash", is_pull_request=True, merged_at="2026-02-01T00:00:00Z", state="closed")
|
||||
)
|
||||
db.upsert_issue_index(
|
||||
_entry(2, title="crash report", state="closed", state_reason="not_planned", labels=("wontfix",))
|
||||
)
|
||||
db.upsert_issue_index(_entry(3, title="crash report open", state="open"))
|
||||
|
||||
merged_prs = db.search_issue_index("octo/widget", keywords=("crash",), is_pr=True, merged=True)
|
||||
assert [e.number for e in merged_prs] == [1]
|
||||
wontfixed = db.search_issue_index("octo/widget", keywords=("crash",), label="wontfix")
|
||||
assert [e.number for e in wontfixed] == [2]
|
||||
open_only = db.search_issue_index("octo/widget", keywords=("crash",), state="open")
|
||||
assert [e.number for e in open_only] == [3]
|
||||
|
||||
|
||||
def test_upsert_refreshes_fts_so_stale_text_stops_matching(db: Database) -> None:
|
||||
"""The UPDATE trigger must swap FTS content, not accumulate it."""
|
||||
db.upsert_issue_index(_entry(1, title="original scrollback wipe"))
|
||||
db.upsert_issue_index(_entry(1, title="renamed: alternate screen request", state="closed"))
|
||||
assert db.search_issue_index("octo/widget", keywords=("scrollback",)) == []
|
||||
found = db.search_issue_index("octo/widget", keywords=("alternate",))
|
||||
assert len(found) == 1 and found[0].state == "closed"
|
||||
|
||||
|
||||
def test_search_issue_index_quotes_fts_metacharacters(db: Database) -> None:
|
||||
"""Reporter text like `"AND (` must never raise an FTS5 syntax error."""
|
||||
db.upsert_issue_index(_entry(1, title='crash with "quoted" AND (parens)'))
|
||||
found = db.search_issue_index("octo/widget", keywords=('"quoted"', "AND", "(parens)"))
|
||||
assert [e.number for e in found] == [1]
|
||||
|
||||
|
||||
def test_issue_index_watermark_roundtrip(db: Database) -> None:
|
||||
assert db.issue_index_watermark("octo/widget") is None
|
||||
db.set_issue_index_watermark("octo/widget", "2026-07-01T00:00:00Z")
|
||||
assert db.issue_index_watermark("octo/widget") == "2026-07-01T00:00:00Z"
|
||||
db.set_issue_index_watermark("octo/widget", "2026-07-02T00:00:00Z")
|
||||
assert db.issue_index_watermark("octo/widget") == "2026-07-02T00:00:00Z"
|
||||
|
||||
|
||||
# ---- webhook ingest ----
|
||||
|
||||
|
||||
def test_ingest_webhook_issue_and_pr_payloads(db: Database) -> None:
|
||||
ingested = ingest_webhook_payload(
|
||||
db,
|
||||
"octo/widget",
|
||||
"issues",
|
||||
{"issue": {"number": 5, "title": "boom", "body": "b", "state": "open", "user": {"login": "alice"}}},
|
||||
)
|
||||
assert ingested
|
||||
# PR-flavored issue payload (issue_comment on a PR) carries pull_request.merged_at.
|
||||
ingest_webhook_payload(
|
||||
db,
|
||||
"octo/widget",
|
||||
"issue_comment",
|
||||
{
|
||||
"issue": {
|
||||
"number": 6,
|
||||
"title": "fixes boom",
|
||||
"state": "closed",
|
||||
"user": {"login": "bob"},
|
||||
"pull_request": {"merged_at": "2026-03-01T00:00:00Z"},
|
||||
}
|
||||
},
|
||||
)
|
||||
# Native pull_request payload: merged_at at top level.
|
||||
ingest_webhook_payload(
|
||||
db,
|
||||
"octo/widget",
|
||||
"pull_request",
|
||||
{"pull_request": {"number": 7, "title": "another fix", "state": "closed", "merged_at": "2026-04-01T00:00:00Z"}},
|
||||
)
|
||||
assert not ingest_webhook_payload(db, "octo/widget", "push", {"ref": "refs/heads/main"})
|
||||
|
||||
boom = db.search_issue_index("octo/widget", keywords=("boom",))
|
||||
assert {e.number for e in boom} == {5, 6}
|
||||
pr6 = next(e for e in boom if e.number == 6)
|
||||
assert pr6.is_pull_request and pr6.merged_at == "2026-03-01T00:00:00Z"
|
||||
pr7 = db.search_issue_index("octo/widget", keywords=("another",))[0]
|
||||
assert pr7.is_pull_request and pr7.merged_at == "2026-04-01T00:00:00Z"
|
||||
|
||||
|
||||
# ---- reconcile sync ----
|
||||
|
||||
|
||||
class _FakeBackend:
|
||||
"""Pages of index entries keyed by page number; records `since` per call."""
|
||||
|
||||
def __init__(self, pages: dict[int, list[IssueIndexEntry]]) -> None:
|
||||
self.pages = pages
|
||||
self.calls: list[tuple[str | None, int]] = []
|
||||
|
||||
async def list_issue_index_entries(
|
||||
self, repo: str, *, since: str | None = None, page: int = 1, per_page: int = 100
|
||||
) -> list[IssueIndexEntry]:
|
||||
self.calls.append((since, page))
|
||||
return self.pages.get(page, [])
|
||||
|
||||
|
||||
class _SyncSettings:
|
||||
issue_index_sync_seconds = 900.0
|
||||
repo_allowlist = frozenset({"octo/widget"})
|
||||
|
||||
|
||||
async def test_sync_repo_backfills_pages_and_sets_watermark(db: Database, tmp_path: Path) -> None:
|
||||
full_page = [_entry(n, updated_at=f"2026-06-{n:02d}T00:00:00Z") for n in range(1, 101)]
|
||||
short_page = [_entry(101, updated_at="2026-07-01T00:00:00Z")]
|
||||
backend = _FakeBackend({1: full_page, 2: short_page})
|
||||
sync = IssueIndexSync(settings=_SyncSettings(), db=db, github=backend) # type: ignore[arg-type]
|
||||
|
||||
ingested = await sync.sync_repo("octo/widget")
|
||||
assert ingested == 101
|
||||
# First run is a backfill: no `since` on any call, pages walked in order.
|
||||
assert backend.calls == [(None, 1), (None, 2)]
|
||||
watermark = db.issue_index_watermark("octo/widget")
|
||||
assert watermark is not None
|
||||
assert db.search_issue_index("octo/widget", keywords=("issue",), limit=5)
|
||||
|
||||
# Second run is incremental: `since` derives from the stored watermark.
|
||||
backend.calls.clear()
|
||||
backend.pages = {1: []}
|
||||
await sync.sync_repo("octo/widget")
|
||||
assert backend.calls and backend.calls[0][0] is not None
|
||||
@@ -958,6 +958,42 @@ def test_webhook_incoming_pr_comment_without_directive_skips_without_counting_bu
|
||||
assert states == ["queued", "queued", "skipped"]
|
||||
|
||||
|
||||
def test_webhook_delivery_populates_issue_index(settings: Settings) -> None:
|
||||
"""Every issue-carrying delivery upserts the local search index — including
|
||||
ones the router skips (here: a conversation comment on an incoming PR)."""
|
||||
app = create_app(settings)
|
||||
with TestClient(app) as client:
|
||||
payload = {
|
||||
"action": "opened",
|
||||
"issue": {
|
||||
"number": 501,
|
||||
"title": "grep misses colon filenames",
|
||||
"body": "read tool peels the selector suffix",
|
||||
"state": "open",
|
||||
"user": {"login": "alice"},
|
||||
"author_association": "NONE",
|
||||
},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
}
|
||||
body = json.dumps(payload).encode()
|
||||
resp = client.post(
|
||||
"/webhook/github",
|
||||
content=body,
|
||||
headers=_signed_headers("test-webhook-secret", body, event="issues", delivery="idx-1"),
|
||||
)
|
||||
assert resp.status_code == 202
|
||||
|
||||
skipped = _post_pr_issue_comment(client, delivery="idx-2", user="stranger", pr_number=502)
|
||||
assert skipped.json()["state"] == "skipped"
|
||||
|
||||
db = get_database(settings.sqlite_path)
|
||||
by_body = db.search_issue_index("octo/widget", keywords=("selector", "suffix"))
|
||||
pr_row = db.search_issue_index("octo/widget", is_pr=True)
|
||||
close_database()
|
||||
assert [e.number for e in by_body] == [501]
|
||||
assert [e.number for e in pr_row] == [502]
|
||||
|
||||
|
||||
def test_webhook_contributor_gets_higher_cap(rate_limited_settings: Settings) -> None:
|
||||
app = create_app(rate_limited_settings)
|
||||
with TestClient(app) as client:
|
||||
|
||||
Reference in New Issue
Block a user