CLI-728 Queue analysis findings in a local file and drain them through the background telemetry flush

[vnaskos-sonar]

Summary

• Move TELEMETRY_ENDPOINT and TELEMETRY_API_KEY from module-level constants in src/telemetry/index.ts to exported constants in src/lib/config-constants.ts so they can be shared across telemetry modules
• Add src/telemetry/findings.ts with three exports:
  • appendFinding(payload) — fire-and-forget low-level write: creates the telemetry dir if missing, appends one JSON line to ~/.sonar/sonarqube-cli/telemetry/findings.ndjson, silently swallows all I/O errors
  • emitSecretsFindings(callerCommand, auth, issues, durationMs) — higher-level helper that gates on isTelemetryEnabled, resolves identity fields from state/auth, and calls appendFinding() per issue; call sites added in CLI-730
  • flushFindings(deadline) — atomically renames findings.ndjson → findings.<uuid>.sending, drops events older than 7 days, POSTs one event per HTTP call, deletes the .sending file in a finally block; a worker that loses the rename race exits immediately
• Extend flushTelemetry() in src/telemetry/index.ts to call flushFindings(deadline) after the existing state.telemetry.events[] drain

---

Summary by Gitar

• New utility:
  • Added timed helper in src/lib/timed.ts to measure execution duration of asynchronous tasks.
• Type definitions:
  • Added AnalysisFindingEventPayload and StoredFindingEvent to src/lib/state.ts for structured finding data.
• Configuration:
  • Added getTelemetryDir() in src/lib/config-constants.ts to centralize the location for telemetry artifacts.
• Testing:
  • Added comprehensive unit tests for findings logic in tests/unit/telemetry/findings.test.ts.
  • Integrated findings drain verification into tests/unit/telemetry/telemetry.test.ts.
  • Added coverage for timed() helper in tests/unit/lib/timed.test.ts.

_{This will update automatically on new commits.}

[hashicorp-vault-sonar-prod]

CLI-728

[gitar-bot]

Code Review ✅ Approved 6 resolved / 6 findings

Implements local queuing and background telemetry flushing for analysis findings, resolving issues with file management, path handling, and identification consistency. No remaining issues identified.

✅ 6 resolved

✅ Bug: flushFindings drops unsent findings permanently on timeout/error

📄 src/telemetry/findings.ts:98-112
flushFindings renames findings.ndjson to a .sending file and unconditionally rmSyncs it in the finally block. Findings are lost forever in two cases:

1. Deadline reached (remainingTime <= 0 → break): every remaining event in the batch is dropped with the .sending file, with no retry on the next flush.
2. Server error: fetchGuarded does NOT throw on non-2xx responses (it returns the Response as-is, see src/lib/fetch-guarded.ts:66), so a 500/429 is treated as success and the finding is discarded.

This differs from the adjacent state.telemetry.events[] drain, which preserves unsent events for the next flush attempt via sentIndices. For large finding batches that exceed the 60s deadline, the tail of the batch is silently lost. If findings completeness matters for downstream analytics, consider re-writing surviving/unsent events back to findings.ndjson instead of deleting the whole .sending file, and check response.ok before considering an event sent.

✅ Quality: TELEMETRY_DIR constant is unused and statically computed

📄 src/lib/config-constants.ts:220-221 📄 src/telemetry/findings.ts:33-34 📄 src/telemetry/findings.ts:52-53
The new TELEMETRY_DIR = join(CLI_DIR, 'telemetry') constant is exported but never referenced anywhere. findings.ts instead builds the telemetry path from getCliDir() plus a locally-defined TELEMETRY_SUBDIR = 'telemetry'. This is a latent trap: TELEMETRY_DIR is derived from the module-level CLI_DIR constant, which is resolved once at load time, whereas getCliDir() re-reads ENV_SONAR_USER_HOME dynamically. Any future caller that imports TELEMETRY_DIR expecting it to honor an env override (as the rest of the path code does, and as the tests rely on) will get a stale path. The custom review guidance also asks that paths be centralized in config-constants. Recommend either removing the dead constant or replacing TELEMETRY_SUBDIR/inline 'telemetry' joins in findings.ts with a dynamic helper (e.g. getTelemetryDir()), so there is a single source of truth that respects the env override.

✅ Quality: Re-append of unsent findings on send failure is untested

📄 tests/unit/telemetry/findings.test.ts:338-352 📄 src/telemetry/findings.ts:147-161
The core durability behavior of flushFindings — re-appending unsent events to findings.ndjson so they survive for the next flush attempt (the finally block at findings.ts:173-186) — is not covered by any test. The closest test, 'silently swallows individual send failures', only asserts that the call does not throw and that fetch was invoked twice; it never reads back findings.ndjson to confirm the failed event (secrets:A) was preserved while the succeeded one was dropped. Likewise, the deadline/timeout path (loop break at findings.ts:155) that re-appends the not-yet-attempted events is untested. This is exactly the regression a previous review flagged ('flushFindings drops unsent findings permanently on timeout'), so it is worth locking in with an assertion. Consider asserting on readLines(testSonarUserHome) after a partial failure to confirm only the unsent finding remains.

✅ Edge Case: Non-null assertion on optional installationId may emit empty id

📄 src/telemetry/findings.ts:86
emitSecretsFindings builds the payload with cli_installation_id: state.telemetry.installationId!, but installationId is declared optional (installationId?: string in state.ts) and isTelemetryEnabled only checks state.telemetry.enabled — it does not guarantee an installation id is present. getDefaultState and the !raw.telemetry migration both set one, so this is safe for normal upgrades; however a state.json that contains a telemetry object missing the installationId field would not be backfilled, and the non-null assertion would pass undefined through (later stripped by the null/undefined JSON replacer), emitting a finding event with no cli_installation_id. Consider falling back to a defined value (e.g. resolve/regenerate the id) or guarding on its presence before emitting.

✅ Edge Case: Orphaned .sending files are never recovered, losing findings

📄 src/telemetry/findings.ts:115-127 📄 src/telemetry/findings.ts:171-184
flushFindings atomically renames findings.ndjson to findings.<uuid>.sending, processes it, and only re-appends unsent events / deletes the .sending file in the finally block. If the detached flush worker is killed or crashes after the renameSync (line 121) but before the finally runs — or hits the 60s hard deadline mid-stream and is terminated — the .sending file is left behind. No code path ever scans for or drains pre-existing .sending files: flushFindings only looks at findings.ndjson (line 116-117). Those findings are therefore stranded permanently and silently. Since this is best-effort telemetry the impact is low, but consider having flushFindings (or a startup sweep) re-ingest any leftover findings.*.sending files before creating a new one, so an interrupted flush can be recovered on the next run.

...and 1 more resolved from earlier reviews

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues
0 New dependency risks

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud