tatara platform - deep audit report (ROUND 2)¶
Post-remediation round-2 audit (after round-1 merged 365 fixes). 34 units, 3-stage find->verify->skeptic. 269 confirmed (regressions, missed/deferred, new defects).
Severity: critical 0, high 16, medium 70, low 116, nit 67.
| repo | high | medium | low | nit | total |
|---|---|---|---|---|---|
| tatara-operator | 8 | 27 | 49 | 24 | 108 |
| tatara-memory | 1 | 13 | 20 | 15 | 49 |
| tatara-memory-repo-ingester | 3 | 8 | 15 | 7 | 33 |
| tatara-claude-code-wrapper | 1 | 6 | 11 | 4 | 22 |
| tatara-cli | 1 | 2 | 5 | 7 | 15 |
| tatara-chat | 0 | 5 | 5 | 4 | 14 |
| cross-cutting | 2 | 9 | 11 | 6 | 28 |
HIGH (16) - full detail¶
H1. [tatara-operator] closeExhaustedPR never stamps tatara-recovery-exhausted label; recovery is permanently unresettable and reopen->reclose loops¶
tatara-operator/internal/controller/projectscan.go:537-567| algorithm- problem: The doc on labelRecoveryExhausted (lines 25-28) states the label "is stamped on a bot PR after closeExhaustedPR runs so subsequent mrScan cycles skip both re-adoption AND re-close", and the close comment (lines 554-557) instructs the human to "remove the
tatara-recovery-exhaustedlabel from the PR before reopening" to reset recovery. But closeExhaustedPR only calls w.ClosePR; it NEVER calls AddLabel. Consequently: (1) the skip-guardif hasLabel(c.labels, labelRecoveryExhausted)at line 731 is dead code (the label is never written by the operator); (2) the documented human reset path is a no-op - removing a label that was never added does nothing; (3) priorTerminalAttempts only ever grows (no terminal-Task TTL/GC exists in the controller), so once a bot PR reaches maxRecoveryAttempts terminal tasks, any human reopen is detected on the next mrScan tick by priorTerminalAttempts >= maxRecoveryAttempts and the PR is auto-closed AGAIN, forever, with no way to reset. The close-then-reopen->reclose loop the round-1 fix was meant to prevent is only avoided because ListOpenPRs filters state=open (a closed PR is invisible) - the moment a human reopens, the loop resumes and cannot be broken via the advertised mechanism. - fix: In closeExhaustedPR, after a successful ClosePR, call w.AddLabel(ctx, token, issueRef, labelRecoveryExhausted) where issueRef is the provider-correct PR ref (slug + sep + c.number, '!' for GitLab MRs). This makes the line-731 skip-guard live and makes the human reset path real (removing the label lets re-adoption resume). Additionally, gate priorTerminalAttempts re-close on the label too (or stop counting once the label is present) so a human who reopens WITHOUT removing the label still gets a clean close, and one who reopens AFTER removing it gets a fresh recovery run rather than an immediate re-close driven by the still-present terminal-task history. Log AddLabel failure non-fatally with a metric.
H2. [tatara-operator] mrScan-created tasks invisible to issueScan: stale shared snapshot lets a bot PR's linked issue spawn a duplicate lifecycle Task¶
tatara-operator/internal/controller/projectscan.go:1400-1450| correctness- problem: runScans lists
existingonce (line 1400) and passes the SAME value slice to mrScan (1418) then issueScan (1443). mrScan creates issueLifecycle Tasks labelled with the LINKED issue number (dedupNumber from "Closes #N", lines 742-757) but never appends them back intoexisting. issueScan then runs isDeduped against the stale snapshot, does not see the just-created task, and creates a SECOND issueLifecycle Task for the same issue (e.g. PR#10 'Closes #42' -> mrScan makes a lifecycle task keyed on #42; issueScan scans open issue #42, finds no dedup match, creates another). Both consume the shared budget and drive redundant agent runs + SCM writes for one logical work item. The audit test TestMRScanCreatedTaskVisibleToIssueScan (projectscan_audit_test.go:60-108) claims this is fixed 'via re-list or append' but the test itself re-lists (freshExisting, line 95) before calling issueScan - production runScans does neither, so the test passes while masking the live bug (a half-applied remediation fix). - fix: Re-list tasks between activities in runScans (call r.existingScanTasks again before issueScan, as recoverOrphans already does at line 1307), or have mrScan/issueScan append each successfully-created Task into a shared *[]Task that runScans threads through. recoverOrphans already re-lists for exactly this reason; mrScan->issueScan must do the same so the second activity sees the first's creations.
H3. [tatara-operator] closeExhaustedPR closes the bot PR but never stamps the recovery-exhausted label its own contract promises¶
tatara-operator/internal/controller/projectscan.go:537-567| correctness- problem: The labelRecoveryExhausted const doc (lines 25-28) and closeExhaustedPR's own doc/close-comment (lines 537-540, 554-557) all state the
tatara-recovery-exhaustedlabel is stamped on the PR after recovery is exhausted, and that a human resets recovery by removing it. But closeExhaustedPR only calls ClosePR; it never calls AddLabel. The SCMWriter interface has AddLabel(ctx, token, issueRef, label) and closeExhaustedPR has token + repo slug + number in hand. Consequences: (1) the close comment tells the user to remove a label that was never added (dead instruction); (2) the documented reset path is impossible - terminal Task CRs are never garbage-collected by the reaper (reaper.go only deletes pods/services), so priorTerminalAttempts stays >= maxRecoveryAttempts forever; if a human reopens the PR to retry, the next mrScan sees no exhaustion label (line 731 skip never fires) and immediately re-closes it via the priorTerminalAttempts>=max branch (lines 735-738), an infinite re-close loop the human cannot escape. The label-presence skip at line 731 is effectively dead code because the label is never written. - fix: After ClosePR succeeds, stamp the label:
if lerr := w.AddLabel(ctx, token, fmt.Sprintf("%s#%d", c.repo, c.number), labelRecoveryExhausted); lerr != nil { l.Error(lerr, "mrScan: stamp recovery-exhausted label (non-fatal)", ...); r.Metrics.ScanItem("mrScan", "recovery_close_error") }. This makes line 731's skip live and the documented human reset path real.
H4. [tatara-operator] clearWritebackPending swallows its update error, so a failed clear re-posts non-idempotent SCM verbs (duplicate approve/request_changes/merge/close)¶
tatara-operator/internal/controller/writeback.go:255-272| correctness- problem: clearWritebackPending is the ONLY idempotency marker for the non-idempotent egress verbs in writeBackReview (Approve/RequestChanges at 666/670) and writeBackSelfImprove (Merge/ClosePR at 734/746): task_controller.go:130 re-enters doWriteBack on every reconcile while WritebackPending==True. clearWritebackPending returns void and only logs on failure (line 270). When RetryOnConflict exhausts on a non-conflict transient API error (or all retries are conflicts), WritebackPending stays True but the caller proceeds as if it succeeded (writeBackReview returns nil at 695, writeBackSelfImprove at 765). The next reconcile re-runs the whole verb: GitHub creates a second APPROVED review / a second request_changes review / re-attempts Merge. There is no PrURL-style persistent marker for review/merge outcomes to fall back on, so the clear failing silently is a genuine duplicate-egress (and double-merge -> ErrMergeConflict mis-classified) path.
- fix: Make clearWritebackPending return error and have writeBackReview/writeBackSelfImprove return that error (so controller-runtime requeues without treating the verb as committed) instead of swallowing it; OR record a per-verb persistent applied-marker (e.g. a status field) checked at entry. At minimum the post-verb clear in writeBackReview (line 694) and writeBackSelfImprove (line 764) must propagate the clear failure so the reconcile re-enters at a guard that recognizes the verb already happened.
H5. [tatara-operator] OpenChange 422 "a pull request already exists" is treated as a generic skip; PrURL never recovered, mis-routing finishImplement into the empty/refusal path¶
tatara-operator/internal/controller/writeback.go:156-209| correctness- problem: openChangeSkipReason (917-925) only special-cases 422 "No commits between". GitHub returns 422 with "A pull request already exists for
<head>" when a PR for the branch already exists - e.g. OpenChange succeeded on a prior reconcile but the subsequent PrURL status update (215-231) failed and exhausted retries. On the retry the OpenChange 422 "already exists" falls into the generic 4xx branch (168-172): lastSkipStatus is set, prURLs stays empty, WritebackPending is cleared, and PrURL is left "". For the lifecycle path this is serious: finishImplement re-reads the task (lifecycle.go:1096-1102), sees PrURL=="", and routes the task into the empty-implement retry / 'refused' parking branch even though a real bot PR exists - the PR is orphaned (never adopted by MRCI, no "Done - opened PR" comment) and the issue is mis-parked. - fix: Detect the "already exists" 422 (GitHub body contains "A pull request already exists"; GitLab returns the existing MR) and recover the existing PR URL via the reader (find open PR for sourceBranch->base) so PrURL is set and the issue comment + MRCI adoption proceed, rather than treating it as an empty/no-change skip.
H6. [tatara-operator] GitHub GraphQL calls use http.DefaultClient (no timeout) - regression vs scmHTTPClient¶
tatara-operator/internal/scm/github_graphql.go:35| concurrency- problem: ghGraphQL issues its POST with http.DefaultClient.Do(req), which has no client-level timeout. github.go:21-23 explicitly created scmHTTPClient (Timeout: 30s) with the comment "http.DefaultClient has no timeout and can hang indefinitely" and every REST path uses it, but the GraphQL path was left on the default client. Every Projects v2 operation goes through here: ListBoardItems (paginated, runs on the cron scan hot path), AddBoardItem, SetBoardColumn, ghProjectID, ghResourceID, ghProjectItemID. A stalled GitHub GraphQL endpoint (TCP black-hole, half-open connection) will hang the reconcile goroutine indefinitely, holding a worker slot and starving the scan loop. The request does carry ctx, so a controller-side deadline would eventually fire, but most of these call sites pass a context without a per-call deadline, so there is no backstop. This is a half-applied fix from the timeout remediation: the REST client was hardened and the GraphQL client was missed.
- fix: Replace http.DefaultClient.Do(req) with scmHTTPClient.Do(req) in ghGraphQL so the 30s timeout (and shared connection pool) applies to all GraphQL/board operations exactly as it does to the REST paths.
H7. [tatara-operator] No per-project/per-task authorization: any valid token can mutate any task or project in the namespace¶
tatara-operator/internal/restapi/handlers.go:135-170, 432-579, 588-687, 749-819| security- problem: The OIDC middleware only authenticates (verifies a Bearer token and injects Claims) but every handler performs zero authorization. Any caller holding any token valid for the operator audience can read/patch ANY Task, post ANY review verdict/PR-outcome/issue-outcome, queue comments, write handovers, and propose issues for ANY Project in the namespace. Claims carry Subject/PreferredUsername (and the Task carries Status.PodName / project/repo) but no handler ever calls auth.ClaimsFromContext to confirm the caller owns or is the agent pod for the resource it is mutating. A compromised or buggy agent for project A can drive project B's tasks to merge/close PRs or post comments.
- fix: Bind the caller to the resource: derive the expected agent identity from the Task (e.g. Status.PodName / Spec.ProjectRef) and compare against Claims.Subject/PreferredUsername in each mutating handler, rejecting with 403 on mismatch. At minimum scope every /tasks/{t}/* and /projects/{p}/* mutation to the project the token is issued for. Centralize via a helper that loads the resource, extracts ClaimsFromContext, and authorizes before any Status().Update or Create.
H8. [tatara-operator] Push receiver accepts arbitrary metric names and re-exposes them on the operator's own registry, risking name collision that breaks the whole /metrics scrape¶
tatara-operator/internal/pushmetrics/receiver.go:307-321| observability- problem: parseAndStamp accepts any wrapper-chosen metric name with no prefix/namespace guard, and Collect emits them verbatim onto ctrlmetrics.Registry (the same registry that backs the operator's own collectors, registered in main.go:90). If a wrapper pushes a name that collides with an operator-owned series (e.g. operator_reconcile_total, go_goroutines, process_cpu_seconds_total) but with different type/help/label dimensions, prometheus Gather over the shared registry returns an error and the ENTIRE operator /metrics endpoint returns 500 - a single misbehaving or hostile wrapper can blind operator monitoring. Run-id stamping prevents inter-run collisions but does NOT prevent collision with the operator's own (non-pushed) collectors.
- fix: Enforce a required prefix on pushed series (e.g. wrapper_/agent_) in parseAndStamp, dropping and counting any family whose name does not match under operator_push_series_dropped_total{reason="reserved_name"}; or expose the receiver on its own dedicated registry/handler rather than ctrlmetrics.Registry so pushed names cannot collide with operator-owned collectors.
H9. [tatara-memory] 24h forced tombstone reap resurrects memories whose upstream DeleteDocs permanently failed¶
tatara-memory/internal/memory/service.go:179-200| correctness- problem: DeleteMemory marks the tombstone BEFORE calling DeleteDocs (lines 183-188), then calls DeleteDocs (189). If DeleteDocs fails after the lightrag client exhausts its 3 internal retries, the function returns an error but the tombstone is left in place to logically mask the memory. The reaper's fast path only deletes a tombstone when TrackStatus returns 404 (reaper.go:109), i.e. when the doc is actually gone. But the reaper's ReapOlderThan/forced path (reaper.go:122, tombstone.go:69-81) blindly deletes any tombstone older than 24h WITHOUT first confirming the doc is gone, and the reaper never re-issues DeleteDocs. So a doc whose DeleteDocs permanently failed stays alive upstream, its tombstone is force-reaped at 24h, and GetMemory then returns the 'deleted' memory again. This is a silent un-delete / data-retention violation.
- fix: Before force-reaping a tombstone, the reaper must confirm the doc is actually gone (TrackStatus 404). If it still exists, re-issue DeleteDocs (or at least keep the tombstone and emit a WARN/metric) instead of dropping the tombstone. Simplest correct fix: have the forced path call TrackStatus per id and only Delete the tombstone on 404; emit a distinct 'force_skipped_still_present' metric + WARN for the rest so the resurrection is observable. Alternatively retry DeleteDocs in DeleteMemory and only mark the tombstone after a confirmed-or-already-gone upstream delete.
H10. [tatara-memory-repo-ingester] Python/JS analyzers lose cross-file call edges on incremental ingest¶
tatara-memory-repo-ingester/internal/analyze/python.go:49-95| correctness- problem: On an incremental ingest (--since), the analyzer only receives the changed files (cmd/tatara-ingest/run.go builds analyzeFiles from A/M/R diff entries only). Python builds repoIndex/importMap/moduleDefs exclusively from
parsed(the diff files), so a changed file's calls into UNCHANGED files cannot resolve via imported_name_match, global_name_match or ambiguous_multi_def. The call is recorded as a dangling_call and NO edge is emitted. The server reconciles by purging the changed file's edges (src_file scoped) and re-inserting the new, edge-less set, so the cross-file edge that the prior FULL ingest created is permanently LOST until the next full re-ingest. Go avoids this because goAnalyzer.Analyze does packages.Load(cfg, "./...") loading the whole module regardless offiles; Python and JS do not. This is the exact incremental-ingest scoping class flagged for this audit, applied to call resolution rather than helm. - fix: Build the resolution index (repoIndex, per-module defs, import maps) from the full repo working tree, not just the diff files, while still scoping EMITTED entities/edges/chunks to in-scope files (mirror the Go analyzer's load-all / emit-in-scope split). Concretely: pass or discover the full file set for the language (e.g. walk repoRoot for *.py / *.js), parse all of them to populate repoIndex, but only call processFile for files in the diff scope. Same fix required in javascript.go.
H11. [tatara-memory-repo-ingester] JavaScript analyzer loses cross-file import/call edges on incremental ingest¶
tatara-memory-repo-ingester/internal/analyze/javascript.go:49-87| correctness- problem: Same root cause as the Python case: repoIndex and moduleSet are built only from
parsed, which on an incremental ingest is just the changed files. jsImportMap and the require()-import edge emission both gate on repoIndex/moduleSet membership, and call resolution gates on repoIndex, so a changed module's imports/calls into unchanged modules resolve to nothing - the import edge and call edges are dropped and recorded as dangling. The prior full-ingest edges are then purged and not re-inserted, losing them. - fix: As with Python, populate repoIndex/moduleSet by parsing the full repo working tree of JS files, then restrict emission (processFile) to the in-scope diff files. Keep the emit-in-scope src_file contract intact.
H12. [tatara-memory-repo-ingester] SCIP-emitted SymbolRows clobber AST cross-repo symbols for the same file (server reconcile deletes per (repo,src_file) with no extractor scope)¶
tatara-memory-repo-ingester/internal/scip/scip.go:106-137,190-208| correctness- problem: scip.Parse emits provides/requires cross_repo SymbolRows. A repo can be ingested both via the regular AST walk and via --scip on the SAME logical repo name and the SAME file paths (README: --scip-repo is the repo name). The tatara-memory reconcile deletes cross_repo_symbols WHERE repo=\(1 AND src_file=\)2 with NO extractor column (pgstore.go:89, comment 'finding 7'). So whichever extractor reconciles foo.go last wipes the other extractor's symbol rows for foo.go. AST and SCIP never share symbols (different Symbol strings and EntityIDs), so the two extractors permanently fight over the cross_repo_symbols table for every shared file, silently destroying half the cross-repo resolution graph on each alternating ingest.
- fix: Either (a) do not emit cross_repo SymbolRows from the SCIP path until the table is extractor-scoped (MEMORY.md says SCIP cross-repo monikers were 'deferred to ROADMAP' - the current emission contradicts that), or (b) add an extractor column to cross_repo_symbols on the server and scope the delete/insert/ON CONFLICT by it so AST and SCIP symbols coexist. Until one is done, SCIP must not push Symbols for files the AST extractor also owns.
H13. [tatara-claude-code-wrapper] Complete() has no Dead/Booting guard; a late Stop hook from the dead claude is accepted as a real completion mid-relaunch¶
tatara-claude-code-wrapper/internal/session/session.go:639-658| correctness- problem: Complete() correlates only on mgr.current and never checks mgr.state. When claude dies mid-turn, watch() sets state=Dead (line 414) but leaves mgr.current set, then relaunch()/bootWait run for up to BootTimeout. A Stop hook from the dead process (cc-stop-hook can POST after the TUI died, or a duplicate delivery in flight) that arrives during the Dead->Booting->Ready window still has mgr.current==in-flight-id and currentSessionID=='' (no hook fired pre-crash for an in-flight turn). The SessionID guard therefore does not engage (it only fires when currentSessionID is already set AND differs). Complete() succeeds: it marks the turn Complete with the dead session's FinalText, clears current, sets state=Ready, resets restarts=0, and fires the success callback - all while a fresh claude is still booting. The subsequent resumeTurn() then no-ops (current!=id). Net effect: a turn that actually crashed is reported as a successful completion using stale output, and restarts budget is silently reset.
- fix: Reject Complete when mgr.state is Dead or Booting (the turn is mid-recovery and any hook must be from the dead process): after capturing id, add
if mgr.state == Dead || mgr.state == Booting { mgr.mu.Unlock(); mgr.log.Warn("hook during recovery; rejecting", "turn_id", id, "state", mgr.state); return fmt.Errorf("hook during recovery: session %s", mgr.state) }. The genuine post-resume completion arrives only after resumeTurn flips state back to Busy, so legitimate hooks are unaffected.
H14. [tatara-cli] MCP server under client-credentials auth never refreshes the token and 401s after expiry¶
tatara-cli/internal/cmd/mcp.go:23-38,66-131| correctness- problem: resolveMCPToken returns tokenPath="" when the bearer comes from the client_credentials grant (auth.AccessToken). For all three Clients (memory/operator/chat) the Refresh/Reload/Save funcs are only wired
if tokenPath != "", so a client-creds Client gets no Refresh func. The token struct it caches also has ExpiresAt = time.Time{} (zero), because raw.go/mcp.go build &auth.Token{AccessToken: tokStr, TokenType: "Bearer"} with no expiry. In ensureFreshLocked, time.Until(zeroTime) is hugely negative so the >30s freshness check is false, but c.refresh==nil so it returns nil and uses the token verbatim. The MCP server is long-running; Keycloak client_credentials access tokens are short-lived (commonly ~5 min). After expiry every tool call gets 401 with no recovery path, even though auth.AccessToken would happily mint a fresh cc token (it maintains its own ccTok/ccExp cache). The in-process cc refresh logic is dead for the MCP server because the Client snapshots the token string once at startup and never calls back into auth.AccessToken. - fix: For the client-credentials case wire a Refresh func that re-mints via auth.AccessToken (or set the Token's ExpiresAt to the cc expiry and provide a Refresh that calls auth.AccessToken(ctx)). Concretely: have resolveMCPToken also return the expiry, set Token.ExpiresAt, and set cfg.Refresh = func(ctx, _) (*auth.Token, error){ s, exp, err := auth.clientCredsAccessTokenWithExpiry(ctx); return &auth.Token{AccessToken:s, ExpiresAt:exp, TokenType:"Bearer"}, err } even when tokenPath=="". This makes the long-lived MCP server self-heal on cc-token expiry.
H15. [cross-cutting] cli issue_outcome sends plan; operator has no Plan field and rejects/drops it¶
tatara-operator/internal/restapi/handlers.go:525-545| correctness- problem: The cli MCP issue_outcome tool sends a
planfield (schema: 'When action=implement, a short description of WHAT will be implemented ... Posted to the issue as the implementation-start message.'). The operator's issueOutcomeReq has only Action and Comment, issueOutcome() decodes with DisallowUnknownFields, and the handler writes Status.IssueOutcome = {Action, Comment} only. So (a) any issue_outcome call carrying plan is hard-rejected with HTTP 400 unknown field "plan", and (b) even ignoring that, the operator has no code that posts the plan as an implementation-start message. The advertised implement-start-message contract is unimplemented and the wire shape mismatches. - fix: Add
Plan stringjson:"plan,omitempty"`` to issueOutcomeReq, persist it (e.g. tatarav1alpha1.IssueOutcome.Plan) and have the issueLifecycle reconcile post it to the issue as the implementation-start message, fulfilling the tool's documented behavior. Until then, every issue_outcome action=implement with a plan 400s.
H16. [cross-cutting] wrapper TATARA_CLI_VERSION pinned to ad2a2db, 2 commits behind cli main (missing comment_on_issue + MCP remediation)¶
tatara-claude-code-wrapper/Dockerfile:6-26| config-chart- problem: The wrapper pins the baked tatara-cli at git SHA ad2a2db (Makefile, Dockerfile ARG, .github/ci/build.sh). cli main is at d148d81, two commits ahead: fc97f25 'feat: add comment_on_issue MCP tool' and d148d81 'fix: deep-audit remediation (2026-06-15)'. BOTH commits modified internal/mcp/tools.go (498 insertions across the mcp package). The deployed wrapper image therefore lacks the comment_on_issue MCP tool and the 2026-06-15 MCP-layer remediation fixes. Agents running in wrapper pods cannot comment on existing issues and run pre-remediation MCP code. Note cli version.go is still 0.7.0 (unbumped by the remediation), so the SHA pin is the only authoritative drift signal.
- fix: Bump TATARA_CLI_VERSION in Dockerfile ARG, Makefile, and .github/ci/build.sh to the current cli main SHA (d148d81 or its short form), rebuild+push the wrapper image via CI, and roll it out via tatara-helmfile. Long-term: pin to the cli's published SHORT_SHA/VERSION emitted on the same main merge so the drift is mechanical to track.
MEDIUM (70)¶
tatara-operator (27)¶
- recordUsage has no stale/duplicate-callback guard, double-counts CumulativeTokens -
tatara-operator/internal/controller/turncallback.go:135-153[correctness] - Guard recordUsage the same way recordResult is guarded: pass turnID into recordUsage and, inside the RetryOnConflict closure after the Get, bail (return nil) wh - recordResult writes subtask result before the stale-turn guard; a late callback can clobber a newer turn's result -
tatara-operator/internal/controller/turncallback.go:163-179[concurrency] - Move the stale-turn / terminal check to the top of recordResult against a fresh Get of the Task, and resolve annCurrentSubtask from that fresh copy, so the subt - Triage discuss arm re-posts comment and double-counts metric when enterConversation fails after the comment lands -
tatara-operator/internal/controller/lifecycle.go:706-714[correctness] - Move r.Metrics.IssueOutcome("discuss") to AFTER enterConversation succeeds so the metric counts only a committed transition, matching the close arm which record - Triage success with no agent outcome silently defaults to implement -
tatara-operator/internal/controller/lifecycle.go:627-633[correctness] - Treat a nil IssueOutcome on a Succeeded run as inconclusive rather than implement: default to "discuss" (enter Conversation awaiting a human decision) or park w - handleMerge calls GetPRState twice per reconcile; second call's error is silently swallowed and never recorded -
tatara-operator/internal/controller/lifecycle.go:1678-1707[efficiency] - Drop the second GetPRState entirely and use the already-fetched prSt: mergedHead := prSt.HeadSHA. This removes one SCM round-trip per Merge reconcile and the in - handleMerge's primary GetPRState call is never passed to recordSCM -
tatara-operator/internal/controller/lifecycle.go:1678-1681[observability] - Add r.recordSCM(provider, "get_pr_state", pserr) immediately after the GetPRState call, before the nil check, mirroring handleMRCI line 1360. - IssueOutcome("discuss") is incremented before the transition commits, so a transient failure re-counts on the next reconcile -
tatara-operator/internal/controller/lifecycle.go:706-714[observability] - Move r.Metrics.IssueOutcome("discuss") to AFTER enterConversation returns nil (i.e. swap lines 711 and 712), so the metric reflects a committed transition like - finishTriage triggers up to 4 separate SCM round-trips (token+reader+GetIssue+ListIssueComments) that the same reconcile already fetched in buildTriagePromptFor -
tatara-operator/internal/controller/lifecycle.go:691-753[efficiency] - Fetch token + reader once in finishTriage, fetch GetIssue (for the authored marker) and ListIssueComments (for the human-reply check) once each, and pass the re - handleMainCI swallows GetCommitCIStatus errors (requeue forever) without recording an SCM metric or WARN -
tatara-operator/internal/controller/lifecycle.go:1824-1827[error-handling] - Record the failure: r.recordSCM(provider, "get_commit_ci_status", cerr) and log at WARN (l.Info/Error with action+resource_id+err) before requeuing, so repeated - ListOpenIssues fetched up to 6x per repo per scan cycle (no caching) -
tatara-operator/internal/controller/projectscan.go:804-810,962-967,987,1062,1086,1178,1258,1319[efficiency] - Fetch open issues (and open PRs) once per repo at the top of runScans into a per-cycle map[repoSlug][]IssueRef, and pass that snapshot into issueScan, recoverOr - proposalBacklog re-lists issues that buildIssuesContext already fetched -
tatara-operator/internal/controller/projectscan.go:962-967,987,1062-1067,1086,1253-1270[efficiency] - Have buildIssuesContext (or a shared lister) return the raw []IssueRef per repo and compute both the proposal-backlog count and the context string from that sin - driveTurns sets subtask+task Running with plain Status().Update after SubmitTurn; a conflict loses the just-submitted turn id and skips a subtask -
tatara-operator/internal/controller/task_controller.go:729-752[concurrency] - Persist the new turnID FIRST (call recordTurn before the Running flips), or wrap both Status().Update calls in retry.RetryOnConflict with a fresh Get (matching - Spawned agent wrapper pods get no node affinity/tolerations by default (cluster runs them on untainted control-plane nodes); chart is correctly agnostic but the deployment has no scheduling configured -
tatara-operator/internal/agent/pod.go:324-330[config-chart] - In the deploying tatara-helmfile, populate agentScheduling with a nodeSelector and/or an anti-affinity/toleration set that keeps agent wrapper pods off control- - findOpenIssueByTitle builds a malformed/wrong GitLab project path, defeating proposal title-dedup on GitLab and subgroups -
tatara-operator/internal/controller/writeback.go:584-593[correctness] - For GitLab derive the full project path via scm.GitLabProjectPath(repoURL) (already used by repoSlugFromURL) and pass it as a single project identifier, rather - writeBackSelfImprove merges then clears; a failed clear re-runs Merge on an already-merged PR and mislabels it MergeConflict -
tatara-operator/internal/controller/writeback.go:746-765[correctness] - Before treating a 405/409 as a conflict, re-check PR state (st.Merged) - if already merged, treat as success/idempotent. Or record a persistent 'merged' marker - writeBackReview 'comment' decision posts via IssueRef while approve/request_changes use the PR number; an empty IssueRef silently drops the review comment -
tatara-operator/internal/controller/writeback.go:662-695[error-handling] - Derive the comment target consistently with approve/request_changes - build the owner/repo#number ref from repo.Spec.URL + task.Spec.Source.Number (repoSlugFrom - GetPRState/GetCommitCIStatus report success when checks are still queued but a separate commit status already passed -
tatara-operator/internal/scm/github.go:394-416[correctness] - This is hard to disambiguate from the REST API alone. At minimum, document the window and rely on branch protection (required status checks) on the GitHub side; - SCM adapter has zero metrics and zero INFO business logs on every fallible/timed external call (hard rules 12,13) -
tatara-operator/internal/scm/github.go:246-308[observability] - Wrap ghDo/glDo/ghGraphQL with a metric (CounterVec by provider+method+status, HistogramVec for duration) and emit an INFO slog line for mutating actions (merge, - Bot-PR dedup is defeated: the pre-create scan and deterministic name key on the PR number while the created Task is labeled with the linked-issue number -
tatara-operator/internal/webhook/server.go:268-341[algorithm] - Compute dedupNumber once (before the pre-create scan) and use it consistently for the pre-create label query, the deterministic Task name, and the created label - A Stopped task with terminal Phase (Succeeded/Failed) is excluded from both findLifecycleTask and findReactivatableTask, so comments on it silently create a duplicate -
tatara-operator/internal/webhook/server.go:590-626[algorithm] - Make the Stopped-task handling consistent: either include Stopped in findReactivatableTask (so a Stopped+terminal-phase task is reactivated rather than orphaned - commentOnIssue uses controller-runtime logr instead of slog and omits structured fields -
tatara-operator/internal/restapi/handlers.go:333, 417-421[observability] - Capturestart := time.Now()before the writer.Comment call and replace with `s.log.InfoContext(r.Context(), "restapi: commentOnIssue", "action", "scm_issue_co - commentOnIssue SCM Comment call records no metric on a fallible network path -
tatara-operator/internal/restapi/handlers.go:410-415[observability] - Wrap the writer.Comment call to recordif s.metrics != nil { s.metrics.SCMWrite(provider, "comment", result) }with result="error" on failure and "ok" on succ - commentOnIssue discards SCM/secret/provider errors without logging, returning bare 500 -
tatara-operator/internal/restapi/handlers.go:393-397, 401-408, 412-415[error-handling] - Log each error server-side before responding, e.g. `s.log.ErrorContext(r.Context(), "restapi: commentOnIssue scm write failed", "err", err, "project", projName, - Task phase CRD enum is stale: Pending/AwaitingApproval still present after remediation removed them from the Go marker -
tatara-operator/api/v1alpha1/task_types.go:124-130[config-chart] - Runmake manifests(controller-gen) and commit the regenerated charts/tatara-operator/crds/tatara.dev_tasks.yaml so the phase enum is exactly Planning;Running - JSON logger never installed as slog default: auth/restapi/webhook emit stdlib text logs to stderr (rule 11 violation) -
tatara-operator/cmd/manager/main.go:73-74[observability] - In run(), immediately after building logger, call slog.SetDefault(logger) so package-level slog.Default() consumers (auth middleware, restapi, webhook) emit the - Auth middleware has no metric for accept/reject outcomes (rule 13 on a fallible security path) -
tatara-operator/internal/auth/middleware.go:22-43[observability] - Add an authTotal *prometheus.CounterVec{result} to OperatorMetrics and have Middleware accept a recorder (or take the metrics bundle) to increment result=accept - TokenSource.Token ignores its ctx argument; caller cancellation/deadline not honored -
tatara-operator/internal/auth/tokensource.go:53-59[concurrency] - Build the clientcredentials.Config once and store it, then in Token derive a per-call source bound to the caller ctx: oauth2.NewClient-style, e.g. ctxWithClient
tatara-memory (13)¶
- RecomputeAnalytics unconditionally clears dirty flag set by a concurrent push it never saw (lost update) -
tatara-memory/internal/codegraph/analytics_store.go:42-110[concurrency] - Capture reconciled_at when loading the snapshot, then make the clear conditional so it only fires if no newer push arrived: replace the upsert with a guarded UP - Betweenness is non-deterministic at float64 level (gonum map-iteration order); round-1 determinism claim is false, masked only by the float32 column -
tatara-memory/internal/analytics/compute.go:106-123[correctness] - Round the normalized value before returning so the float64 result is order-insensitive:betweenness[nid] = math.Round((v/norm)*1e6) / 1e6(verified to elimina - Round-1 betweenness node cap (MaxNodes/betweenness skip) is never wired to config; Brandes always runs full O(V*E) in production -
tatara-memory/internal/codegraph/analytics_store.go:52[efficiency] - Plumb a configurable MaxNodes from the service/worker config (a camelCase scalar -> ConfigMap key per hard rule 6) into RecomputeAnalytics and pass it through: - Brandes betweenness runs unbounded O(V*E) on every recompute; MaxNodes guard is dead config -
tatara-memory/internal/codegraph/analytics_store.go:52[efficiency] - Wire a real cap: add MaxNodes to AnalyticsWorkerConfig (camelCase scalar -> env), thread it into RecomputeAnalytics, and pass analytics.Config{MaxNodes: w.maxNo - code_entities.degree is written by analytics but never read (half-applied dead-column cleanup) -
tatara-memory/internal/codegraph/analytics_store.go:64-83[tech-debt] - Either drop code_entities.degree (migration, like cohesion in 0005) and stop populating it in RecomputeAnalytics, or actually read it on the betweenness ranking - ShortestPath recursive CTE enumerates all simple paths to maxDepth (worst-case exponential), no early termination at target -
tatara-memory/internal/codegraph/pgstore.go:398-411[efficiency] - Switch to a BFS-style frontier that dedups by node and stops once the target is settled: UNION over (id, depth) tracking only first-reach depth, then reconstruc - scanProps swallows corrupt-JSON errors silently despite comment promising a WARN log -
tatara-memory/internal/codegraph/pgstore.go:40-52[observability] - Actually emit the WARN: slog.Warn("codegraph: corrupt properties jsonb", "err", err) (the package already imports nothing blocking log/slog use; analytics/compu - limit query param on Related/Hyperedges/Communities/Community is cosmetic; full table is fetched then truncated in memory -
tatara-memory/internal/httpapi/codegraph.go:458-520, 582-642[efficiency] - Push the limit into the service/store so the SQL carries aLIMIT $n(and for Hyperedges, batch or limit the members fetch). Pass clampListLimit(limit) through - No per-repo/per-subject authorization: any valid OIDC token can read, write, and delete any repo's graph and any memory -
tatara-memory/internal/httpapi/router.go:55-110[security] - If multi-tenant isolation is intended, enforce repo/branch scope from the verified claims (e.g. a claim listing permitted repos, or a per-repo write check) befo - Reaper fast-path never reaps a tombstone when TrackStatus returns 200 with zero documents -
tatara-memory/internal/memory/reaper.go:106-120[algorithm] - Treat both 404 AND a 200 response whose Documents slice is empty as 'confirmed gone' and reap the tombstone in that case. Capture the TrackStatusResponse return - DeleteDocs / InsertText logical-failure Status field is ignored (HTTP 200 + failure envelope treated as success) -
tatara-memory/internal/memory/service.go:123-137,189-192[error-handling] - Check the response envelope: in DeleteDocs/InsertText (or in the service) treat resp.Status not in the success set (mirroring QueryData's LogicalError pattern) - ListEdges issues one unbounded full-subgraph read per label with no pagination or node cap -
tatara-memory/internal/memory/service.go:350-378[efficiency] - Pass a bounded max_nodes per Graph call and paginate, or add a dedicated lightrag list-edges/relations endpoint. At minimum record the deferral in MEMORY.md wit - QueryData logical failure is recorded as a successful call in metrics -
tatara-memory/internal/lightrag/http.go:288-305[observability] - After detecting the logical failure, correct the metric before returning: call c.metrics.incError(OpQueryData) (and ideally a paired decrement of the success co
tatara-memory-repo-ingester (8)¶
- Python relative imports emit a bogus empty-string requires SymbolRow -
tatara-memory-repo-ingester/internal/analyze/python.go:345-376[correctness] - Detect relative imports (modNode.Type()=="relative_import" or leading '.') and skip them in pyExternalImports (they are intra-repo, not external dependencies). - Go analyzer re-reads the whole source file once per function via sourceSlice -
tatara-memory-repo-ingester/internal/analyze/golang.go:259,553-565[efficiency] - Read each in-scope file's bytes once per file (cache by absFilePath in a map[string][]byte within processPackage / Analyze) and slice from the cached bytes usin - Terraform analyzer emits duplicate edges (no dedup) -
tatara-memory-repo-ingester/internal/analyze/terraform.go:241-323[correctness] - Add a per-file (or per-block) seen-set keyed on relation+from+to (the same appendEdgeOnce pattern used in helm.go) and dedup before appending. Apply across hand - Terraform analyzer ignores references inside nested blocks -
tatara-memory-repo-ingester/internal/analyze/terraform.go:61-75,241-251[correctness] - Recurse into body.Blocks within edgesFromBody/edgesFromExpr, collecting attribute expressions from nested blocks (recursively) and routing depends_on specially - SCIP SymbolRows use raw SCIP symbol strings that never join with AST provides/requires rows, so SCIP cross-repo links are dead -
tatara-memory-repo-ingester/internal/scip/scip.go:107-114,199-207[correctness] - Normalize the SCIP symbol moniker to the same canonical form the AST extractor uses before emitting the SymbolRow (parse the SCIP package/descriptor and reconst - enclosingDef returns the first def whose body contains the ref, not the innermost - misattributes call edges in nested funcs/closures/methods -
tatara-memory-repo-ingester/internal/scip/scip.go:324-343[algorithm] - Iterate all defs, track the candidate with the smallest containing span (endLine-startLine) among those that contain refLine, and return that innermost def. - Extraction spec node-ID format (parent_filename_entity) does not match this repo's AST entity IDs (go:func:/py:func:/js:func:), so all semantic code-node edges are orphaned -
tatara-memory-repo-ingester/internal/semantic/parse.go:60-98[correctness] - Either change the extraction spec to instruct the model to emit AST-format ids (': : ') for code nodes, or have ParseFragment translate graphify- - git quotes paths with special chars; walk passes the quoted/escaped string through verbatim, breaking contentSHA and analysis -
tatara-memory-repo-ingester/internal/walk/walk.go:49-63, 65-123, 125-149[correctness] - Pass-zto ls-files (NUL-delimited, never quoted) and usegit diff -M -z --name-status(NUL-separated fields and records), splitting on \x00 instead of \n/\
tatara-claude-code-wrapper (6)¶
- Submit() holds mgr.mu across time.Sleep(SubmitDelay), blocking health-probe Snapshot/Alive for the full delay -
tatara-claude-code-wrapper/internal/session/session.go:561-594[concurrency] - Mirror the Interject fix: capture id/w/seq under the lock, transition to a reserved/Busy state and set mgr.current before unlocking (so a concurrent Submit stil - resumeTurn sends a bare CR into the --continue session; if claude finished the turn before crashing this submits an empty turn or yields no Stop hook -
tatara-claude-code-wrapper/internal/session/session.go:501-527[algorithm] - Resume cannot be made fully idempotent from a bare CR. Detect the conversation's last-message role from the restored transcript (the tailer already parses it) b - InstallHooks uses the default slog logger and emits no metric for fallible mise/pre-commit installs -
tatara-claude-code-wrapper/internal/bootstrap/hooks.go:15-25[observability] - Pass the injected *slog.Logger and *metrics.Metrics into InstallHooks (mirroring Render's Params.Log/M pattern), log via that logger, and add a BootstrapHookIns - tatara MCP registration failure (or tatara not on PATH) silently boots an agent with no operator tools -
tatara-claude-code-wrapper/cmd/wrapper/app.go:43-47[error-handling] - Treat tatara MCP registration as mandatory: when LookPath fails or mcp-config errors, increment a result=fail counter and return the error from newApp (or at mi - cc-stop-hook never extracts stop_reason; turn record stopReason always empty -
tatara-claude-code-wrapper/cmd/cc-stop-hook/transcript.go:10-54[correctness] - AddStopReason stringjson:"stop_reason"`` to assistantLine.Message in transcript.go, capture it alongside usage when text!="" (lastStop = al.Message.StopReas - turn-complete handler blocks on synchronous commit/push, causing stop-hook timeouts and 409 retries -
tatara-claude-code-wrapper/cmd/wrapper/app.go:70-100[concurrency] - Run the commit/push (and ideally the Deliver) in a tracked background goroutine off the request path, or return 204 from turnComplete before doing the push. At
tatara-cli (2)¶
- MCP server metrics are registered and incremented but never exposed (no /metrics, push, or textfile) - hard rule 13 violated -
tatara-cli/internal/obs/obs.go:1-37[observability] - The MCP server is a long-running stdio process and can still serve HTTP on a side port. In newMCPCmd (internal/cmd/mcp.go), start a lightweight http.Server on a - code_hyperedge tool makes repo optional but the backend requires it - guaranteed 400 when repo omitted -
tatara-cli/internal/mcp/tools.go:222-224[correctness] - Make repo required to match the server and the other code-graph tools: change the schema required to ["id","repo"] and the Build to codeGet("/code-graph/hypered
tatara-chat (5)¶
- Long-poll ignores hasMore: a capped page of all-invisible (DM) messages keeps the client blocked instead of draining -
tatara-chat/internal/chat/handler.go:470-516[algorithm] - Treat hasMore as an early-return condition alongside len(msgs)>0 in both the immediate poll and the ticker branch: change the guards to `if len(msgs) > 0 || has - StoreDuration/StoreErrors only wired on 3 of 14 store methods (half-applied instrumentation) -
tatara-chat/internal/chat/store.go:307-308, 354-355, 428-429[observability] - Convert each remaining store method to a named error return and adddefer s.observe("<op>", time.Now(), &err)as the first line (e.g. create_room, list_rooms, - Auth rejection logs omit request_id / user / route structured fields -
tatara-chat/internal/auth/middleware.go:27-37[observability] - Include the request id and an action label: log.WarnContext(r.Context(), "auth: rejected", "action", "auth_reject", "request_id", httpapi.RequestIDFromContext(r - Auth 401 responses use text/plain http.Error, breaking the unified JSON error envelope -
tatara-chat/internal/auth/middleware.go:29-37[correctness] - Use the shared JSON writer: httpapi.WriteError(w, http.StatusUnauthorized, "missing bearer token") after setting WWW-Authenticate, and likewise for the invalid- - OTLP tracing is initialized and shut down but no HTTP span is ever created -
tatara-chat/cmd/tatara-chat/app.go:82-87[observability] - Add otelhttp middleware to the router so each request gets a server span, e.g. wrap the handler with otelhttp.NewHandler(router, "tatara-chat") in newApp, or ad
cross-cutting (9)¶
- Wrapper NetworkPolicy blocks Prometheus /metrics scrape when enabled, but chart ships a ServiceMonitor for it -
tatara-claude-code-wrapper/charts/tatara-claude-code-wrapper/templates/networkpolicy.yaml:16-23[observability] - Add a second ingress rule that permits the HTTP/metrics port from any namespace (matching tatara-chat's pattern: an ingress block with only `ports: [{port: http - clone retryStrategy is defeated: retry pod re-clones into existing 'src' on the shared workflow PVC and always fails -
tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-go-ci.yaml:69-99[correctness] - Make the clone idempotent so retries can succeed:rm -rf srcbefore clone, e.g. start the script withset -eu; rm -rf src. (Same one-line fix in all six clo - No activeDeadlineSeconds: a hung clone/build/deploy step runs indefinitely holding a Ceph PVC and harbor/github creds -
tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-container-build.yaml:104-158[concurrency] - SetactiveDeadlineSecondsper long-running template (e.g. 1800 for build, 600 for clone/status, 1200 for deploy) or a workflow-level default, sourced from val - No synchronization/mutex: concurrent helmfile-deploy runs for the same release can race and corrupt helm release state -
tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-helmfile-deploy.yaml:103-152[concurrency] - Addspec.synchronization: {mutex: {name: tatara-helmfile-deploy}}(or a semaphore keyed per-release via a parameter) to the deploy CWTs so only one helmfile a - Workflow secrets are conditionally rendered (if .Values.X) but unconditionally mounted; an empty value yields a successful deploy with broken pipelines -
tatara-argo-workflows/charts/tatara-argo-workflows/templates/secret-harbor-robot.yaml:1-13[config-chart] - Drop the outer{{- if }}guards so therequiredactually fail-fasts on a missing secret value, or extend validateClusterValues to require githubStatusToken, - cli change_summary sends mostProblematic; operator rejects it (DisallowUnknownFields) -
tatara-operator/internal/restapi/handlers.go:691-702[correctness] - AddMostProblematic stringjson:"mostProblematic,omitempty"`` to operator changeSummaryReq and thread it into the Task status / MR body where prBody/delivered - cli code_hyperedge marks repo optional but memory handler requires it -
tatara-cli/internal/mcp/tools.go:222-224[correctness] - Make repo required in the cli code_hyperedge tool: change Schema required to ["id","repo"] and Build to codeGet("/code-graph/hyperedge", []string{"id","repo"}, - Producer can emit duplicate idempotency keys; memory rejects whole batch and maps to uncategorized 500 -> retry storm -
tatara-memory/internal/httpapi/errmap.go:13-32[error-handling] - Two-sided: (1) producer ItemsFromChunks should de-duplicate items by IdempotencyKey before sending (identical key == identical content, safe to collapse); (2) m - Ingester has no terminal run-result counter; total-duration histogram only observed on success -
tatara-memory-repo-ingester/cmd/tatara-ingest/run.go:45-167[observability] - Add a result-labeled terminal counter (e.g. IngestRunsTotal becomes a CounterVec with label result=success|failure, or add ingest_run_result_total{result}). In
LOW (116)¶
tatara-operator (49)¶
- Reaper lists only Pods; a Service whose delete persistently fails is never retried (orphan Service leak) -
tatara-operator/internal/controller/reaper.go:119-137[correctness] - Either also List wrapper Services with WrapperPodSelector() and reap Services whose backing Pod is absent/orphaned (giving an independent retry path), or return - Scan items dropped because the shared open-task budget is exhausted are not metered -
tatara-operator/internal/controller/projectscan.go:719-773[observability] - When breaking on budget exhaustion, emit r.Metrics.ScanItem(activity, "skipped_budget") once per remaining selected item (len(selected)-created), mirroring the - selfImprove/exhausted close path re-posts the close comment on retry after a partial ClosePR failure -
tatara-operator/internal/controller/writeback.go:732-765[correctness] - Mirror the review-verb guard: treat a successful state transition as commit-point. Either split ClosePR so the comment is best-effort (log+metric, never fail th - setLifecycleState resets ImplementEmptyRetries/ImplementOutcome by keying on target string, masking a stale refusal on CI-failure re-entries -
tatara-operator/internal/controller/lifecycle.go:165-168[correctness] - Drive the reset off the caller's intent rather than the target string: pass a flag/reason (e.g. resetImplementBudget) so a CI-failure re-entry's budget handling - parkWithComment derives provider only from Source.Provider; recordSCM mislabels for board/cron-sourced tasks -
tatara-operator/internal/controller/lifecycle.go:106-130[observability] - Resolve provider with the same fallback used everywhere else: if task.Spec.Source.Provider == "" use project.Spec.Scm.Provider. parkWithComment does not receive - Conversation idle-stop ignores human replies / PendingInterjections; can stop a task a human just engaged -
tatara-operator/internal/controller/lifecycle.go:853-894[correctness] - Before stopping, re-check for human engagement: if a human comment exists since LastActivityAt (hasHumanComment) or PendingInterjections is non-empty, refresh D - Handover threshold uses integer-truncated percentage and cannot express a 0 threshold -
tatara-operator/internal/controller/lifecycle.go:1571-1578[correctness] - Document the truncation is intentional (delays handover by <1%) or round. If 0 should mean 'always handover', distinguish unset from 0 rather than treating <=0 - Internal turn-complete endpoint has no authn; any in-cluster pod can forge turn completions -
tatara-operator/internal/controller/turncallback.go:26-46[security] - Add a shared-secret HMAC over the body, or a per-task client-credential the operator mints and embeds in the callback URL it hands the wrapper, verified in hand - resolveTaskByTurn does a full-namespace List on every callback -
tatara-operator/internal/controller/turncallback.go:210-225[efficiency] - Have the wrapper include the Task name (and ideally a per-task callback credential) in the payload or callback URL path, and resolve via a direct Get(name) with - PendingInterjections/PendingComments positional [n:] trim depends on an undocumented append-only invariant -
tatara-operator/internal/controller/lifecycle.go:773-786[concurrency] - Assert/document the append-only invariant both queues rely on, or match-and-remove by value/identity of the delivered items instead of positional [n:] trimming, - Five poll handlers each repeat the ensureDeadline/deadlinePassed/parkWithComment/RecordGiveup("deadline") block verbatim -
tatara-operator/internal/controller/lifecycle.go:1410-1425[simplification] - Extract a helper, e.g. func (r *TaskReconciler) parkOnDeadline(ctx, task, writer, token, msg string) error that posts the comment, parks, and records the deadli - reconcileLifecycle's per-state dispatch repeats the same Metrics.ReconcileResult error/success wrapping seven times -
tatara-operator/internal/controller/lifecycle.go:475-531[simplification] - Either build a map[string]func(ctx, *Project, *Task)(ctrl.Result,error) and run the ReconcileResult accounting once after the lookup, or define a tiny helper th - handleConversation re-implements the nil-deadline RetryOnConflict set-once logic instead of calling a shared helper -
tatara-operator/internal/controller/lifecycle.go:853-883[simplification] - Generalize ensureDeadline to take the minutes value (or a resolver) and call it from both MRCI/Merge/MainCI (babysit minutes) and handleConversation (idle minut - handleMainCI never records a get_commit_ci_status SCM metric on success either -
tatara-operator/internal/controller/lifecycle.go:1824-1831[observability] - Wrap with r.recordSCM(provider, "get_commit_ci_status", cerr) after the call (covers both this finding and the error-swallow one). Note GetCommitCIStatus is a r - finishImplement performs four sequential full Task Gets (fresh, fresh, fresh2) where one re-read would suffice -
tatara-operator/internal/controller/lifecycle.go:1096-1244[efficiency] - Consolidate: do the HeadBranch/PRNumber/DeadlineAt write and the state transition with the object already in hand, and pass the post-transition object to resetA - setLifecycleState does two separate RetryOnConflict read-modify-write passes (status then annotation) where the boot-crash clear could ride along -
tatara-operator/internal/controller/lifecycle.go:152-201[efficiency] - Cheap guard before the second block: skip it entirely when task.Annotations has no annBootCrashAttempts key (read the in-memory task, which setLifecycleState al - healthCheck never calls SetOpenProposals, leaving the open-proposals gauge stale while it backs off on the same backlog -
tatara-operator/internal/controller/projectscan.go:1055-1075[observability] - Mirror brainstorm: call r.Metrics.SetOpenProposals(slug, float64(backlog)) for each repo queried in healthCheck so the gauge stays current regardless of which a - healthCheck never updates operator_open_proposals gauge while brainstorm does -
tatara-operator/internal/controller/projectscan.go:1055-1075[observability] - Call r.Metrics.SetOpenProposals(slug, float64(backlog)) inside the healthCheck backlog loop, matching brainstorm, so the gauge reflects the latest backlog regar - open_proposals gauge left stale for repos when brainstorm short-circuits -
tatara-operator/internal/controller/projectscan.go:916-923,948-972[observability] - Either drop the 'never stale' claim from the comment, or refresh the gauge for all repos on a path independent of the cap/in-flight/budget short-circuits (e.g. - selectPerRepo iterates repos in first-seen order, not global stale-first -
tatara-operator/internal/controller/projectscan.go:312-333[algorithm] - Order the repo iteration by each repo's stalest eligible candidate (e.g. sort order by min updatedAt per repo) so global starvation is bounded, or document expl - GitHub ListBoardItems user/org disambiguation can pick the wrong project -
tatara-operator/internal/scm/github_scan.go:139-143[correctness] - Disambiguate on project existence, not item presence: include the projectV2 id/title in the query and prefer whichever of user/organization actually returned a - recordResult writes the subtask result before the stale-turn / terminal guard, so a poll-backstop stale callback overwrites the wrong subtask's result -
tatara-operator/internal/controller/turncallback.go:163-179[correctness] - Move the stale-turn / terminal check ahead of the subtask write, or fold the whole recordResult body into one RetryOnConflict that first re-Gets the Task, bails - resolveTaskByTurn / PollOnce / ReapOrphans do full-namespace Task LISTs with O(N) linear scans on every callback and poll tick -
tatara-operator/internal/controller/turncallback.go:210-225[efficiency] - Have the wrapper POST the Task name/namespace in the turn-complete payload (it already knows TATARA_TASK + POD_NAME from BuildPod env) and Get the Task by key, - Writeback skip / no-change / no-PR business outcomes have no metric, only logs (hard rule 13) -
tatara-operator/internal/controller/writeback.go:164-209[observability] - Add a counter (e.g. r.Metrics.WritebackOutcome("no_change"|"skip_4xx"|"no_pr"|"opened")) at each terminal branch, mirroring how IssueOutcome is recorded in writ - Ingest Job duration metric never recorded for failed jobs -
tatara-operator/internal/controller/repository_controller.go:411-413[observability] - For the failure branch, fall back to time.Now() (or the Job's last failed-condition LastTransitionTime) when CompletionTime is nil: observe job.Status.StartTime - Race-loser password read-back omits the empty-password guard the primary path has -
tatara-operator/internal/controller/project_memory.go:48-56[error-handling] - Extract the read-and-validate into a small helper and use it on both paths, or inline the same check: `pw := string(existing.Data["password"]); if pw == "" { re - Provisioning requeue cadence dropped when memory health read errors -
tatara-operator/internal/controller/project_memory.go:210-223[correctness] - Either return a nil error with memoryRequeue when the read failure is genuinely transient (so the fixed 10s cadence applies and the reconcile is treated as a so - Scheduled re-ingest silently lost when annotation write fails after the dedup stamp -
tatara-operator/internal/controller/repository_controller.go:355-370[correctness] - Stamp the annotation (the trigger) first, then persist LastScheduledReingest as the dedup guard; or use a single combined write. If the annotation must lead, ma - Repository status writes lack RetryOnConflict applied elsewhere in the package -
tatara-operator/internal/controller/repository_controller.go:115-122,360-370,436-446[concurrency] - For the multi-write paths (handleFinishedJob, scheduleNextReingest), wrap the status/metadata mutations in retry.RetryOnConflict with a re-Get inside the closur - Incremental --since SHA can re-fail BackoffLimit times before fallback engages -
tatara-operator/internal/controller/repository_controller.go:286-294[algorithm] - This is an accepted tradeoff but worth flagging: consider lowering BackoffLimit to 0 for incremental jobs (a missing-SHA failure is not retryable at the pod lev - ghDoPaged can loop forever / hammer rate limits if Link rel=next is self-referential -
tatara-operator/internal/scm/github.go:225-243[efficiency] - Add a bounded iteration guard (e.g. cap at a few hundred pages, or break when raw == nextURL) and return an error when the cap is exceeded so the scan surfaces - glDoPaged can loop forever if X-Next-Page does not advance -
tatara-operator/internal/scm/gitlab.go:274-298[efficiency] - Track the previous page value and break (or error) if nextPage <= current page, and add an absolute iteration cap. Surface an error when the cap is hit instead - GitLab GetPRState returns Author="" silently when MR author is null; relies entirely on caller equality gate -
tatara-operator/internal/scm/gitlab.go:406-432[security] - In GitLab GetPRState, when head_pipeline.status is empty, fall back to GetCommitCIStatus(proj, "", mr.SHA) and fold the result, mirroring GitHub's commitCIStatu - GitLab Suggest posts notes one-by-one and aborts mid-loop on first error, leaving partial suggestions with no idempotency -
tatara-operator/internal/scm/gitlab.go:487-499[error-handling] - Either accept partial posting as best-effort (document it and have the caller not retry Suggest), or batch into a single MR discussion via the discussions API. - GitLab GetCommitCIStatus reads commit statuses without pagination - can miss a failing status past the first page -
tatara-operator/internal/scm/gitlab.go:629-658[correctness] - Use glDoPaged[struct{Status string}] with ?per_page=100 (and follow X-Next-Page) so all commit statuses are aggregated before deciding failure/pending/success. - GitLab GetPRState reads head_pipeline.status without verifying it belongs to mr.sha -
tatara-operator/internal/scm/gitlab.go:406-432[correctness] - Decode head_pipeline.sha; if it does not equal mr.sha, treat CIStatus as "pending" (the new pipeline has not reported yet) so the merge gate waits rather than a - GitHub ev.Action is passed unnormalized into the webhook metric label, causing unbounded Prometheus cardinality -
tatara-operator/internal/webhook/server.go:120-491[observability] - Normalize the GitHub action to the same closed set the GitLab path uses before it ever reaches the metric. Either map p.Action in ghWorkItemEvent to {opened,lab - reactivateTask runs two sequential Updates inside one RetryOnConflict; a non-conflict error on the second leaves Status committed but returns 500 -
tatara-operator/internal/webhook/server.go:659-691[correctness] - Split into two independent RetryOnConflict blocks (status first, then metadata), each fetching fresh, so a conflict on the metadata update does not re-run the s - readBody truncates payloads at 5 MiB with io.LimitReader, which can corrupt the HMAC verification and JSON parse silently -
tatara-operator/internal/webhook/server.go:768-771[correctness] - Read one extra byte (LimitReader(r.Body, maxBody+1)) and if len(body) > maxBody, return a distinct 413 Request Entity Too Large with a dedicated metric result ( - listRepositories/listTasks return 200 [] for a non-existent project instead of 404 -
tatara-operator/internal/restapi/handlers.go:79-109[correctness] - Get the Project by {p} first (returning writeClientErr on NotFound -> 404) before listing and filtering, matching proposeIssue/commentOnIssue. Or use a field se - commentOnIssue returns 500 for a client-side misconfiguration (Project has no SCM provider) -
tatara-operator/internal/restapi/handlers.go:389-397[error-handling] - Detect the missing-provider case explicitly: if proj.Spec.Scm == nil || provider == "" return 409/400 with a message like 'project has no scm provider configure - proposeIssue accepts an arbitrary 'kind' string with no allowlist validation -
tatara-operator/internal/restapi/handlers.go:257-294[error-handling] - Add a switch validating req.Kind against the supported issue kinds (the same set the controller/SCM layer understands), returning 400 on an unrecognized value, - proposeIssue (issue-proposal creation) records no metric -
tatara-operator/internal/restapi/handlers.go:301-314[observability] - Increment a proposal counter (existing SetOpenProposals is a gauge; add or reuse a ScanTaskCreated-style counter, e.g. s.metrics.ScanTaskCreated("propose", "imp - isDeduped and recovery-label logic hand-roll the terminal check instead of calling the new single-source TaskTerminal helper -
tatara-operator/internal/controller/projectscan.go:155-156, 1289[simplification] - Replace both hand-rolled combinations with tatarav1alpha1.TaskTerminal(t): at line 156 useif !tatarav1alpha1.TaskTerminal(t) {(dropping the local lifecycleT - ConditionApprovalApproved constant is dead code from the retired approval-label flow -
tatara-operator/api/v1alpha1/task_types.go:8-9[tech-debt] - Delete the ConditionApprovalApproved constant and its assertion in types_test.go. If a migration window is needed, mark it// Deprecated:like the ScmSpec lab - TaskSpec.ApprovalRequired is vestigial: always written false, never read for any gating decision -
tatara-operator/api/v1alpha1/task_types.go:103-104[tech-debt] - Either remove TaskSpec.ApprovalRequired (and its DTO echo) since approval is now conversation-driven, or, if it is intentionally reserved, document it as reserv - propose-issue handler does not validate Kind against the bug;improvement enum before Task create -
tatara-operator/internal/restapi/handlers.go:257-302[error-handling] - Add an explicit allow-list check after line 257: `if req.Kind != "bug" && req.Kind != "improvement" { writeError(w, http.StatusBadRequest, "kind must be bug or - Webhook/REST and callback HTTP servers are leader-election gated (default), so they do not serve on non-leader replicas / until lease acquired -
tatara-operator/cmd/manager/wire.go:109-216[correctness] - Make HandlerRunnable and callbackRunnable implement NeedLeaderElection() bool { return false } so the HTTP listeners start on every replica regardless of leader - CallbackServer graceful shutdown uses context.Background() with no timeout; a hung handler blocks shutdown forever -
tatara-operator/internal/controller/turncallback.go:431-434[error-handling] - Use a bounded shutdown context: shutdownCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second); defer cancel(); _ = srv.Shutdown(shutdownCtx)
tatara-memory (20)¶
- Compute (CPU-heavy, fallible-by-timeout path) emits no Prometheus metric for duration or betweenness-skip; only an slog line -
tatara-memory/internal/analytics/compute.go:163-170[observability] - Have Compute accept (or the caller derive from Result) a betweenness-skipped counter and a compute-duration histogram, e.g. add a counter analytics_betweenness_ - Worker acquires semaphore slot and spawns a goroutine before the single-flight check, wasting a slot on an immediate no-op -
tatara-memory/internal/codegraph/worker.go:117-147[concurrency] - Move the inflight check ahead of the sem acquisition: in processOnce, beforew.sem <- struct{}{}, take w.mu andcontinuethe loop if w.inflight[repo] is alr - TestCompute_Deterministic gives false confidence: passes on a 6-node graph while betweenness is non-deterministic on real graphs -
tatara-memory/internal/analytics/compute_test.go:78-88[tech-debt] - After applying the betweenness rounding fix, add a determinism case on a larger generated graph (e.g. 40+ nodes, several planted communities) that runs Compute - processOnce blocks on semaphore acquire without context awareness, delaying shutdown -
tatara-memory/internal/codegraph/worker.go:117-131[concurrency] - Guard the acquire: select { case w.sem <- struct{}{}: case <-ctx.Done(): return }. Drop the now-redundantrepo := repo(Go 1.25 loop-var semantics make it unn - Repo/package-scoped entities (empty FilePath) are never reclaimed by file-granular reconcile -
tatara-memory/internal/codegraph/pgstore.go:79-117[correctness] - Either require repo-scoped entities to carry a synthetic owning file (e.g. a sentinel path always present in Files), or add an extractor-scoped delete of file_p - Query/traversal store methods have no per-operation metrics; only push is metered -
tatara-memory/internal/codegraph/metrics.go:5-33[observability] - Add a code_graph_query_duration_seconds histogram and code_graph_query_total{op,result} counter, observed in Service for the traversal/path/stats methods, so sl - Bulk-ingest MaxBytesReader passed nil ResponseWriter, inconsistent with other body-limit call sites -
tatara-memory/internal/httpapi/ingest.go:57-59[correctness] - Thread the ResponseWriter through: `func readAllLimited(w http.ResponseWriter, r *http.Request) ([]byte, error) { return io.ReadAll(http.MaxBytesReader(w, r.Bod - Bulk ingest decode silently ignores unknown fields, unlike all other endpoints -
tatara-memory/internal/httpapi/ingest.go:31-45[correctness] - Decode the object form with a strict decoder (json.NewDecoder over bytes.NewReader(body) with dec.DisallowUnknownFields()) so misspelled top-level fields are re - Invalid
byvalue on /code-graph/important silently falls back to degree ranking instead of 400 -tatara-memory/internal/httpapi/codegraph.go:439-446[error-handling] - Validatebyagainst the known set (degree, betweenness) in the handler and WriteError 400 for anything else, e.g. `if by != "" && by != codegraph.ImportantByD - Read operations (GetMemory, Query, Describe, GetEntity, SearchEntities, ListEdges, edge CRUD) have no service-level metric or INFO business log -
tatara-memory/internal/memory/service.go:142-403[observability] - Add the entity/edge write ops (PatchEntity, CreateEdge, DeleteEdge) to newServiceOps label set and instrument them with incOp + InfoContext(action, resource_id, - Schema migrations are idempotent CREATE-only with no version tracking; future ALTERs cannot be applied -
tatara-memory/internal/memory/migrate.go:23-29[tech-debt] - Introduce a real migration runner (e.g. golang-migrate/goose or a hand-rolled schema_migrations table that records applied filenames in a transaction). Track ap - DeleteMemoriesBySource leaves source-index rows for already-purged tracks on mid-loop failure -
tatara-memory/internal/memory/service.go:222-256[correctness] - Either delete each source row immediately after its track's DeleteMemory succeeds (per-row consistency), or wrap the whole purge in a clearer best-effort contra - runJob has no terminal-state/dedup guard; a stale or duplicate notify re-finalizes a job and double-counts ingest_jobs_total -
tatara-memory/internal/ingest/pool.go:178-291[concurrency] - At the top of runJob, after GetJob, return early if j.Status.Terminal() (memory.JobStatus already exposes Terminal()). That single check makes runJob idempotent - Job finalization derives status from Done/Failed only, never comparing against Total, so a zero-progress drain can mark a job terminal prematurely with the wrong status -
tatara-memory/internal/ingest/pool.go:262-272[correctness] - Guard finalization on completeness: if final.Done+final.Failed < final.Total, do not finalize (leave the job 'running' so a later drain/resume completes it). On - errors_json json.Unmarshal failures are silently discarded in GetJob and IncrementJobProgress, so corrupt job error state is invisible and the capped-error append can clobber prior errors -
tatara-memory/internal/ingest/pgstore.go:75-76, 196-201[error-handling] - Check the unmarshal error: on failure in IncrementJobProgress, do not overwrite errors_json with a slice derived from a failed parse (log at WARN, increment a s - On retry of an item whose TrackID is already persisted, source indexing depends on Metadata that may not have been reloaded, and a prior sources.Add failure is never retried -
tatara-memory/internal/ingest/pool.go:319-362[correctness] - Either make SourceSink.Add idempotent and retried (e.g. a small separate reconcile that re-adds (track_id,repo,file) for items whose source row is missing), or - readyzHandler and healthzHandler are dead production code (router defines its own probes) -
tatara-memory/cmd/tatara-memory/health.go:19-43[tech-debt] - Delete healthzHandler() and readyzHandler() (and their tests), keeping only readyzFunc/readyzCheck which are actually used. If a standalone handler is desired, - readyzHandler runs db ping + lightrag health twice on the failure path -
tatara-memory/cmd/tatara-memory/health.go:30-43[efficiency] - Compute result := readyzCheck(ctx, db, lr) once, then derive ok by scanning result for non-"ok" values; branch on that single result. The router's own /readyz a - analytics.Compute has no Prometheus metric despite being a fallible/timed CPU-bound path -
tatara-memory/internal/analytics/compute.go:70-173[observability] - Return the timing and betweennessSkipped flag in Result (or take a small metrics struct) so the codegraph AnalyticsWorker records a duration histogram, a runs c - fake DeleteDocs mutates state then errors on the first missing doc, diverging from real backend -
tatara-memory/internal/lightrag/fake/fake.go:175-208[correctness] - Either validate all ids up front before mutating, or (better, to mirror the real async backend) drop the per-id 404 and treat unknown ids as no-ops returning de
tatara-memory-repo-ingester (15)¶
- JS analyzer computes jsFileDefs twice per file (regression vs the Python fix) -
tatara-memory-repo-ingester/internal/analyze/javascript.go:72-83[efficiency] - Add a defs field to the parsedFile struct, compute jsFileDefs once when parsing each file, store it, and reuse pf.defs both for repoIndex aggregation and as mod - Duplicate ES imports edges when a file imports the same relative module twice -
tatara-memory-repo-ingester/internal/analyze/javascript.go:413-426[correctness] - Reuse the existing emittedImports set (or a seen-edge key relation|from|to) for the ES relative-import edges too, skipping if the target was already emitted. - Go tree-sitter fallback emits a duplicate go:package entity per file in a broken package -
tatara-memory-repo-ingester/internal/analyze/golang_fallback.go:155-163[efficiency] - Track emitted package IDs in fallbackAnalyzeGoPackage (a seen map keyed by pkgID) and emit each go:package entity at most once, matching processPackage's emitPk - Confidence cap uses lexicographic string comparison instead of numeric -
tatara-memory-repo-ingester/internal/analyze/golang_fallback.go:359-363[correctness] - Parse confidence to float64 (strconv.ParseFloat), compare numerically against 0.45, and format back with a single canonical formatter; or have ConfidenceFor exp - JS/Python imported classes never resolve via the import map (func-only candidate ID) -
tatara-memory-repo-ingester/internal/analyze/python.go:192-210[correctness] - When matching imported names against repoIndex, accept any entity ID whose suffix matches the imported name regardless of kind (func/class), or build candidate - jsFileDefs computed twice per file (Python caches, JS does not) -
tatara-memory-repo-ingester/internal/analyze/javascript.go:70-84[efficiency] - Add a defs map[string]string field to the JS parsedFile struct, compute it once in the first loop, store it, and reuse it as moduleDefs in the second loop (mirr - helm Match claims any Chart.yaml/values.yaml basename without chart validation, swallowing non-chart files -
tatara-memory-repo-ingester/internal/analyze/helm.go:35-56[correctness] - Apply the same disk validation to the Chart.yaml/values.yaml basename branch when repoRoot is set: only claim if a Chart.yaml exists in the same directory (path - enclosingDef discards occStartLine ok flag; a malformed (<2 element) ref Range becomes refLine=0 and spuriously matches the file's first def -
tatara-memory-repo-ingester/internal/scip/scip.go:325[error-handling] - Capture the ok return and bail out (return nil,false) when the ref range is malformed: refLine, ok := occStartLine(ref.Range); if !ok { return nil, false }. - SCIP type-resolved edges get ConfidenceScore 0.98 -> tier INFERRED, under-ranking the most certain SCIP edges below AST EXTRACTED edges -
tatara-memory-repo-ingester/internal/scip/scip.go:176-188[correctness] - Set type-resolved SCIP call/reference edges to ConfidenceScore 1.0 (or set ConfidenceTier explicitly to TierExtracted), since a SCIP type resolution is as certa - stripFences uses LastIndex('}') which captures trailing prose that contains a brace -
tatara-memory-repo-ingester/internal/semantic/parse.go:185-193[correctness] - Decode with a json.Decoder from the first '{' and stop at the first complete JSON value (dec.Decode then ignore trailing), or balance braces forward from start - conceptID/slugLabel produce 'concept:
:' for empty/punctuation-only labels, collapsing distinct concepts to one id -tatara-memory-repo-ingester/internal/semantic/parse.go:141-163,66-75[correctness] - When slugLabel(label) is empty, fall back to a stable hash of the raw label (as hyperedgeID already does for empty fields) so distinct labels get distinct ids; - shaFor does a linear scan of changes.Files per loaded miss file (O(misses*files)) -
tatara-memory-repo-ingester/cmd/tatara-ingest/run.go:211-219, 263-276[efficiency] - Before the miss loop buildshaByPath := make(map[string]string, len(changes.Files))from changes.Files, thenfileSHAs[p] = shaByPath[p]. Drop shaFor. - do() does not drain the response body on the non-2xx path, defeating keep-alive exactly on the retry path -
tatara-memory-repo-ingester/internal/push/push.go:188-195, 201-205[efficiency] - After capturing the 2KB error snippet, drain the rest before returning:_, _ = io.Copy(io.Discard, resp.Body). Either add it before both return statements in - Semantic stage fans out 4 concurrent LLM calls that all retry on a fixed (jitter-free) backoff, risking a synchronized 429 retry burst -
tatara-memory-repo-ingester/internal/llm/openai.go:122-149[concurrency] - Add small randomized jitter to the fixed-backoff branch (e.g. retryDelay + rand up to retryDelay/2) so the 4 concurrent retries spread out when no Retry-After i - runSemantic reads server-returned miss paths via filepath.Join without confining to repoRoot (path-escape on a malicious/buggy server response) -
tatara-memory-repo-ingester/cmd/tatara-ingest/run.go:263-276[security] - After joining, verify the cleaned absolute path is within repoRoot: `clean := filepath.Clean(filepath.Join(o.repoRoot, p)); if !strings.HasPrefix(clean, repoRoo
tatara-claude-code-wrapper (11)¶
- resumeTurn resets currentStarted, so TurnDuration metric on a resumed turn excludes all pre-crash time -
tatara-claude-code-wrapper/internal/session/session.go:519-526[observability] - Separate the timeout anchor from the duration anchor: keep currentStarted as the original Submit time and add a distinct field (e.g. currentTimeoutStarted) that - ClaudeRestarts counter is incremented on the fatal death where no relaunch happens -
tatara-claude-code-wrapper/internal/session/session.go:412-428[observability] - Move mgr.m.ClaudeRestarts.Inc() after the budget check (just before mgr.relaunch()), or only increment on the relaunch path. The attempt counter (mgr.restarts) - Rejected/stale Stop hooks bump no metric, hiding hook-delivery problems -
tatara-claude-code-wrapper/internal/session/session.go:642-654[observability] - Increment HookReceived at the top of Complete (every delivery is a received hook) and add a labeled outcome counter (e.g. ccw_hook_received_total{result=accepte - Resume detection re-runs ls-remote, duplicating a network call checkoutTaskBranch already made -
tatara-claude-code-wrapper/internal/bootstrap/bootstrap.go:128-137, 158-168[efficiency] - Have checkoutTaskBranch return the action (or a bool resumed) it already determined, e.g.func checkoutTaskBranch(...) (resumed bool, err error), and use that - copyTree discards filepath.Rel error, which on failure collapses the whole tree onto dst root -
tatara-claude-code-wrapper/internal/bootstrap/skills.go:36-37[error-handling] - Check the error:rel, err := filepath.Rel(src, path); if err != nil { return fmt.Errorf("rel %s: %w", path, err) }. - namespacePath drops the host, so same owner/repo on different hosts collide and the second clone is silently skipped -
tatara-claude-code-wrapper/internal/bootstrap/namespace.go:39-52[correctness] - Either include a host-derived prefix in the namespace path, or detect duplicate dest dirs across Repos in Render and fail loudly when two distinct URLs resolve - gitRunner error wrap mixes %v and %w; combined git output is interpolated into the error string -
tatara-claude-code-wrapper/cmd/wrapper/app.go:212-216[error-handling] - Keep wrapping the underlying err with %w but scrub or omit rawoutfrom the error (or redact any token-shaped substrings) before surfacing it, since this erro - Webhook deliver sleeps a full backoff after the last failed attempt before giving up -
tatara-claude-code-wrapper/internal/webhook/webhook.go:100-124[efficiency] - Skip the sleep on the last attempt:if attempt == s.cfg.Retries { break }(or guard the select) beforecase <-time.After(backoff), so the dropped outcome is - Webhook marshal failure logs error but records no delivery outcome metric -
tatara-claude-code-wrapper/internal/webhook/webhook.go:67-72[observability] - Increment s.m.WebhookDelivery.WithLabelValues("dropped").Inc() before returning on the marshal-error path so the metric counts all non-delivered callbacks. - Metrics pusher has no success/failure counter for its own push attempts -
tatara-claude-code-wrapper/internal/pushclient/pushclient.go:109-143[observability] - Add a CounterVec (e.g. ccw_metric_push_total with label result=ok|encode_fail|transport_fail|rejected) incremented on each pushOnce outcome; the operator alread - Tailer accumulates partial lines with no size bound; a never-terminated line grows memory unbounded -
tatara-claude-code-wrapper/internal/transcript/tailer.go:114-160[efficiency] - Add a max partial-line size (e.g. 16MiB to match the stop-hook scanner); when exceeded, emit a raw/truncated event and reset partial so a pathological line cann
tatara-cli (5)¶
- patch_entity declares patch required but Build only validates id; a missing patch sends a nil PATCH body -
tatara-cli/internal/mcp/tools.go:132-142[error-handling] - Validate patch before building the request, e.g. addif _, ok := a["patch"]; !ok { return "", "", nil, fmt.Errorf("patch required") }(and reject a non-object - 204 No Content responses render as an empty tool-result text - no success confirmation to the agent -
tatara-cli/internal/mcp/server.go:87-93[observability] - When body is empty (len(body)==0), return a small confirmation result instead of empty text, e.g. NewToolResultText({"ok":true}) or a `{"status":"no_content"} - raw command builds a client-creds token with zero ExpiresAt and no refresh -
tatara-cli/internal/cmd/raw.go:57-101[correctness] - Have auth expose the cc expiry (it already computes exp in ClientCredentialsToken) and set token.ExpiresAt so ensureFreshLocked's freshness math is meaningful; - ClientCredentialsToken returns an empty access token when token endpoint omits access_token -
tatara-cli/internal/auth/clientcreds.go:53-60[error-handling] - After decode addif tr.AccessToken == "" { return "", time.Time{}, errors.New("token response: empty access_token") }mirroring the discovery endpoint's no-to - RefreshToken accepts a 200 response with empty access_token, persisting a broken token -
tatara-cli/internal/auth/refresh.go:50-67[error-handling] - Validate after unmarshal:if tr.AccessToken == "" { return nil, fmt.Errorf("auth: refresh response missing access_token") }. Also preserve the old refresh tok
tatara-chat (5)¶
- Handler does not guard nil *Metrics while Store does, an inconsistent nil-safety contract that nil-derefs -
tatara-chat/internal/chat/handler.go:172, 242, 290, 334, 403, 436, 476-513[correctness] - Either require non-nil Metrics in NewHandler (panic/return error if nil, and document it), or guard each h.metrics.* site as the store does. Given the store alr - AddParticipant enrolls into archived rooms (no status filter, plus GetRoom-then-INSERT TOCTOU) -
tatara-chat/internal/chat/store.go:227-242[correctness] - Insert conditionally on active status in one statement, e.g.INSERT INTO chat_participants (...) SELECT ... FROM chat_rooms WHERE id=$1 AND status='active'an - Auth middleware logs to slog default (non-JSON) logger, never the configured JSON logger -
tatara-chat/internal/auth/middleware.go:27-34[observability] - Either call slog.SetDefault(o.Logger) in buildObs after constructing the bundle (cheapest), or thread the configured *slog.Logger into auth.Middleware (e.g. Mid - shutdown swallows the OTLP-flush error and always returns nil -
tatara-chat/cmd/tatara-chat/app.go:44-60[error-handling] - Log each non-nil error (e.g. a.log.Error("otlp shutdown", "error", err) and a.log.Error("db close", "error", err)), and consider joining them with errors.Join i - recoverPanicsTotal package global is mutated by NewMetrics and shared across router instances -
tatara-chat/internal/httpapi/middleware.go:81-84,104-106,170-171[concurrency] - Make Recover a method/closure that captures the metric and logger instead of reading package globals: have NewMetrics/NewRouter pass the *CounterVec and *slog.L
cross-cutting (11)¶
- Ingester Job has no ttlSecondsAfterFinished, so completed/failed Jobs accumulate -
tatara-memory-repo-ingester/charts/tatara-memory-repo-ingester/templates/job.yaml:8-15[config-chart] - AddttlSecondsAfterFinished: 3600(and consideractiveDeadlineSeconds) to the Job spec so finished Jobs are garbage-collected. Surface the TTL as a camelCas - tatara-memory Secret renders two keys (pg-password, oidc-client-secret) that nothing consumes -
tatara-memory/charts/tatara-memory/templates/secret.yaml:10-15[tech-debt] - Remove the pg-password key entirely (PG_DSN is the only DB credential path). Remove oidc-client-secret and the oidcClientSecret value unless a consumer is added - Argo ClusterWorkflowTemplate CI containers run with no pod/container securityContext -
tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-go-ci.yaml:75-131[security] - Add a securityContext (runAsNonRoot, allowPrivilegeEscalation:false, capabilities drop ALL, seccompProfile RuntimeDefault) to the steps that do not require root - helm dependency update runs before helm registry login; OCI chart deps on private harbor would fail to resolve -
tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-helm-publish.yaml:150-153[correctness] - Move thehelm registry loginblock to run beforehelm dependency updatein both cwt-helm-publish.yaml and cwt-tatara-memory-tag.yaml. - package-push retryStrategy is futile against immutable harbor repos: re-pushing an already-pushed chart/image tag always fails -
tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-helm-publish.yaml:112-153[error-handling] - Make publish idempotent: detect 'already exists'/409 fromhelm push(and kaniko/goreleaser) and treat it as success, or guard with a pre-check that skips push - Container build still uses kaniko with shared --cache-repo, not the buildkit-on-ceph rootless build referenced by current platform direction -
tatara-argo-workflows/charts/tatara-argo-workflows/values.yaml:31[tech-debt] - Either retire these kaniko CWTs in favour of the ARC buildkit-on-ceph pipeline (if this substrate is superseded - record the decision in MEMORY), or align them - github-status treats every non-201 as failure but cannot distinguish auth failure from a bad sha, and continueOn:failed hides all of it -
tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-github-status.yaml:73-84[observability] - Acceptable to keep continueOn for resilience, but surface the failure: emit a structured (JSON) line distinguishing 401/403 (auth) from 422 (bad sha) so log-bas - go-ci test step has no retryStrategy, no timeout, and
set -euxleaks nothing sensitive but a hunggo test -raceblocks indefinitely -tatara-argo-workflows/charts/tatara-argo-workflows/templates/cwt-go-ci.yaml:119-131[error-handling] - AddactiveDeadlineSecondsto the test template and pass an explicitgo test -timeout=<e.g. 5m>so a hung suite fails fast rather than relying on per-package - Operator injects no TATARA_CHAT_URL into agent pods; chat tools fall back to public ingress default -
tatara-operator/internal/agent/pod.go:260-292[config-chart] - Add a TATARA_CHAT_URL env to BuildPod pointing at the in-cluster chat service (mirroring how memoryEndpoint/OperatorURL are wired), or explicitly confirm public - Operator MemoryConfigMap omits OPENAI_API_KEY; community labels silently degrade to member names -
tatara-operator/internal/memory/memory_builders.go:14-40[observability] - If LLM community labels are intended for per-project memory, add secretEnv("OPENAI_API_KEY", cfg.OpenAISecretName, "LLM_BINDING_API_KEY") (and optionally SEMANT - commentOnIssue SCM write is not counted in operator_scm_writes_total -
tatara-operator/internal/restapi/handlers.go:332-424[observability] - After the writer.Comment call, record the result like the controller paths do: result := "ok"; if err != nil { result = "error" }; if s.metrics != nil { s.metri
NIT (67)¶
tatara-operator (24)¶
- triage-await-approval enters Conversation before resetAgentRun, risking a leaked live wrapper pod -
tatara-operator/internal/controller/lifecycle.go:745-751[correctness] - Tear down the wrapper (resetAgentRun) before, or as part of, the Conversation transition so a failure leaves the task in Triage with the pod still owned/reapabl - PollOnce passes a cached list snapshot to recordResult, widening the stale subtask-write window -
tatara-operator/internal/controller/turncallback.go:277-283[concurrency] - Fixing the recordResult subtask-write guard (resolve annCurrentSubtask from a fresh Get under the turnId guard) resolves this too; until then, recordResult shou - parkWithComment checks task.Spec.Source != nil twice in immediate succession (dead branch structure) -
tatara-operator/internal/controller/lifecycle.go:107-130[simplification] - Merge into a singleif task.Spec.Source != nil { provider = task.Spec.Source.Provider; commentRef := task.Spec.Source.IssueRef; ... }block. - maybeOpenFollowupIssue builds the issue body/title with no INFO log of remaining-scope size and no validation of empty goal -
tatara-operator/internal/controller/lifecycle.go:1252-1285[observability] - Add the parent "pr_url" field (and optionally duration_ms around the CreateIssue call) to the INFO log so follow-up issue creation is auditable per hard rule 12 - buildTriagePromptFor uses content.Title/Body after a GetIssue error without resetting content to a known-empty value -
tatara-operator/internal/controller/lifecycle.go:588-597[correctness] - For uniformity with the other non-fatal branches, on GetIssue error eitherreturn lifecycleTriageText(task, "", "")or explicitly note the fall-through is int - Per-candidate ScanItem(.Inc()) loops do N atomic increments where one Add(N) suffices -
tatara-operator/internal/controller/projectscan.go:696-698[efficiency] - Add a metrics helper ScanItemAdd(activity, outcome string, n int) doing WithLabelValues(...).Add(float64(n)) and replace the count loops with a single call. - runScans consider() uses soonest==0 as both 'unset' and clamped-due-now, so a genuine due-now requeue is indistinguishable from no-requeue -
tatara-operator/internal/controller/projectscan.go:1377-1389[correctness] - Use a boolsetflag (or a sentinel like time.Duration(-1)) to distinguish 'unset' from a clamped 0, so a genuine due-now value of 0 forces an immediate requeu - Per-candidate ScanItem Inc loops instead of a single Add -
tatara-operator/internal/controller/projectscan.go:696-698,820-822,714-718,871-875[efficiency] - Add a counted helper (e.g. r.Metrics.ScanItems(activity, outcome, n) wrapping WithLabelValues(...).Add(float64(n))) and replace the loops with a single call; th - selectCandidates may return a slice aliasing the priority backing array -
tatara-operator/internal/controller/projectscan.go:234-238[correctness] - Build out with an explicit fresh allocation: out := make([]candidate, 0, len(withPriority)+len(rest)); out = append(out, withPriority...); out = append(out, res - recovery_exhausted metric emitted on both skip and close paths obscures close rate -
tatara-operator/internal/controller/projectscan.go:731-739[observability] - Drop the recovery_exhausted increment on the close branch (closeExhaustedPR already emits recovery_closed/recovery_close_error), or use a distinct outcome label - Recovery-exhausted skip path has metric but no INFO business log -
tatara-operator/internal/controller/projectscan.go:729-739[observability] - Add an l.Info on the already-exhausted skip with action, resource_id (proj.Name), repo, pr fields (debounced or at low frequency) so the permanent-park decision - updateInflightGauge does a full-namespace Task list and is invoked redundantly multiple times per reconcile -
tatara-operator/internal/controller/task_controller.go:944-970[efficiency] - Call updateInflightGauge once per reconcile (drop the duplicate at line 273, since line 213 already runs on the success path), or move the in-flight gauge recom - pod.go hardcodes the brainstorm-sources annotation key literal instead of sharing the projectscan const -
tatara-operator/internal/agent/pod.go:313-314[tech-debt] - Promote the annotation key to a shared exported constant in api/v1alpha1 (where LabelActivity etc. already live) and reference it from both projectscan.go and p - mergeAllowed carries five unused parameters (ctx, repo, writer, token, number) - dead signature surface -
tatara-operator/internal/controller/writeback.go:879-895[simplification] - Reduce the signature to mergeAllowed(proj *tatarav1alpha1.Project, st scm.PRState) (bool, error) and update the single call site at line 737. - writeBackReview discards proj with
_ = proj- either use scmContext's narrower return or drop the value -tatara-operator/internal/controller/writeback.go:657-661[simplification] - Assign proj to the blank identifier directly in the call (_, repo, writer, token, provider, err := r.scmContext(...)) and remove the separate_ = projline. - Dead defensive check on a Required, MinLength-validated cron field -
tatara-operator/internal/controller/repository_controller.go:312-314[simplification] - Remove|| repo.Spec.ReingestSchedule == ""(the parse at line 316 already handles a malformed value), or add a one-line comment noting it guards pre-migration - GitHub combinedStatus drops unknown combined-status states to "" instead of pending -
tatara-operator/internal/scm/github.go:536-548[correctness] - Add a default branch that maps any unrecognized non-empty state with TotalCount>0 to "pending" (fail-safe), so an unknown state never silently becomes no-data o - GitHub GraphQL error handling surfaces only errors[0] and ignores partial-data errors -
tatara-operator/internal/scm/github_graphql.go:53-62[error-handling] - For the dual-root queries, only treat errors as fatal when env.Data could not be decoded into a usable result; otherwise join all error messages (errors.Join or - ghResourceID/ghProjectItemID pass caller itemURL straight into a GraphQL URI! variable without validation -
tatara-operator/internal/scm/github_graphql.go:181-219[security] - Validate itemURL parses as https and host is the configured GitHub host before the GraphQL call; reject otherwise. Same for ghProjectItemID. - ghOwnerRepo treats only the last two path segments as owner/repo, silently mishandling deeper paths -
tatara-operator/internal/scm/github.go:181-192[correctness] - Require exactly two non-empty segments (len(parts)==2) for a GitHub repo URL and error otherwise, so malformed/deeper URLs fail fast at parse time instead of pr - Inbound provider (from headers) is never checked against the project's configured SCM provider -
tatara-operator/internal/webhook/server.go:80-112[correctness] - After loading the Project, if proj.Spec.Scm != nil && proj.Spec.Scm.Provider != "" && proj.Spec.Scm.Provider != providerName, reject with a distinct 400/provide - CronActivity.MaxPerRepo lacks a Minimum validation; 0/negative is silently clamped to 1 rather than rejected -
tatara-operator/api/v1alpha1/project_types.go:77-80[correctness] - Add+kubebuilder:validation:Minimum=1above MaxPerRepo so the apiserver rejects 0/negative at admission, making the spec value and the effective value identic - Push receiver DELETE (best-effort run cleanup) is not counted in operator_push_receive_total -
tatara-operator/internal/pushmetrics/receiver.go:267-272[observability] - Increment r.receiveTotal.WithLabelValues("deleted").Inc() in the DELETE branch and pre-init the "deleted" label so it appears in Gather. - Verifier.Verify decodes sub/iss via Claims then overwrites them from the token, making the struct json tags partly dead -
tatara-operator/internal/auth/verifier.go:43-51[simplification] - Drop the json:"sub" and json:"iss" tags (keep only preferred_username decoded via Claims) since Subject/Issuer are authoritatively set from the verified token,
tatara-memory (15)¶
- betweenness stored as Postgres real (float32) while computed/bound as float64, silently downcasting -
tatara-memory/internal/codegraph/analytics_store.go:73-82[correctness] - Either widen the column to double precision (migration: ALTER COLUMN betweenness TYPE double precision) to match the computed type, or accept float32 explicitly - Redundant
repo := repoloop-variable copy on Go 1.25 -tatara-memory/internal/codegraph/worker.go:118[simplification] - Delete therepo := repoline; the closure captures the per-iteration variable correctly under Go 1.22+. - Invalid direction on /code/neighbors silently coerced to "out" -
tatara-memory/internal/httpapi/codegraph.go:223[error-handling] - Reject directions outside {"", "in", "out"} with a 400 in the handler, or document the normalization in the OpenAPI/contract so clients do not rely on rejected - Pure-deletion reconcile returns 202 Accepted with a synthetic succeeded job -
tatara-memory/internal/httpapi/ingest.go:106-110[correctness] - Return http.StatusOK for the synchronous pure-deletion path (and consider returning the purge count rather than a job with no ID), reserving 202 for the genuine - Local variable
capshadows the builtin cap() in five codegraph handlers -tatara-memory/internal/httpapi/codegraph.go:366-368, 488-491, 514-517, 600-603, 636-639[simplification] - Rename the local tolim(ormaxN) in all five sites, e.g.lim := clampListLimit(limit); if len(edges) > lim { edges = edges[:lim] }. - EntityUpdatePayload lets raw Properties keys silently overwrite reserved entity fields -
tatara-memory/internal/memory/translate.go:109-124[correctness] - Skip the three reserved keys when copying Properties (mirror EntityFromGraphNode), or assign Properties first and the typed fields last so typed fields always w - DeleteMemoriesBySources performs a batch business action with no INFO log or metric and aborts on first file error -
tatara-memory/internal/memory/service.go:206-216[observability] - Emit one INFO at completion (repo, files_count, total_purged, duration_ms) and optionally a counter; include the failing file in any returned error for traceabi - Resume's doc comment claims no worker claims resumed jobs concurrently, but Start() launches workers before Resume() runs (app.go), making the comment false and masking a real concurrent-drain window -
tatara-memory/internal/ingest/pool.go:136-162[concurrency] - Either call Resume before Start at the app.go call site (workers not yet running while ResetRunningItems + the notify loop execute), or correct the comment to s - ListUnfinishedJobs has no ORDER BY, so resume order is nondeterministic and oldest jobs can be starved behind newer ones on a backlog at startup -
tatara-memory/internal/ingest/pgstore.go:216-234[algorithm] - Add ORDER BY created_at to make resume FIFO and deterministic:SELECT id FROM ingest_jobs WHERE status IN ('queued','running') ORDER BY created_at. - json.Marshal return errors discarded across CreateJob/UpdateJob/IncrementJobProgress; a marshal failure silently writes empty/garbage JSON -
tatara-memory/internal/ingest/pgstore.go:34, 46, 81, 201[error-handling] - Leave as-is if the types are provably marshal-safe (current case), but if any non-trivial type is ever stamped here, return fmt.Errorf("...: %w", err) on marsha - Retry-After HTTP-date form is silently ignored, falling through to exponential backoff -
tatara-memory/internal/lightrag/http.go:192-201[error-handling] - On Atoi failure, attempt http.ParseTime(ra) (or time.Parse(http.TimeFormat, ra)) and, if it parses to a future instant, sleep until then with the same ctx-aware - analytics.Compute logs via package-global slog, not an injected logger -
tatara-memory/internal/analytics/compute.go:164-170[observability] - Accept a *slog.Logger (or pass Config.Logger) and log through it; if Compute must stay dependency-free, return the duration/counters in Result and let the calle - fake removeLabel reuses backing array via s[:0], can corrupt a caller-shared slice -
tatara-memory/internal/lightrag/fake/fake.go:428-436[correctness] - Allocate a fresh slice: out := make([]string, 0, len(s)) and append non-matching elements, matching the non-aliasing style of SeedLabels. - resp.Body Close uses defer inside the retry loop instead of explicit close -
tatara-memory/internal/lightrag/http.go:212-220[simplification] - Close explicitly on the success path too (close after the io.Copy/Decode, before each return), so the loop has no deferred resource and the pattern is uniform w - Query() does not detect HTTP-200 logical-failure envelopes the way QueryData() does -
tatara-memory/internal/lightrag/http.go:270-284[correctness] - Confirm the /query response shape for the pinned LightRAG version; if it can carry a status/failure envelope, mirror the QueryData LogicalError handling. Otherw
tatara-memory-repo-ingester (7)¶
- Confidence cap uses fragile lexicographic string comparison -
tatara-memory-repo-ingester/internal/analyze/python.go:619-623[tech-debt] - Parse to float and compare numerically (strconv.ParseFloat, math.Min(v, 0.45)) then format back, or centralize a cappedConfidence(resolution, cap float64) helpe - helm Match does a filesystem stat per templates/ file during grouping -
tatara-memory-repo-ingester/internal/analyze/helm.go:41-54[efficiency] - Memoize chartRoot->exists results on the helmAnalyzer (a small map guarded for the single-threaded Group call, or computed once in Analyze). Low impact; accepta - ExtractorSCIP ('scip') is not a documented server extractor constant; relies on free-string column behavior -
tatara-memory-repo-ingester/internal/contract/contract.go:76-81[tech-debt] - Add an ExtractorSCIP constant to tatara-memory codegraph and reference the same vocabulary, or document in both contracts that 'scip' is a recognized extractor - README/MEMORY claim SCIP pushes 'code graph only' and cross-repo symbols are 'deferred to ROADMAP', but Parse now emits provides/requires SymbolRows -
tatara-memory-repo-ingester/internal/scip/scip.go:106-114,199-207[tech-debt] - Update MEMORY.md/README to reflect that SCIP now emits SymbolRows (and document the moniker-normalization + extractor-scoping requirements), or revert the Symbo - headCommit shells out to
git rev-parse HEADtwice per run -tatara-memory-repo-ingester/cmd/tatara-ingest/run.go:122-128[efficiency] - Hoistcommit := headCommit(o.repoRoot)above theif len(touched) == 0check and reference the single variable in both the no-op log and the push payload. - OIDCClient builds the token URL by string concat with no validation; an empty oidc-issuer yields a request to a relative/garbage token endpoint that only surfaces as a generic transient error -
tatara-memory-repo-ingester/cmd/tatara-ingest/main.go:45-48[error-handling] - In config.Load or realMain, when oidc-client-id is set, require oidc-issuer (non-empty, parseable URL) and oidc-client-secret, returning a clear config error be - metrics push failure is logged but not counted; no metric distinguishes a failed end-of-job metrics push -
tatara-memory-repo-ingester/cmd/tatara-ingest/run.go:52-58[observability] - This is inherently hard to self-report; at minimum keep the WARN (already present) and ensure the operator's pushmetrics receiver records a receive-rate it can
tatara-claude-code-wrapper (4)¶
- relaunch ring.reset() can still be followed by a late write from the dying proc's readPTY; comment overstates the ordering guarantee -
tatara-claude-code-wrapper/internal/session/session.go:466-478[correctness] - Either join the old readPTY goroutine before reset (track it on a per-proc done channel and wait on it), or have readPTY exit-and-signal so relaunch can reset o - writeClaudeJSON persists the last 20 chars of the Anthropic API key to disk -
tatara-claude-code-wrapper/internal/bootstrap/claudejson.go:21-26, 37-42[security] - Acceptable per the onboarding recipe; keep 0600. If hardening, confirm real keys are always >20 chars (they are for sk-ant-*), and document the partial-secret-a - claudeHome and skills dirs created 0755 while .mcp.json/.claude.json may sit beside secret-bearing MCP env -
tatara-claude-code-wrapper/internal/bootstrap/mcp.go:53-55[config-chart] - Document that overlay fragments must not embed secrets (use env passthrough), or detect anenvkey in a merged server and either drop it or tighten the file t - pushOnce uses the long-lived loop context with no per-push deadline beyond the 10s client timeout -
tatara-claude-code-wrapper/internal/pushclient/pushclient.go:84-130[error-handling] - Optional: wrap each pushOnce in its own context.WithTimeout(p.ctx, p.cfg.Interval) so a push can never outlive its interval, making overlap impossible regardles
tatara-cli (7)¶
- Passthrough tools forward the whole args map to backends that use DisallowUnknownFields, with no additionalProperties:false on the schema -
tatara-cli/internal/mcp/tools.go:42-96[correctness] - Either add "additionalProperties":false to these object schemas so MCP clients reject unknown keys before dispatch, or build an explicit whitelist body (text/me - Device-flow Poll ignores server-provided interval after slow_down and uses a fixed +5s bump -
tatara-cli/internal/auth/device_flow.go:65-98[algorithm] - Parse an optionalintervalfrom the slow_down error response and adopt it (clamped to a sane max) instead of a blind +5s; or document the fixed-bump choice. L - expiryDesc reports a token expired by up to 499ms as "valid for 0s" -
tatara-cli/internal/cmd/status.go:80-86[correctness] - Compute the sign before rounding: `raw := time.Until(exp); if raw >= 0 { return "valid for " + raw.Round(time.Second).String() }; return "expired " + (-raw).Rou - Do cannot retry/refresh-then-resend with an io.Reader body (single-use stream consumed once) -
tatara-cli/internal/client/client.go:70-122[correctness] - If/when a 401-driven retry is added, buffer non-[]byte bodies (or require callers to pass []byte) so the request is replayable. Document the no-retry contract o - mcp-config writes 0644 .mcp.json (fine, but only safe because no secrets are written) -
tatara-cli/internal/cmd/mcp_config.go:61-63[security] - No change needed for current behavior; optionally note in code that .mcp.json must never hold secrets, or honor an existing stricter file mode when merging into - raw prints HTTP status line to stderr unconditionally, mixing with the response body on stdout -
tatara-cli/internal/cmd/raw.go:112-119[observability] - Pass a slog logger into the raw client config (respecting --verbose) so the method/path/status_code/duration_ms INFO log is emitted, satisfying hard rule 12 for - device Start error includes raw server response body which may carry sensitive detail -
tatara-cli/internal/auth/device_flow.go:54-57[security] - Surface the status code plus a parsederror/error_descriptionfield (as the token exchange path does) rather than the entire raw body; or truncate the body.
tatara-chat (4)¶
- 0002 re-runs SET NOT NULL on every startup, re-validating with a full-table scan -
tatara-chat/internal/chat/migrations/0002_last_activity.sql:1-17[efficiency] - Guard the NOT NULL flip so it only runs when needed, e.g. wrap in a DO block that checks information_schema.columns.is_nullable before ALTER, or (preferred) int - RemoveParticipant has no store metric despite removeParticipant being a logged business action -
tatara-chat/internal/chat/store.go:263-279[observability] - Add a named error return anddefer s.observe("remove_participant", time.Now(), &err); covered by the broader fix in store-metrics-partial-instrumentation but - readyz logs failures but never the recovery / no metric for readiness flaps -
tatara-chat/cmd/tatara-chat/health.go:16-25[observability] - Increment a readyz_failures_total counter (registered on the shared registry) alongside the WARN so readiness flaps are alertable from Prometheus, not just grep - Inbound X-Request-Id is trusted and echoed verbatim with no validation -
tatara-chat/internal/httpapi/middleware.go:43-55[security] - Validate the inbound id before trusting it: cap length (e.g. <= 128 bytes) and restrict to a safe charset (hex/UUID/alnum-dash); on failure generate a fresh id
cross-cutting (6)¶
- Ingester ConfigMap omits namespace metadata, inconsistent with every other template -
tatara-memory-repo-ingester/charts/tatara-memory-repo-ingester/templates/configmap.yaml:1-6[config-chart] - Addnamespace: {{ .Release.Namespace }}to the ConfigMap metadata to match job.yaml and the other charts. - Ingester ConfigMap hand-rolls env keys instead of an envConfig helper used by every other chart -
tatara-memory-repo-ingester/charts/tatara-memory-repo-ingester/templates/configmap.yaml:7-13[simplification] - Define atatara-memory-repo-ingester.envConfigin _helpers.tpl and reference it from configmap.yaml, matching the other charts. (A config checksum on a one-sh - tatara-memory deployment secretRef points at a Secret whose only keys are unused -
tatara-memory/charts/tatara-memory/templates/deployment.yaml:44-45[config-chart] - Once the dead keys are removed (see memory-secret-dead-keys), drop the default secretRef envFrom for the generated Secret, or gate it on existingSecret so a no- - rbac.yaml header comment claims cluster-admin was replaced, but a prior commit re-added cluster-admin; verify the scoped Role is actually the deployed state -
tatara-argo-workflows/charts/tatara-argo-workflows/templates/rbac.yaml:1-9[tech-debt] - Add a dated MEMORY.md entry recording: cluster-admin was used briefly (CRD perms), replaced by the scoped Role + optional cluster-scoped CRD/ClusterRole grant g - Wrapper internal turn-complete handler emits no business log or metric on its failure branches -
tatara-claude-code-wrapper/internal/httpapi/internal.go:10-21[observability] - Add a metric (e.g. extend metrics.HookReceived into a CounterVec with a result label, or add a ccw_hook_post_total{result=ok|bad_payload|rejected}) and an a.log - turn_timeout_total{source=planning_watchdog} label combination never pre-initialised -
tatara-operator/internal/obs/operator_metrics.go:201-203[observability] - Add "planning_watchdog" to the pre-seed slice at operator_metrics.go:201 so the series exists from startup, matching the documented pre-initialisation intent an