feat: add provider-neutral telemetry adapters by samtay32 · Pull Request #32 · samtay32/ultimate-agent-stack

samtay32 · 2026-07-29T08:16:29Z

Summary

add optional read-only PostHog, Sentry, and New Relic telemetry adapters
keep providers disabled by default with fixed endpoints, credential isolation, bounded redacted receipts, and repository fallback
add guided setup/health documentation, behavioral coverage, package coverage, and provider-specific adversarial tests

Validation

npm run release:check (145 tests, 14 behavioral contracts, dry pack, packed smoke)
npx --yes node@22 --test (145 tests)
npx --yes markdownlint-cli2@0.18.1 '**/*.md' '#node_modules' (59 files)\n- git diff --check\n\n## Deferred by design\n\nNative telemetry sessions, automatic capture, Agent Auth, and vendor-specific lock-in remain outside this change.

Summary by CodeRabbit

New Features
- Added optional, reviewed read-only telemetry connections for PostHog, Sentry, and New Relic.
- Added telemetry-setup and telemetry-health commands to configure credentials and verify provider identity, scope, and availability.
- Added telemetry status details to diagnostics and startup checks.
Documentation
- Added setup guidance, provider requirements, usage constraints, and fallback behavior.
Security
- Restricted connections to approved endpoints and bounded health checks without retaining raw provider data.

coderabbitai · 2026-07-29T08:16:39Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d3f08b55-ffff-4258-a52e-6dd7b6ae3abe

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

This change adds protected, read-only telemetry health adapters for PostHog, Sentry, and New Relic, integrates provider configuration and health commands into the CLI, updates onboarding guidance and documentation, packages the helper, and adds coverage for validation, bounded responses, fallback behavior, and tamper detection.

Changes

Reviewed telemetry connections

Layer / File(s)	Summary
Protected provider health adapter `scripts/telemetry-readonly.mjs`, `test/telemetry-readonly.test.mjs`	Adds bounded, redirect-blocked health checks for PostHog, Sentry, and New Relic with provider identity validation, normalized results, credential redaction, and input validation.
CLI telemetry configuration and health flow `bin/ultimate-agent-stack.mjs`	Adds `--telemetry`, provider validation, source-hash protection, `telemetry-setup`, `telemetry-health`, provider health reporting, startup readiness checks, and onboarding integration.
Telemetry onboarding and provider boundaries `README.md`, `STARTER_PROMPT.md`, `assets/project-template/`, `docs/`, `skills/use-project-telemetry/*`, `.codex-plugin/plugin.json`	Documents reviewed providers, credential setup, health verification, bounded observation rules, repository fallback, and deferred provider-native capabilities.
Packaging and behavioral validation `package.json`, `scripts/packed-smoke.mjs`, `test/*`, `evals/scenarios.json`	Packages and syntax-checks the helper and tests configuration, provider requests, bounded outputs, fallback behavior, required outcomes, and protected-helper tampering.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

samtay32/ultimate-agent-stack#16: Introduced the trust-boundary documentation expanded here with telemetry-specific enforcement promises.
samtay32/ultimate-agent-stack#26: Established the project telemetry contract and CLI schema that this change extends with concrete reviewed provider adapters.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the main change: adding new telemetry adapters for PostHog, Sentry, and New Relic.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🛠️ Fix failing CI checks 💡

Fix failing CI checks

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch codex/telemetry-provider-adapters

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

samtay32 · 2026-07-29T08:16:48Z

/review

qodo-code-review · 2026-07-29T08:18:00Z

PR Summary by Qodo

Add provider-neutral read-only telemetry adapters (PostHog/Sentry/New Relic)

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Add optional read-only telemetry adapters for PostHog, Sentry, and New Relic.
• Enforce fixed endpoints, bounded redacted responses, and hash-pinned helper execution.
• Add guided setup/health commands plus docs and adversarial tests for safety invariants.

Diagram

graph TD
  CLI(["ultimate-agent-stack CLI"]) --> CFG["configure --telemetry"] --> CONF[".agent-stack/config.json"] --> HEALTH(["telemetry-health"])
  HEALTH --> PIN["hash check (pinned)"] --> HELPER(["telemetry-readonly.mjs"])
  HELPER --> EXT{{"PostHog / Sentry / New Relic"}} --> OUT["normalized health JSON"] --> SKILL(["use-project-telemetry skill"])

  subgraph Legend
    direction LR
    _svc([Service/Command]) ~~~ _file["Config/File"] ~~~ _ext{{External API}}
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Rely solely on OpenTelemetry/Collector and avoid provider APIs

➕ Avoids per-vendor API quirks and tokens
➕ Keeps a single vendor-neutral transport boundary
➖ Does not solve the immediate need to verify existing provider identity/scope
➖ Still requires out-of-band collector deployment and configuration choices
➖ Harder to enforce a fixed, minimal remote read surface from within this CLI

2. Implement provider checks directly in the main CLI (no helper script)

➕ Fewer moving parts (no separate helper file to install/hash-pin)
➕ Simpler execution path
➖ Harder to maintain a small, auditable remote surface separated from broader CLI concerns
➖ Loses the clean 'protected helper' boundary that’s easy to pin and adversarially test

3. Use provider SDKs instead of raw HTTP fetch

➕ Less hand-written request/response validation
➕ Potentially more robust to API changes
➖ Adds heavier dependencies and transitive risk
➖ SDKs often expose broader capabilities than intended for a fixed read-only adapter
➖ More difficult to prove bounded payload capture and no-redirect behavior

Recommendation: Keep the PR’s current approach: a separate hash-pinned, bounded, redirect-rejecting helper with fixed endpoints and a narrow normalized output. Given the trust/safety posture of this repo, the explicit helper boundary is a practical way to make the remote surface auditable, enforceable, and testable while keeping providers optional and repository-fallback by default.

Files changed (21) +1797 / -33

Enhancement (2) +1136 / -8

ultimate-agent-stack.mjsAdd telemetry provider registry, config parsing, and setup/health commands +662/-8

Add telemetry provider registry, config parsing, and setup/health commands

• Introduces a reviewed telemetry provider registry (roles, regions, credential envs), parses repeated --telemetry specs into bounded scopes, and enforces external-data=approved_providers. Adds telemetry-setup and telemetry-health commands, integrates telemetry health into doctor/start output, and installs/runs a hash-pinned telemetry-readonly helper with redacted bounded output.

bin/ultimate-agent-stack.mjs

telemetry-readonly.mjsAdd protected read-only telemetry health helper (fixed endpoints) +474/-0

Add protected read-only telemetry health helper (fixed endpoints)

• Implements bounded, redirect-rejecting health checks for PostHog (insight metadata), Sentry (project identity), and New Relic (fixed NerdGraph account identity query). Enforces approved regions only, validates scopes, bounds response capture, and returns normalized results without retaining raw payloads.

scripts/telemetry-readonly.mjs

Tests (6) +419 / -2

scenarios.jsonAdd telemetry scope health outcome to eval scenario requirements +1/-0

Add telemetry scope health outcome to eval scenario requirements
• Extends scenario required outcomes to include telemetry_scope_health alongside telemetry observation receipt.
evals/scenarios.json

packed-smoke.mjsVerify telemetry helper is present in packed install smoke test +4/-0

Verify telemetry helper is present in packed install smoke test
• Adds the protected telemetry-readonly helper to the packed smoke checks, ensuring it’s included and installed under .agent-stack/bin.
scripts/packed-smoke.mjs

agent-stack.test.mjsAdd behavioral + adversarial tests for telemetry configuration and helper pinning +207/-1

Add behavioral + adversarial tests for telemetry configuration and helper pinning

• Extends lifecycle tests to assert telemetry capabilities metadata is present. Adds tests that configure multiple providers, validate external-data gating, reject duplicates/custom specs, and prove telemetry-health refuses a tampered helper even if the installation manifest is spoofed.

test/agent-stack.test.mjs

maintenance.test.mjsAdd maintenance assertions for telemetry helper safety properties +12/-0

Add maintenance assertions for telemetry helper safety properties
• Ensures the telemetry helper is packaged and checks for key safety invariants (redirect error mode, bounded responses, no mutation surface, no raw payload retention).
test/maintenance.test.mjs

skill-eval.test.mjsUpdate skill eval expectations to include telemetry scope health +4/-1

Update skill eval expectations to include telemetry scope health
• Adjusts the telemetry diagnosis evaluation record to require telemetry_scope_health in addition to telemetry_observation_receipt.
test/skill-eval.test.mjs

telemetry-readonly.test.mjsAdd unit tests for telemetry-readonly helper bounding and identity checks +191/-0

Add unit tests for telemetry-readonly helper bounding and identity checks

• Adds provider-specific tests asserting fixed URLs/queries, identity verification behavior, and that results do not leak sensitive fields from responses. Includes negative tests for oversized responses, credential echo prevention, malformed scopes, and custom endpoints.

test/telemetry-readonly.test.mjs

Documentation (12) +240 / -22

plugin.jsonUpdate plugin copy to reflect reviewed telemetry adapters +2/-2

Update plugin copy to reflect reviewed telemetry adapters
• Adjusts the long description and capabilities text to reference verified PostHog, Sentry, and New Relic read-only telemetry connections.
.codex-plugin/plugin.json

README.mdDocument reviewed telemetry adapters and safe setup flow +34/-0

Document reviewed telemetry adapters and safe setup flow

• Adds a provider table, example configuration flags, and explicit safety boundaries (fixed endpoints, bounded responses, no arbitrary query/mutation). Clarifies deferred scope such as native sessions and auto-capture.

README.md

STARTER_PROMPT.mdRequire telemetry-health before using telemetry evidence +10/-3

Require telemetry-health before using telemetry evidence
• Updates delivery contract guidance to run telemetry-health first and to gate provider usage on approved, verified PostHog/Sentry/New Relic connections.
STARTER_PROMPT.md

HANDOFF.mdAdd telemetry-health prerequisite in handoff guidance +4/-4

Add telemetry-health prerequisite in handoff guidance
• Refines the project template instructions to require telemetry-health before telemetry use and to fall back to repository evidence on provider failure or scope issues.
assets/project-template/.agent-stack/HANDOFF.md

AGENTS.mdGuide agents to verify telemetry identity before use +3/-0

Guide agents to verify telemetry identity before use
• Adds explicit direction to run telemetry-health for PostHog/Sentry/New Relic and to avoid silently broadening scope or switching providers.
assets/project-template/AGENTS.md

ADAPTERS.mdDescribe reviewed telemetry adapter constraints and workflow +32/-0

Describe reviewed telemetry adapter constraints and workflow
• Documents the PostHog/Sentry/New Relic fixed roles, approved regions, fixed live checks, and the protected helper’s endpoint/redirect/payload bounds.
docs/ADAPTERS.md

ARCHITECTURE.mdAdd architecture notes for initial telemetry provider registry +8/-0

Add architecture notes for initial telemetry provider registry
• Explains the provider registry, source-hash-pinned helper behavior, and how this layer relates to OpenTelemetry without managing instrumentation.
docs/ARCHITECTURE.md

OPERATING_MANUAL.mdUpdate telemetry onboarding and safety operating guidance +16/-5

Update telemetry onboarding and safety operating guidance

• Replaces repository-only telemetry onboarding text with a guided decision + setup flow using --telemetry, telemetry-setup, and telemetry-health. Reinforces fixed checks, bounded payload handling, and fallback behavior.

docs/OPERATING_MANUAL.md

SOURCES_AND_TRADEOFFS.mdRecord design decision and references for telemetry adapters +40/-0

Record design decision and references for telemetry adapters

• Adds a dated decision narrative covering bounded control loops, optional adapters, and why provider-native sessions/Agent Auth remain deferred. Includes upstream references for Linear, PostHog, Sentry, New Relic, and OpenTelemetry.

docs/SOURCES_AND_TRADEOFFS.md

TRUST.mdStrengthen trust model details for telemetry helper enforcement +10/-5

Strengthen trust model details for telemetry helper enforcement

• Updates trust table and narrative to emphasize fixed endpoints, redirect rejection, bounded responses, and source-hash protection. Clarifies what the CLI can and cannot prove about upstream credential permissions.

docs/TRUST.md

SKILL.mdRequire telemetry-health and provider reference before telemetry use +11/-3

Require telemetry-health and provider reference before telemetry use
• Updates the skill workflow to read provider-specific limitations and to run telemetry-health before any telemetry observation. Clarifies that the shipped health adapter is not a general query surface.
skills/use-project-telemetry/SKILL.md

telemetry-providers.mdAdd provider reference for shipped telemetry connection surface +70/-0

Add provider reference for shipped telemetry connection surface
• Documents the exact fixed live checks per provider, credential environment variables, minimum requested access, and observation boundaries. Explicitly warns against turning the helper into an arbitrary query proxy.
skills/use-project-telemetry/references/telemetry-providers.md

Other (1) +2 / -1

package.jsonInclude telemetry helper in package and lint pipeline +2/-1

Include telemetry helper in package and lint pipeline
• Adds scripts/telemetry-readonly.mjs to the published files list and to the lint (node --check) script to keep the helper syntax-validated.
package.json

qodo-code-review · 2026-07-29T08:21:42Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (2)

Context used

✅ Compliance rules (platform): 95 rules

✅ Skills: 4 invoked

verify-change

maintain-agent-stack

build-vertical-slice

secure-launch

1. ~~Telemetry scope type bypass~~ ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New

Description

validateTelemetryProvider() applies RegExp.test() directly to scope fields, so non-string values
(including undefined and numbers) can be coerced and mistakenly pass validation. Those untyped scope
values are later inserted into the child-process argv for the telemetry helper, which can throw and
crash telemetry-health (and downstream callers like doctor/start).

Code

bin/ultimate-agent-stack.mjs[R1793-1821]

+      !TELEMETRY_NUMERIC_ID.test(value.scope.project_id) ||
+      !Number.isSafeInteger(Number(value.scope.project_id))
+    ) {
+      errors.push(`${label}.scope.project_id must be a positive numeric identifier`);
+    }
+  } else if (expectedProvider === "sentry") {
+    rejectUnknownKeys(
+      errors,
+      value.scope,
+      new Set(["organization", "project"]),
+      `${label}.scope`,
+    );
+    if (!TELEMETRY_IDENTIFIER.test(value.scope.organization)) {
+      errors.push(`${label}.scope.organization must be a bounded slug`);
+    }
+    if (!TELEMETRY_IDENTIFIER.test(value.scope.project)) {
+      errors.push(`${label}.scope.project must be a bounded slug`);
+    }
+  } else {
+    rejectUnknownKeys(
+      errors,
+      value.scope,
+      new Set(["account_id"]),
+      `${label}.scope`,
+    );
+    if (
+      !TELEMETRY_NUMERIC_ID.test(value.scope.account_id) ||
+      !Number.isSafeInteger(Number(value.scope.account_id))
+    ) {

Relevance

⭐⭐⭐ High
Team recently accepted adding typeof guards before RegExp.test to avoid coercion crashes in
validation paths.
PR-#29

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The validator currently tests potentially-missing/non-string values with RegExp.test(), which can
incorrectly accept them due to implicit string coercion, and these values are later used as raw argv
elements for spawning the telemetry helper.
bin/ultimate-agent-stack.mjs[1752-1824]
bin/ultimate-agent-stack.mjs[4603-4622]
lib/portable-process.mjs[541-550]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`validateTelemetryProvider()` uses `RegExp.test(...)` on `value.scope.*` without first validating the field types. Because `RegExp.test()` string-coerces its input, `undefined` becomes the string "undefined" (which matches the current slug regex), and numbers can also pass numeric-id checks. Later, `telemetryHelperArguments()` passes these scope values directly into `spawnPortable()` argv, where non-string args can throw and crash `telemetry-health` (and callers such as `doctor`/`start`).

### Issue Context
This is mainly reachable if a config is malformed/tampered but still passes `validateConfig()` and becomes “approved”. The validator should fail closed on missing/wrong-typed scope fields.

### Fix Focus Areas
- bin/ultimate-agent-stack.mjs[1752-1825]

### Recommended fix
- In `validateTelemetryProvider()`:
 - For Sentry: require `typeof value.scope.organization === "string"` and `typeof value.scope.project === "string"` before applying `TELEMETRY_IDENTIFIER.test(...)`.
 - For PostHog/New Relic: require `typeof value.scope.project_id === "string"` / `typeof value.scope.account_id === "string"` before applying `TELEMETRY_NUMERIC_ID.test(...)` and `Number.isSafeInteger(...)`.
 - Also fail if required keys are missing (e.g., `!Object.hasOwn(value.scope, "organization")`).
- Optional defense-in-depth: in `telemetryHelperArguments()`, coerce argv values with `String(...)` only after validating presence, to avoid passing non-strings to the process layer.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Test asserts calls sequence 📜 Skill insight ▣ Testability ⭐ New

Description

The new telemetry health shares one aggregate provider probe budget test asserts the internal
runTelemetry call sequence and passed timeouts via assert.deepEqual(calls, ...), coupling the
test to implementation details instead of publicly observable behavior. This makes the test brittle
to refactors that preserve behavior but change internal orchestration.

Code

test/agent-stack.test.mjs[R1645-1648]

+    assert.deepEqual(calls, [
+      { provider: "new-relic", timeout: 20_000 },
+      { provider: "posthog", timeout: 5_000 },
+    ]);

Relevance

⭐⭐ Medium
Repo often uses deepEqual to pin detailed behavior; unclear if they’ll relax this internal
call-sequence assertion.
PR-#29

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2395833 forbids tests that assert private call sequences. The test block captures
internal invocations in calls and then asserts exact order/timeout values, which is not a publicly
observable outcome of commandTelemetryHealth.

test/agent-stack.test.mjs[1645-1648]
Skill: build-vertical-slice

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A newly added test asserts private/internal call sequencing (`calls`) rather than only the observable output from `commandTelemetryHealth`.

## Issue Context
The compliance checklist requires tests to validate publicly observable behavior (return values / state / externally visible side effects) instead of coupling to internal call order/count.

## Fix Focus Areas
- test/agent-stack.test.mjs[1593-1653]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Telemetry availability false-positive~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

commandCapabilities() marks telemetry providers as available when the credential env var is merely
non-empty, but the telemetry helper rejects credentials shorter than 8 chars or containing
CR/LF/NUL. This can mislead onboarding/automation into thinking telemetry is usable when
telemetry-health will always fail for that credential.

Code

bin/ultimate-agent-stack.mjs[R4403-4406]

+                available:
+                  telemetryHelperAvailable &&
+                  typeof process.env[credential] === "string" &&
+                  process.env[credential].length > 0,

Relevance

⭐⭐⭐ High
Correctness mismatch: capabilities “available” check should align with helper’s credential
validation; team accepts env-validation hardening.
PR-#29
PR-#26

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Capabilities uses only length > 0 to decide availability, while the helper’s validCredential()
requires length >= 8 and forbids CR/LF/NUL; the new test demonstrates a short key is considered
invalid by health.
bin/ultimate-agent-stack.mjs[4341-4407]
scripts/telemetry-readonly.mjs[94-120]
test/agent-stack.test.mjs[1513-1520]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`commandCapabilities()` reports telemetry providers as `available` when the provider helper is intact and the credential environment variable is a non-empty string. However, the shipped helper rejects credentials that are too short (< 8) or contain `\r`, `\n`, or `\0`, so capabilities can show `available: true` even though `telemetry-health` will fail immediately.

## Issue Context
- The helper’s credential validity rules are stricter than the capabilities “available” predicate.
- A test already demonstrates a failing case (`POSTHOG_PERSONAL_API_KEY = "short"`) for `telemetry-health`, but capabilities would still mark PostHog as available.

## Fix Focus Areas
- bin/ultimate-agent-stack.mjs[4396-4407]

### Suggested fix
Update the telemetry provider `available:` predicate to apply the same syntactic checks as the helper (min length >= 8, max length <= 8192 if desired, and reject CR/LF/NUL). This keeps capabilities aligned with actual health execution behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. ~~Availability skips hash check~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

commandCapabilities marks telemetry providers as available when the helper file exists and a
credential env var is set, but it does not verify the helper matches the CLI-pinned protected hash.
This can advertise a tampered helper as “available” even though telemetry-health will refuse to run
it via protectedProjectFileIssue().

Code

bin/ultimate-agent-stack.mjs[R4400-4404]

+                available:
+                  projectExists(target, TELEMETRY_READONLY_PATH) &&
+                  typeof process.env[credential] === "string" &&
+                  process.env[credential].length > 0,
+                external: true,

Relevance

⭐⭐⭐ High
Team often accepts integrity hardening to fail closed; availability should align with protected
helper hash enforcement.
PR-#15
PR-#30

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new capability availability predicate checks only file existence + env var, while the execution
path enforces the pinned helper hash; this mismatch can report availability for a helper that will
be blocked at runtime.
bin/ultimate-agent-stack.mjs[4386-4417]
bin/ultimate-agent-stack.mjs[7049-7078]
test/agent-stack.test.mjs[1579-1616]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`commandCapabilities()` reports telemetry adapters as `available` without checking helper integrity against the protected/pinned hash. This creates inconsistent behavior vs `telemetry-health`, which correctly refuses tampered helpers.

### Issue Context
The telemetry helper is explicitly pinned via `TELEMETRY_READONLY_SOURCE_HASH` and enforced by `protectedProjectFileIssue()`. Capability discovery should use the same trust predicate to avoid claiming a provider is usable when it will be rejected later.

### Fix Focus Areas
- bin/ultimate-agent-stack.mjs[4386-4417]
- bin/ultimate-agent-stack.mjs[7049-7078]
- test/agent-stack.test.mjs[1579-1616]

### Suggested fix
- In the telemetry section of `commandCapabilities(target)`, incorporate `protectedProjectFileIssue(target, TELEMETRY_READONLY_PATH)` into the `available` computation (e.g., require it to be null).
- Add/extend a regression assertion in the existing tampered-helper test to ensure `commandCapabilities(...).available.telemetry.posthog.available` is `false` when the helper is modified.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View more (2)

5. ~~Health drops failure detail~~ ✓ Resolved 🐞 Bug ◔ Observability

Description

commandTelemetryHealth uses parseProviderJson, which does not parse helper JSON when the helper
exits non-zero; it then returns only a generic ${label} failed error. Since telemetry-readonly.mjs
always prints structured JSON (including a bounded error message) even when ok:false,
telemetry-health loses the specific failure reason (timeout/HTTP failure/bounds) and becomes hard to
troubleshoot.

Code

bin/ultimate-agent-stack.mjs[R5454-5464]

+    const parsed = parseProviderJson(
+      runTelemetryReadonly(target, provider),
+      `${provider.provider} read-only health check`,
+    );
+    if (!parsed.ok) {
+      return {
+        ok: false,
+        ...base,
+        live_check: "failed",
+        error: parsed.error,
+      };

Relevance

⭐⭐⭐ High
Very similar precedent: they accepted parsing raw stdout to preserve structured provider error
details.
PR-#19

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The helper prints JSON on stdout and signals failure via exit code 1; parseProviderJson does not
parse stdout for non-zero exit, and commandTelemetryHealth returns only parsed.error, dropping the
helper’s bounded failure message.
bin/ultimate-agent-stack.mjs[4508-4528]
bin/ultimate-agent-stack.mjs[5385-5467]
scripts/telemetry-readonly.mjs[72-84]
scripts/telemetry-readonly.mjs[439-445]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`telemetry-health` discards the telemetry helper’s structured failure reason when the helper exits with status 1 (its normal failure mode). This results in generic errors like "<provider> read-only health check failed" instead of the helper’s bounded reason.

### Issue Context
`scripts/telemetry-readonly.mjs` always writes JSON to stdout, and returns exit code 1 when `result.ok` is false. However, `parseProviderJson()` currently returns early on non-zero exit and `commandTelemetryHealth()` ignores the returned `detail`, so the bounded helper error is not surfaced.

### Fix Focus Areas
- bin/ultimate-agent-stack.mjs[4508-4528]
- bin/ultimate-agent-stack.mjs[5385-5467]
- scripts/telemetry-readonly.mjs[72-84]
- scripts/telemetry-readonly.mjs[439-445]

### Suggested fix
Implement one of:
1) Update `parseProviderJson()` to attempt `JSON.parse((result.raw_stdout ?? result.stdout).trim())` even when `!result.ok`; if parsing succeeds and yields an object with a bounded `error` field, return it in a structured way (while keeping redacted output for reporting).
2) Alternatively, in `commandTelemetryHealth()`, when `parsed.ok` is false, use `parsed.detail` (or parse `parsed.detail` as JSON and extract its `error`) as the returned provider error, instead of only `parsed.error`.

Add a unit test that stubs `runTelemetryReadonly()` (or exercises the helper) to return `ok:false` JSON on stdout and verifies `commandTelemetryHealth()` returns the bounded helper error message (not only the generic label failure).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. No retries in boundedJsonRequest 📜 Skill insight ☼ Reliability

Description

The new telemetry adapter performs third-party fetch calls without any retry policy or
circuit-breaker behavior, so transient provider failures can cause repeated immediate failures
instead of controlled backoff/short-circuiting. This violates the requirement that third-party API
calls implement timeout plus retry limits and circuit-breaker/fallback behavior.

Code

scripts/telemetry-readonly.mjs[R103-176]

+async function boundedJsonRequest({
+  provider,
+  url,
+  token,
+  headers = {},
+  method = "GET",
+  body,
+  fetchImpl,
+  signal,
+}) {
+  if (!validCredential(token)) {
+    return {
+      ok: false,
+      failure: boundedProviderFailure(
+        provider,
+        `${CREDENTIAL_ENVIRONMENTS[provider]} is missing or invalid`,
+      ),
+    };
+  }
+  try {
+    const response = await fetchImpl(url, {
+      method,
+      headers,
+      ...(body === undefined ? {} : { body }),
+      redirect: "error",
+      signal,
+    });
+    const capture = await readBoundedResponse(response);
+    if (!capture.ok) {
+      return {
+        ok: false,
+        failure: boundedProviderFailure(
+          provider,
+          `${provider} health response exceeded the bounded capture limit`,
+          response.status,
+        ),
+      };
+    }
+    let payload;
+    try {
+      payload = JSON.parse(capture.text);
+    } catch {
+      return {
+        ok: false,
+        failure: boundedProviderFailure(
+          provider,
+          `${provider} returned invalid JSON`,
+          response.status,
+        ),
+      };
+    }
+    if (!response.ok) {
+      return {
+        ok: false,
+        failure: boundedProviderFailure(
+          provider,
+          `${provider} health request failed`,
+          response.status,
+        ),
+      };
+    }
+    return { ok: true, payload };
+  } catch (error) {
+    return {
+      ok: false,
+      failure: boundedProviderFailure(
+        provider,
+        error?.name === "TimeoutError" || error?.name === "AbortError"
+          ? `${provider} health request timed out`
+          : `${provider} health request failed`,
+      ),
+    };
+  }
+}

Relevance

⭐⭐ Medium
They’ve enforced timeouts, but no clear precedent requiring retries/circuit-breakers; change is
policy/architecture sized.
PR-#8

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2395920 requires timeout, retry limits, and circuit-breaker behavior for
third-party API calls. In scripts/telemetry-readonly.mjs, boundedJsonRequest() makes a single
fetchImpl(url, ...) call and returns a failure on error/timeout without any retry loop/backoff or
circuit-breaker state, demonstrating the missing controls.

scripts/telemetry-readonly.mjs[103-176]
Skill: secure-launch

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`scripts/telemetry-readonly.mjs` introduces third-party API calls but does not implement any retry policy or circuit-breaker behavior.

## Issue Context
PR Compliance ID 2395920 requires third-party API calls to include timeout, retry limits, and circuit-breaker (or equivalent short-circuit/fallback) behavior.

## Fix Focus Areas
- scripts/telemetry-readonly.mjs[103-176]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

Previous review results

Review updated until commit 1ebea2b

Results up to commit 4d9feca ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (1)

1. ~~Availability skips hash check~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

commandCapabilities marks telemetry providers as available when the helper file exists and a
credential env var is set, but it does not verify the helper matches the CLI-pinned protected hash.
This can advertise a tampered helper as “available” even though telemetry-health will refuse to run
it via protectedProjectFileIssue().

Code

bin/ultimate-agent-stack.mjs[R4400-4404]

+                available:
+                  projectExists(target, TELEMETRY_READONLY_PATH) &&
+                  typeof process.env[credential] === "string" &&
+                  process.env[credential].length > 0,
+                external: true,

Relevance

⭐⭐⭐ High
Team often accepts integrity hardening to fail closed; availability should align with protected
helper hash enforcement.
PR-#15
PR-#30

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new capability availability predicate checks only file existence + env var, while the execution
path enforces the pinned helper hash; this mismatch can report availability for a helper that will
be blocked at runtime.
bin/ultimate-agent-stack.mjs[4386-4417]
bin/ultimate-agent-stack.mjs[7049-7078]
test/agent-stack.test.mjs[1579-1616]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`commandCapabilities()` reports telemetry adapters as `available` without checking helper integrity against the protected/pinned hash. This creates inconsistent behavior vs `telemetry-health`, which correctly refuses tampered helpers.

### Issue Context
The telemetry helper is explicitly pinned via `TELEMETRY_READONLY_SOURCE_HASH` and enforced by `protectedProjectFileIssue()`. Capability discovery should use the same trust predicate to avoid claiming a provider is usable when it will be rejected later.

### Fix Focus Areas
- bin/ultimate-agent-stack.mjs[4386-4417]
- bin/ultimate-agent-stack.mjs[7049-7078]
- test/agent-stack.test.mjs[1579-1616]

### Suggested fix
- In the telemetry section of `commandCapabilities(target)`, incorporate `protectedProjectFileIssue(target, TELEMETRY_READONLY_PATH)` into the `available` computation (e.g., require it to be null).
- Add/extend a regression assertion in the existing tampered-helper test to ensure `commandCapabilities(...).available.telemetry.posthog.available` is `false` when the helper is modified.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Health drops failure detail~~ ✓ Resolved 🐞 Bug ◔ Observability

Description

commandTelemetryHealth uses parseProviderJson, which does not parse helper JSON when the helper
exits non-zero; it then returns only a generic ${label} failed error. Since telemetry-readonly.mjs
always prints structured JSON (including a bounded error message) even when ok:false,
telemetry-health loses the specific failure reason (timeout/HTTP failure/bounds) and becomes hard to
troubleshoot.

Code

bin/ultimate-agent-stack.mjs[R5454-5464]

+    const parsed = parseProviderJson(
+      runTelemetryReadonly(target, provider),
+      `${provider.provider} read-only health check`,
+    );
+    if (!parsed.ok) {
+      return {
+        ok: false,
+        ...base,
+        live_check: "failed",
+        error: parsed.error,
+      };

Relevance

⭐⭐⭐ High
Very similar precedent: they accepted parsing raw stdout to preserve structured provider error
details.
PR-#19

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The helper prints JSON on stdout and signals failure via exit code 1; parseProviderJson does not
parse stdout for non-zero exit, and commandTelemetryHealth returns only parsed.error, dropping the
helper’s bounded failure message.
bin/ultimate-agent-stack.mjs[4508-4528]
bin/ultimate-agent-stack.mjs[5385-5467]
scripts/telemetry-readonly.mjs[72-84]
scripts/telemetry-readonly.mjs[439-445]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`telemetry-health` discards the telemetry helper’s structured failure reason when the helper exits with status 1 (its normal failure mode). This results in generic errors like "<provider> read-only health check failed" instead of the helper’s bounded reason.

### Issue Context
`scripts/telemetry-readonly.mjs` always writes JSON to stdout, and returns exit code 1 when `result.ok` is false. However, `parseProviderJson()` currently returns early on non-zero exit and `commandTelemetryHealth()` ignores the returned `detail`, so the bounded helper error is not surfaced.

### Fix Focus Areas
- bin/ultimate-agent-stack.mjs[4508-4528]
- bin/ultimate-agent-stack.mjs[5385-5467]
- scripts/telemetry-readonly.mjs[72-84]
- scripts/telemetry-readonly.mjs[439-445]

### Suggested fix
Implement one of:
1) Update `parseProviderJson()` to attempt `JSON.parse((result.raw_stdout ?? result.stdout).trim())` even when `!result.ok`; if parsing succeeds and yields an object with a bounded `error` field, return it in a structured way (while keeping redacted output for reporting).
2) Alternatively, in `commandTelemetryHealth()`, when `parsed.ok` is false, use `parsed.detail` (or parse `parsed.detail` as JSON and extract its `error`) as the returned provider error, instead of only `parsed.error`.

Add a unit test that stubs `runTelemetryReadonly()` (or exercises the helper) to return `ok:false` JSON on stdout and verifies `commandTelemetryHealth()` returns the bounded helper error message (not only the generic label failure).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. No retries in boundedJsonRequest 📜 Skill insight ☼ Reliability

Description

The new telemetry adapter performs third-party fetch calls without any retry policy or
circuit-breaker behavior, so transient provider failures can cause repeated immediate failures
instead of controlled backoff/short-circuiting. This violates the requirement that third-party API
calls implement timeout plus retry limits and circuit-breaker/fallback behavior.

Code

scripts/telemetry-readonly.mjs[R103-176]

+async function boundedJsonRequest({
+  provider,
+  url,
+  token,
+  headers = {},
+  method = "GET",
+  body,
+  fetchImpl,
+  signal,
+}) {
+  if (!validCredential(token)) {
+    return {
+      ok: false,
+      failure: boundedProviderFailure(
+        provider,
+        `${CREDENTIAL_ENVIRONMENTS[provider]} is missing or invalid`,
+      ),
+    };
+  }
+  try {
+    const response = await fetchImpl(url, {
+      method,
+      headers,
+      ...(body === undefined ? {} : { body }),
+      redirect: "error",
+      signal,
+    });
+    const capture = await readBoundedResponse(response);
+    if (!capture.ok) {
+      return {
+        ok: false,
+        failure: boundedProviderFailure(
+          provider,
+          `${provider} health response exceeded the bounded capture limit`,
+          response.status,
+        ),
+      };
+    }
+    let payload;
+    try {
+      payload = JSON.parse(capture.text);
+    } catch {
+      return {
+        ok: false,
+        failure: boundedProviderFailure(
+          provider,
+          `${provider} returned invalid JSON`,
+          response.status,
+        ),
+      };
+    }
+    if (!response.ok) {
+      return {
+        ok: false,
+        failure: boundedProviderFailure(
+          provider,
+          `${provider} health request failed`,
+          response.status,
+        ),
+      };
+    }
+    return { ok: true, payload };
+  } catch (error) {
+    return {
+      ok: false,
+      failure: boundedProviderFailure(
+        provider,
+        error?.name === "TimeoutError" || error?.name === "AbortError"
+          ? `${provider} health request timed out`
+          : `${provider} health request failed`,
+      ),
+    };
+  }
+}

Relevance

⭐⭐ Medium
They’ve enforced timeouts, but no clear precedent requiring retries/circuit-breakers; change is
policy/architecture sized.
PR-#8

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2395920 requires timeout, retry limits, and circuit-breaker behavior for
third-party API calls. In scripts/telemetry-readonly.mjs, boundedJsonRequest() makes a single
fetchImpl(url, ...) call and returns a failure on error/timeout without any retry loop/backoff or
circuit-breaker state, demonstrating the missing controls.

scripts/telemetry-readonly.mjs[103-176]
Skill: secure-launch

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`scripts/telemetry-readonly.mjs` introduces third-party API calls but does not implement any retry policy or circuit-breaker behavior.

## Issue Context
PR Compliance ID 2395920 requires third-party API calls to include timeout, retry limits, and circuit-breaker (or equivalent short-circuit/fallback) behavior.

## Fix Focus Areas
- scripts/telemetry-readonly.mjs[103-176]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Results up to commit c4ae28e ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Telemetry availability false-positive~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

commandCapabilities() marks telemetry providers as available when the credential env var is merely
non-empty, but the telemetry helper rejects credentials shorter than 8 chars or containing
CR/LF/NUL. This can mislead onboarding/automation into thinking telemetry is usable when
telemetry-health will always fail for that credential.

Code

bin/ultimate-agent-stack.mjs[R4403-4406]

+                available:
+                  telemetryHelperAvailable &&
+                  typeof process.env[credential] === "string" &&
+                  process.env[credential].length > 0,

Relevance

⭐⭐⭐ High
Correctness mismatch: capabilities “available” check should align with helper’s credential
validation; team accepts env-validation hardening.
PR-#29
PR-#26

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Capabilities uses only length > 0 to decide availability, while the helper’s validCredential()
requires length >= 8 and forbids CR/LF/NUL; the new test demonstrates a short key is considered
invalid by health.
bin/ultimate-agent-stack.mjs[4341-4407]
scripts/telemetry-readonly.mjs[94-120]
test/agent-stack.test.mjs[1513-1520]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`commandCapabilities()` reports telemetry providers as `available` when the provider helper is intact and the credential environment variable is a non-empty string. However, the shipped helper rejects credentials that are too short (< 8) or contain `\r`, `\n`, or `\0`, so capabilities can show `available: true` even though `telemetry-health` will fail immediately.

## Issue Context
- The helper’s credential validity rules are stricter than the capabilities “available” predicate.
- A test already demonstrates a failing case (`POSTHOG_PERSONAL_API_KEY = "short"`) for `telemetry-health`, but capabilities would still mark PostHog as available.

