feat: API-surface trigger gating, Redis key prefix, pkg/manifestanalyzer + scan-folder/scan-repo rename

[sunib]

Three self-contained pieces split out of #220 so they can land ahead of the larger
multi-tenant work. Each was a clean cherry-pick; nothing here depends on sourceCluster,
write exclusion, GitTarget retarget, or the asserted commit author, and #220 will rebase
onto this.

feat(watch): start API-surface trigger informers only for served resources

The APIService trigger informer was created unconditionally. On an API server without an
aggregation layer (kcp, and other non-aggregating control planes) apiregistration.k8s.io
is not served at all, so client-go's reflector retried and logged forever — benign,
endlessly repeated noise, which is exactly how a real error gets missed.

Both trigger informers (CustomResourceDefinition, APIService) are now gated on discovery
reporting the resource served with list+watch, and re-evaluated after every successful
catalog refresh rather than once at startup. An aggregation layer installed later is picked
up without a restart; an absent one is logged once, not once per refresh. The decision is
extracted as the pure selectAPISurfaceTriggers, so the no-aggregation case is testable
without an API server.

feat(queue): --redis-key-prefix so reversers can share one Redis

Redis offers 16 logical databases, and --redis-db was the only separator this operator
provided. An install that runs one reverser per tenant, or per branch environment, hits
that ceiling long before it hits any real Redis limit.

Every key (watch resume cursors, attribution facts, command author records) was rooted at
the compile-time const gitops-reverser. That root is now a RedisStoreConfig field
threaded into RedisStore, AttributionIndex and CommandAuthorStore, and surfaced as
--redis-key-prefix / queue.redis.keyPrefix. It defaults to the historic value, so an
upgrade that sets nothing keeps reading the keys the previous release wrote.

The prefix is validated rather than escaped: the attribution fact-index gauge SCANs
<prefix>:author:v1:audit:*, so a glob metacharacter in the prefix would silently count the
wrong keyspace. % is rejected for the same reason against escapeKeyField. : is allowed
(a prefix like cell-a:tenant-7 nests naturally); a trailing : is normalized away.

feat(pkg): publish pkg/manifestanalyzer as the stable adoption contract

The acceptance gate that decides "may this folder become a GitTarget?" is the same one the
writer enforces before it commits a byte — which makes it the authoritative answer for an
onboarding wizard. But Go forbids importing another module's internal/ tree, so a
downstream tool could only exec the binary and parse a CLI output that carried no stability
promise, or reimplement the rules and guarantee drift from the writer.

pkg/manifestanalyzer publishes two entry points — ScanFolder (may this folder be adopted,
and if not, why) and ScanRepo (which folders in this repository could be) — over its own
JSON-tagged types carrying a SchemaVersion. It delegates every decision to
internal/manifestanalyzer, so it cannot disagree with the operator; the public shape is a
copy rather than a type alias, so the engine stays free to move.

Breaking, for manifest-analyzer --format json only

The CLI's --format json for both modes now goes through this package, which is what keeps
the two contracts identical. That drops plan from folder-scan JSON (structurally always empty
there — no cluster state, no desired resources) and adds schemaVersion. --mode analyze
and every --format text output are unchanged. Documented in
docs/UPGRADING.md.

refactor(manifest-analyzer)!: name the scan modes after the question they answer

Freezing pkg/manifestanalyzer as a supported contract is the moment to make the CLI and the
docs speak the same two nouns — folder and repo — before anyone depends on the old ones.

Before	After	Answers
--mode scan	--mode scan-folder	May this folder become a GitTarget? (ScanFolder)
--mode repo-walker	--mode scan-repo	Which folders under this repo root could? (ScanRepo)

repo-walker named an internal traversal phase, not a contract: it promised "descend
directories" where the job is really "classify this Git source for adoptable targets". A bare
scan was fine while it was the only scan, and became asymmetric the moment a repo-level scan
existed next to it.

No back-compat aliases. Both retired names now exit 2 as usage errors rather than falling
through to the default analyze mode — the only way a stale invocation fails loudly instead of
silently analyzing something else. The contract is days old; carrying deprecated spellings costs
more than it saves. Pinned by two cases in the CLI's usage-error table.

Internal follows the same nouns: WalkRepo → ScanRepo, walkRepoFS → scanRepoFS,
repowalk.go → scan_repo.go, testdata corpus repo-walker/ → scan-repo/. --mode analyze
and --mode discovery are untouched. CHANGELOG.md keeps its historical repo-walker release
note — release-please regenerates it from commit messages.

fix(chart): --redis-username quoting (pre-existing; droppable)

The last commit is not part of the split. While fixing the above I found the same defect on the
line above it: - --redis-username={{ . | quote }}. It predates this branch and is unreachable at
the chart defaults (username is empty, so with skips the arg), so it has never fired — but a user
who sets queue.redis.auth.username gets "reverser", quotes included, sent as the Redis ACL
username. It is a separate commit so it can be dropped without touching the rest.

Validation

task lint and task test pass (unit coverage 75.4%, baseline raised from 75.3% and committed).

The first push of this branch went red on the two INSTALL_MODE=helm e2e legs
(quickstart-install, image-refresh) while both config-dir legs passed. The manager was in
CrashLoopBackOff: - --redis-key-prefix={{ . | quote }} renders a YAML plain scalar, so its
double quotes are literal characters and the flag arrived as --redis-key-prefix="gitops-reverser".
ValidateKeyPrefix rejects ", parseFlagsWithArgs returned an error, and the binary exited
before it could serve anything.

Three things had to be true at once for this to escape:

• task test-e2e defaults to INSTALL_MODE=config-dir, which never renders the chart.
• helm lint and helm template in the chart job only check that the chart renders, never that
  the rendered argv is accepted.
• The value is only used when non-empty, and keyPrefix is the first arg of its kind with a
  non-empty default.

Fixed by rendering the whole argument through the printf-then-quote form the chart already uses
for --additional-sensitive-resources, and pinned by a new TestChartRendersArgsTheBinaryAccepts,
which renders the chart's Deployment and feeds the manager container's argv through the real
parseFlagsWithArgs. Verified as a genuine regression test: it fails on the pre-fix template and
passes on the fixed one. It reads the chart files in-process so go test treats them as cache
inputs — otherwise a chart-only edit would replay a cached PASS.

That test also needs helm-sync (it renders every template, including the rbac.yaml that reads
the gitignored charts/gitops-reverser/config/role.yaml), so task test gained it as a dependency
— otherwise the unit suite fails on a fresh checkout. Verified by deleting the synced artifacts and
re-running.

Both previously failing legs now pass in CI, and INSTALL_MODE=helm task test-e2e-quickstart-helm
passes locally with the manager 1/1 Running, 0 restarts, and argv --redis-key-prefix=gitops-reverser.

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Walkthrough

The change introduces public versioned manifest-analyzer APIs, routes CLI JSON output through them, adds configurable Redis key prefixes across queue storage and Helm deployment, and makes API-surface trigger informers respond dynamically to discovery catalog refreshes.

Changes

Manifest analyzer public contract

Layer / File(s)	Summary
Public scan report contracts pkg/manifestanalyzer/*	Adds versioned folder and repository report types, scan entry points, JSON writers, projections, and package stability documentation.
CLI JSON routing and exit behavior cmd/manifest-analyzer/main.go, internal/manifestanalyzer/*	Routes JSON scan modes through pkg/manifestanalyzer, preserves internal text rendering, and removes internal JSON renderers.
Contract tests, golden output, and migration documentation pkg/manifestanalyzer/*_test.go, internal/manifestanalyzer/repowalk_test.go, docs/UPGRADING.md, docs/design/gitops-api/*	Tests scan results and JSON contracts, updates golden encoding, and documents the versioned API migration.

Redis key prefix configuration

Layer / File(s)	Summary
Prefix validation and store configuration internal/queue/key_prefix.go, internal/queue/redis_store.go, cmd/main.go	Adds prefix normalization and validation, stores the resolved namespace, exposes it, and wires the CLI flag.
Namespace propagation across Redis key families internal/queue/redis_store.go, internal/queue/attribution_index.go, internal/queue/command_author_store.go	Applies the resolved prefix to cursor, attribution, scan, and command-author keys.
Deployment wiring and isolation tests charts/gitops-reverser/*, cmd/main_audit_server_test.go, internal/queue/key_prefix_test.go	Adds Helm configuration and tests validation, key formatting, default compatibility, and prefix isolation.

API-surface trigger lifecycle

Layer / File(s)	Summary
Watchability checks and trigger selection internal/watch/api_resource_catalog.go, internal/watch/manager_catalog.go, internal/watch/api_surface_triggers_test.go	Checks exact list/watch support and selects served, unserved, and already-running trigger resources.
Informer context and refresh lifecycle internal/watch/manager.go, internal/watch/manager_catalog.go, internal/watch/api_surface_triggers_test.go	Arms trigger context during startup and re-evaluates informer setup after successful catalog refreshes.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant PublicAnalyzer
  participant InternalAnalyzer
  participant JSONWriter
  CLI->>PublicAnalyzer: ScanFolder or ScanRepo
  PublicAnalyzer->>InternalAnalyzer: execute scan or repository walk
  InternalAnalyzer-->>PublicAnalyzer: internal result
  PublicAnalyzer->>JSONWriter: WriteJSON(report)
  JSONWriter-->>CLI: versioned JSON output

Loading

sequenceDiagram
  participant WatchManager
  participant ResourceCatalog
  participant TriggerSelection
  participant InformerFactory
  WatchManager->>ResourceCatalog: refresh discovery
  ResourceCatalog-->>WatchManager: trusted catalog
  WatchManager->>TriggerSelection: select API-surface triggers
  TriggerSelection-->>WatchManager: served and unserved GVRs
  WatchManager->>InformerFactory: start new trigger informers
  InformerFactory-->>WatchManager: add/update/delete events
  WatchManager->>ResourceCatalog: request catalog refresh

Loading

Possibly related PRs

• ConfigButler/gitops-reverser#209: Overlaps the Redis startup wiring and validation path in cmd/main.go.
• ConfigButler/gitops-reverser#215: Connects to the manifest-analyzer JSON routing and public repository reporting APIs.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 47.37% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The PR description is detailed, but it does not follow the required template sections like Type of Change, Testing, Checklist, Related Issues, or Screenshots.	Reformat the description to match the repository template and add the missing sections, including type of change, testing, checklist, related issues, and notes/screenshots if applicable.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly names the three main changes: API-surface gating, Redis key prefixes, and manifestanalyzer scan rename.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/multi-tenant-prereqs

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 89.35185% with 23 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/watch/manager_catalog.go	77.1%	10 Missing and 3 partials ⚠️
pkg/manifestanalyzer/folder.go	81.3%	6 Missing and 2 partials ⚠️
pkg/manifestanalyzer/repo.go	95.9%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

pkg/manifestanalyzer/repo.go (1)

123-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider initializing nil CandidatesByLayout in WriteJSON for contract consistency.

WriteJSON initializes nil Candidates to []Candidate{} but does not handle a nil Summary.CandidatesByLayout map. Since that field lacks omitempty, a nil map marshals as "candidatesByLayout": null rather than "candidatesByLayout": {}, which can break consumers expecting an object. repoReportFrom always initializes the map, so ScanRepo output is safe, but WriteJSON is a public method callable on manually constructed reports.

♻️ Proposed fix

 func (r RepoReport) WriteJSON(w io.Writer) error {
 	if r.Candidates == nil {
 		r.Candidates = []Candidate{}
 	}
+	if r.Summary.CandidatesByLayout == nil {
+		r.Summary.CandidatesByLayout = map[Layout]int{}
+	}
 	enc := json.NewEncoder(w)
 	enc.SetIndent("", "  ")
 	return enc.Encode(r)
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/manifestanalyzer/repo.go` around lines 123 - 130, Update
RepoReport.WriteJSON to initialize a nil Summary.CandidatesByLayout map to an
empty map before encoding, while preserving the existing nil Candidates
normalization. Ensure manually constructed reports serialize candidatesByLayout
as an object rather than null.

internal/queue/command_author_store.go (1)

83-83: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use s.keyPrefix directly instead of re-resolving on every key() call.

s.keyPrefix was already resolved via resolveKeyPrefix when RedisStore was constructed and passed to CommandAuthorStore. The resolveKeyPrefix call here is redundant and inconsistent with watchCursorKey in redis_store.go (line 153), which uses s.keyPrefix directly. The behavior is correct (idempotent), but this adds unnecessary string operations on every key construction.

♻️ Proposed refactor for consistency

 func (s *CommandAuthorStore) key(uid types.UID) string {
-	return resolveKeyPrefix(s.keyPrefix) + commandAuthorKeySuffix + escapeKeyField(string(uid))
+	return s.keyPrefix + commandAuthorKeySuffix + escapeKeyField(string(uid))
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/queue/command_author_store.go` at line 83, Update the
CommandAuthorStore key construction method to use s.keyPrefix directly instead
of calling resolveKeyPrefix(s.keyPrefix), matching watchCursorKey and avoiding
redundant resolution on every key generation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@charts/gitops-reverser/templates/deployment.yaml`:
- Around line 74-76: Remove the `| quote` filter from the `--redis-key-prefix`
argument within the `.Values.queue.redis.keyPrefix` block so YAML passes the raw
validated value without embedded quotes; if quoting is required, quote the
entire argument string instead. Apply the same correction to the related
`--redis-username` argument if it uses the same pattern.

---

Nitpick comments:
In `@internal/queue/command_author_store.go`:
- Line 83: Update the CommandAuthorStore key construction method to use
s.keyPrefix directly instead of calling resolveKeyPrefix(s.keyPrefix), matching
watchCursorKey and avoiding redundant resolution on every key generation.

In `@pkg/manifestanalyzer/repo.go`:
- Around line 123-130: Update RepoReport.WriteJSON to initialize a nil
Summary.CandidatesByLayout map to an empty map before encoding, while preserving
the existing nil Candidates normalization. Ensure manually constructed reports
serialize candidatesByLayout as an object rather than null.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7d58aff8-02a2-405e-8304-d99ff246dd2b

📥 Commits

Reviewing files that changed from the base of the PR and between 68a3b93 and 3acd731.

📒 Files selected for processing (26)

• .coverage-baseline
• charts/gitops-reverser/templates/deployment.yaml
• charts/gitops-reverser/values.yaml
• cmd/main.go
• cmd/main_audit_server_test.go
• cmd/manifest-analyzer/main.go
• docs/UPGRADING.md
• docs/design/gitops-api/f8-repo-discovery-and-onboarding-scan.md
• internal/manifestanalyzer/render.go
• internal/manifestanalyzer/render_test.go
• internal/manifestanalyzer/repowalk.go
• internal/manifestanalyzer/repowalk_test.go
• internal/queue/attribution_index.go
• internal/queue/command_author_store.go
• internal/queue/key_prefix.go
• internal/queue/key_prefix_test.go
• internal/queue/redis_store.go
• internal/watch/api_resource_catalog.go
• internal/watch/api_surface_triggers_test.go
• internal/watch/manager.go
• internal/watch/manager_catalog.go
• pkg/manifestanalyzer/doc.go
• pkg/manifestanalyzer/folder.go
• pkg/manifestanalyzer/folder_test.go
• pkg/manifestanalyzer/repo.go
• pkg/manifestanalyzer/repo_test.go

💤 Files with no reviewable changes (3)

• internal/manifestanalyzer/repowalk.go
• internal/manifestanalyzer/render.go
• internal/manifestanalyzer/render_test.go