Commit Graph

2 Commits

Author SHA1 Message Date
neversettle 3a93e33331
refactor: move project manager to service layer (#68)
* refactor(project): manager talks to the sqlite store; drop the in-memory store

The project Manager now runs only against the durable backend store: remove the
process-local MemoryStore (and NewMemoryManager), and require a real Store. The
daemon already wires the sqlite store; tests now build a real temp-dir sqlite
store instead of the mock.

- Move Row + the Store port to project/store.go. The Store interface stays
  because it is the dependency-inversion port that lets the manager reach the
  backend without an import cycle (storage imports project.Row), not an extra
  mock layer — there is no longer any in-memory implementation.
- NewManager requires a non-nil Store (no in-memory fallback).
- Add project/manager_test.go: List/Add/Get/Remove happy paths +
  PATH_REQUIRED/NOT_A_GIT_REPO/PATH_ALREADY_REGISTERED/ID_ALREADY_REGISTERED,
  PROJECT_NOT_FOUND/INVALID_PROJECT_ID, and UpdateConfig — all against a real
  sqlite store (the service-logic tests #47 lacked).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* refactor(project): trim routes, consolidate package, add code-first OpenAPI

- Remove POST /reload, PATCH /{id}, POST /{id}/repair routes and their
  Manager methods (Reload, UpdateConfig, Repair) and DTOs (ReloadResult,
  UpdateConfigInput) — not needed at this stage
- Merge Manager interface into manager.go; delete project.go (single-impl
  split served no purpose)
- Remove dead notImplemented helper from errors.go
- Port PR #59 code-first OpenAPI generation: controllers/dto.go named
  response types, specgen/build.go (4 routes), parity + drift tests,
  cmd/genspec, go generate wiring; regenerate openapi.yaml
- Add swaggest deps; add YAML() method to apispec.Spec

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fix(project): address PR review comments

- t.Skipf → t.Fatalf in gitRepo helper: git failures now hard-fail
  instead of silently skipping manager tests on a misconfigured runner
- FindProjectByPath: add AND archived_at IS NULL so archived paths don't
  permanently block re-registration (update queries/projects.sql and
  generated gen/projects.sql.go)
- Add TestManager_ReaddAfterRemove to lock the fix

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* fixed lint and fmt

* addressed greptile comments

* Apply suggestions from code review

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* project tests fix

* project_tests fix

* fix: Linting and formatting fix

* refactor: move project manager into service layer (#68)

* refactor: split service package by resource (#68)

* fix: ignore archived project id conflicts (#68)

* refactor: move pr manager into service layer (#68)

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: itrytoohard <ayetrytoohard@gmail.com>
2026-06-02 01:26:48 +05:30
Harshit Singh Bhandari f9b08aada4
feat(scm): GitHub provider adapter — Observe(prURL) → PRObservation (#69)
* feat(scm): GitHub provider adapter — Observe(prURL) → PRObservation

A fresh GitHub SCM provider adapter under
backend/internal/adapters/scm/github/ exposing one method:

  (*Provider).Observe(ctx, prURL) (ports.PRObservation, error)

It performs a REST GET on /repos/{o}/{r}/pulls/{n} for the authoritative
draft/merged/closed/head-SHA, one GraphQL query for the reviewDecision +
mergeStateStatus + statusCheckRollup + unresolved review threads, and
(only for failure-class CheckRuns) a REST GET on
/actions/jobs/{job_id}/logs to splice the last 20 lines of the failed
job into the observation.

The package is the observation primitive; the polling loop, cadence
selection, daemon wiring, persistence and webhook receiver are all
intentionally out of scope (separate PRs / lanes).

Closes #27 — this supersedes PR #28's attempt, which targeted types
(domain.SCMProvider / SCMSnapshot / ports.SCMObserveRequest) that the
PR #62 simplification refactor has since removed. The GraphQL queries
and mergeability composition logic are credited to @whoisasx from
PR #28's provider.go; the package was re-implemented against the
current ports.PRObservation seam (post-#62) rather than rebased.

Bot-author detection uses ONLY GitHub's typed signal (__typename
"Bot" / User.Type "Bot"). The strings.Contains(login, "bot") fallback
from PR #28 was intentionally dropped — aa-18's review flagged it as
a false-positive magnet for logins like "robothon" / "lambot123".

46 table-driven tests against httptest.NewServer cover happy path,
draft, merged, closed (not merged), CI passing/failing/pending,
StatusContext legacy, log-tail extraction (and the best-effort
log-fetch failure case), mergeability mergeable/conflicting/blocked
(including ci-failing → blocked even when GitHub still says CLEAN —
the load-bearing aa-18 contract)/unstable/unknown, review
approved/changes-requested/required/none, bot-author filtering
(including the robothon false-positive guard), unresolved-only
threads, all-bots → empty Comments, ETag-304 cache hit, primary +
secondary rate-limit (with errors.As → *RateLimitError), 401 →
ErrAuthFailed, malformed JSON → Fetched:false, network error →
Fetched:false, Authorization Bearer header injection,
StaticTokenSource blank/whitespace rejection, GHTokenSource memoize
+ invalidate.

Verification:
- go build ./...               clean
- go vet ./...                 clean
- gofmt -l backend/internal/adapters/scm/   clean
- golangci-lint run ./... (v2.12, repo .golangci.yml)   0 issues
- go test -race ./internal/adapters/scm/github/...      46/46 PASS

References:
- aa-18 review of PR #28: ~/.ao/agent-reports/aa-18.md
- aa-26 tracker adapter (sibling Go-adapter pattern): #36 / agent-reports/aa-26.md

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(scm): address greptile review on #69

Four fixes from the greptile review of PR #69:

1. CI rollup pagination (P1) — when GraphQL reports
   pageInfo.hasNextPage=true for the statusCheckRollup contexts, a
   visible "all passing" set could be hiding a failing context on the
   next page. ciSummaryFromGraphQL now degrades Passing / Pending /
   Unknown to CIUnknown in that case; a known CIFailing on the visible
   page is still safe and is NOT degraded. Also bumped the per-page
   limit from 50 to 100 (GraphQL's documented max for the contexts
   connection). Two new tests pin both branches.

2. Empty GraphQL inline fragment (P2) — dropped
   `... on User { }` from the reviewThreads author selection. The
   empty selection set was technically invalid GraphQL and a future
   API tightening could reject the query. __typename already tells us
   whether the actor is a Bot, so the fragment carried no information.

3. rest.MergeStateStatus dead-code (P2) — the field decoded from the
   non-existent REST `merge_state_status` was always empty, making the
   firstNonEmpty fallback dead code. Removed the field and switched
   the tiebreaker to rest.MergeableState (the actual REST field, upper-
   cased so the same switch covers both GraphQL and REST shapes).

4. Wrong Accept header on /actions/jobs/{id}/logs (P2) — GitHub's
   REST API validates the Accept header before issuing the 302 to the
   log blob; sending text/plain risks a 406. Switched to the canonical
   application/vnd.github+json; the redirected blob serves text/plain
   regardless.

Verification:
- go build ./...               clean
- go vet ./...                 clean
- golangci-lint run ./...      0 issues
- go test -race ./internal/adapters/scm/github/...   48 / 48 PASS

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-06-01 21:44:56 +05:30