## Fix Focus Areas
- bin/ultimate-agent-stack.mjs[4396-4407]

### Suggested fix
Update the telemetry provider `available:` predicate to apply the same syntactic checks as the helper (min length >= 8, max length <= 8192 if desired, and reject CR/LF/NUL). This keeps capabilities aligned with actual health execution behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

samtay32 · 2026-07-29T08:25:47Z

/improve

qodo-code-review · 2026-07-29T08:31:11Z

Code review by qodo was updated up to the latest commit c4ae28e

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@bin/ultimate-agent-stack.mjs`:
- Around line 5463-5505: Bound the aggregate telemetry probe duration in the
provider results flow around runTelemetryReadonly and its callers commandStart
and commandDoctor. Use a shared overall deadline or shorter interactive
per-provider timeout so sequential provider checks cannot stall startup or
doctor commands for roughly three full probe timeouts; when the budget is
exhausted, return the existing repository-fallback result for remaining
providers.
- Around line 1777-1822: Extract the numeric-ID and bounded-identifier regexes
into shared constants, preserving the case-insensitive identifier behavior used
by the config validator. Replace the duplicated literals in
validateTelemetryProvider, parseTelemetrySpec, and
sanitizeTelemetryHealthResult, and update scripts/telemetry-readonly.mjs to
reuse the shared constants where available so all validation gates apply
identical rules.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3c5e3ec5-9dd4-4423-8541-f1e7b914c97b

📥 Commits

Reviewing files that changed from the base of the PR and between 8087765 and c4ae28e.

📒 Files selected for processing (21)

.codex-plugin/plugin.json
README.md
STARTER_PROMPT.md
assets/project-template/.agent-stack/HANDOFF.md
assets/project-template/AGENTS.md
bin/ultimate-agent-stack.mjs
docs/ADAPTERS.md
docs/ARCHITECTURE.md
docs/OPERATING_MANUAL.md
docs/SOURCES_AND_TRADEOFFS.md
docs/TRUST.md
evals/scenarios.json
package.json
scripts/packed-smoke.mjs
scripts/telemetry-readonly.mjs
skills/use-project-telemetry/SKILL.md
skills/use-project-telemetry/references/telemetry-providers.md
test/agent-stack.test.mjs
test/maintenance.test.mjs
test/skill-eval.test.mjs
test/telemetry-readonly.test.mjs

samtay32 · 2026-07-29T08:34:04Z

/improve

qodo-code-review · 2026-07-29T08:37:07Z

Code review by qodo was updated up to the latest commit 9d1da73

samtay32 · 2026-07-29T08:42:07Z

/improve

qodo-code-review · 2026-07-29T08:46:26Z

Code review by qodo was updated up to the latest commit a594884

samtay32 · 2026-07-29T08:52:25Z

/improve

qodo-code-review · 2026-07-29T08:55:26Z

Code review by qodo was updated up to the latest commit 1ebea2b

feat: add reviewed telemetry connection adapters

4d9feca

qodo-code-review Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread scripts/telemetry-readonly.mjs

Comment thread bin/ultimate-agent-stack.mjs

Comment thread bin/ultimate-agent-stack.mjs Outdated

fix: align telemetry capability health

c4ae28e

qodo-code-review Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread bin/ultimate-agent-stack.mjs Outdated

coderabbitai Bot requested changes Jul 29, 2026

View reviewed changes

Comment thread bin/ultimate-agent-stack.mjs

Comment thread bin/ultimate-agent-stack.mjs

fix: validate telemetry capability credentials

9d1da73

fix: bound aggregate telemetry health

a594884

coderabbitai Bot approved these changes Jul 29, 2026

View reviewed changes

qodo-code-review Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread test/agent-stack.test.mjs

Comment thread bin/ultimate-agent-stack.mjs

fix: reject untyped telemetry scopes

1ebea2b

samtay32 merged commit 4fac728 into main Jul 29, 2026
10 of 16 checks passed

samtay32 deleted the codex/telemetry-provider-adapters branch July 29, 2026 08:57

qodo-code-review Bot mentioned this pull request Aug 2, 2026

Harden mechanical activation and independent-review receipts #40

Merged

Conversation

samtay32 commented Jul 29, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

samtay32 commented Jul 29, 2026

Uh oh!

qodo-code-review Bot commented Jul 29, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

Uh oh!

Uh oh!

Uh oh!

samtay32 commented Jul 29, 2026

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 29, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

samtay32 commented Jul 29, 2026

Uh oh!

qodo-code-review Bot commented Jul 29, 2026

Uh oh!

samtay32 commented Jul 29, 2026

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 29, 2026

Uh oh!

samtay32 commented Jul 29, 2026

Uh oh!

qodo-code-review Bot commented Jul 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

samtay32 commented Jul 29, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading

qodo-code-review Bot commented Jul 29, 2026 •

edited

Loading