feat(telemetry): anonymous PostHog usage telemetry with funnel instrumentation

[Q00]

Closes #1909

Summary

• Anonymous, opt-out usage telemetry (PostHog) so the install funnel, interview → seed → run → evolve conversion, terminal-CLI vs in-agent entry ratio, and per-backend success rates become measurable across all 13 runtimes
• Single chokepoint instrumentation: MCPServerAdapter.call_tool (covers Claude/Codex stdio, legacy serve wrapper, and the codex ooo intercept), plus CLI callback and install.sh pings; polling tools sampled 1/50 with a sample_rate re-weighting property
• Trust contract shipped in the same commit: TELEMETRY.md (event whitelist, declared uses incl. public aggregate stats, fixed counting rule "1 active user = 1 anonymous ID with ≥1 successful run/week — installs/CI/retries don't count", k-anonymity floor, append-only changelog), one-time first-run notice, 3 opt-out paths (DO_NOT_TRACK, OUROBOROS_TELEMETRY=0, config telemetry.enabled)

Design notes

• stdlib-only transport (urllib + daemon thread, bounded queue) — no new dependency, never raises, never blocks a command, drops events offline
• Identity is a random UUID in ~/.ouroboros/telemetry.json (no PII, no fingerprinting; removed by ouroboros uninstall); the embedded phc_ key is PostHog's public write-only project key, safe by design in OSS
• ouroboros mcp serve boots are captured as mcp_serve_started (session-attach denominator) and excluded from terminal-CLI counts so host-spawned serves don't inflate direct usage
• In-process servers (ooo auto) and detached job workers bypass call_tool, so internal automation cannot pollute the cli-vs-agent ratio
• tests/conftest.py force-disables telemetry suite-wide; telemetry unit tests neutralize the embedded key and inject a fake transport

Checks passed

• ruff lint + format (9 files)
• mypy (7 source files)
• pytest: telemetry 20/20, config 499, cli + mcp adapter — no new failures vs clean tree (combined-run cross-contamination failures reproduce identically on main; 2 known pre-existing failures deselected)

Test plan

• End-to-end verified live: smoke event sent through the real module and confirmed queryable in PostHog via HogQL
• After merge/release: watch the Overview dashboard (Lifecycle/funnel/reliability) populate from real installs; first-run notice appears once per machine

Credit

@code-yeongyu suggested this Posthog to track user data in Ouroboros funnel.

🤖 Generated with Claude Code

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	ee4d2c6ffc319ea9eef4e3f622f9ad87e4e2226e
Request ID	req_1785984259_616-retry-1
Review record	fafc457a-5fa1-4293-ad0e-5754d4118201

What Improved

• Adds a centralized, bounded, stdlib-only telemetry transport with anonymous UUID identity, event batching, polling sampling, and failure isolation.
• Adds telemetry configuration, opt-out documentation, CLI/MCP/install instrumentation, and hermetic telemetry tests.

Issue Requirements

Requirement	Status
Anonymous opt-out telemetry across all supported runtimes	Partially met — core instrumentation exists, but hostile project environments can force or redirect it and several execution paths report only submission outcomes.
Measure install, interview → seed → run → evolve, entry-surface, and per-backend success funnels	Partially met — funnel events exist, but asynchronous start receipts are treated as run success and validation/security failures are omitted.
Use MCPServerAdapter.call_tool as a single instrumentation chokepoint, with CLI and installer events	Partially met — successful handler calls are covered, but pre-handler SDK validation and adapter early returns bypass capture.
Sample high-frequency polling tools at 1/50 with reweighting metadata	Partially met — implemented for the explicit set, but the set omits public status-query tools.
Ship a truthful telemetry trust contract and event whitelist	Partially met — the event table is useful, but runtime behavior violates opt-out/counting promises and the source-location statement is inaccurate.
Support DO_NOT_TRACK, OUROBOROS_TELEMETRY=0, and telemetry.enabled: false as complete opt-outs	Not met — installer telemetry ignores the config opt-out, malformed config enables collection, and project .env can override the persisted opt-out.
Show a one-time first-run notice	Partially met — the CLI notice is stateful, but the installer sends its first event before notice and failed installs may never show one.
Use a random stable UUID and remove it on normal uninstall	Met — the UUID is random and stored under ~/.ouroboros, which default uninstall removes with the data directory.
Use stdlib-only, bounded, best-effort transport that never raises into commands	Met for exception isolation and queue bounds; synchronous config/state access remains outside the background transport.
Keep tests from sending production telemetry	Met — suite-wide disablement and fake transport/key injection are present.

Prior Findings Status

No prior ouroboros-agent review rounds were present. No previous findings required maintenance, modification, or withdrawal.

Blockers

#	File:Line	Severity	Finding
1	src/ouroboros/telemetry.py:99	BLOCKING	The PostHog API key and destination are read from os.environ, but importing config.loader loads the current project's untrusted .env into that same environment without denying OUROBOROS_POSTHOG_HOST, OUROBOROS_POSTHOG_API_KEY, or OUROBOROS_TELEMETRY. A cloned repository can therefore force telemetry on despite telemetry.enabled: false and redirect the stable anonymous ID plus runtime metadata to an attacker-controlled or local endpoint. A subprocess probe reproduced is_enabled() == True and an attacker-provided host while the user config explicitly disabled telemetry. These keys must be denied from project .env input or resolved exclusively from a trusted environment source.
2	src/ouroboros/config/loader.py:1436	BLOCKING	The config opt-out fails open: any ConfigError from the full configuration returns True. An explicit telemetry.enabled: false is therefore ignored whenever an unrelated stale or invalid setting makes OuroborosConfig validation fail. A focused probe with telemetry disabled and an invalid logging value returned True. Privacy opt-outs must remain effective independently of unrelated configuration validity, or configuration errors must disable telemetry fail-closed.
3	scripts/install.sh:100	BLOCKING	Installer telemetry implements only the two environment-variable opt-outs and never reads ~/.ouroboros/config.yaml, contradicting the documented promise that any of the three opt-out paths disables telemetry completely. It also emits install_started at line 191 before showing any notice at line 703; failure exits can therefore send an event without ever displaying the notice. A probe with telemetry.enabled: false captured a real /capture/ request and no notice. Honor the persisted opt-out before the first ping and disclose collection before that ping.
4	src/ouroboros/telemetry.py:70	BLOCKING	ouroboros_start_execute_seed is classified as a successful run whenever its handler returns Result.ok, but that handler explicitly returns immediately after queuing background work; the job can later fail, be cancelled, or fail verification. No telemetry is emitted from the durable terminal job boundary. Consequently the documented “successful verified run” weekly-active-user rule and per-backend success rates cannot be computed truthfully and will count queue acceptance as product success. Emit terminal outcome telemetry from the durable job/execution completion boundary and keep start receipts distinct from successful verified runs.

Follow-up Findings

• src/ouroboros/mcp/server/adapter.py:970 [warning] The claimed single call_tool chokepoint does not instrument every invocation or failure. Unknown tools and security-check failures return before capture_tool_call, while SDK schema validation at lines 106–118 rejects malformed/missing arguments before MCPServerAdapter.call_tool is entered at all. These common failure classes disappear from command_run, biasing the advertised success-rate metric. Instrument the outer request envelope or ensure every return and validation failure emits exactly one sanitized event, with regression tests preventing duplicates.
  | # | File:Line | Priority | Confidence | Suggestion |
  |---|-----------|----------|------------|------------|
  | 1 | src/ouroboros/telemetry.py:47 | Medium | Medium | Audit the polling roster against all public read-only status tools. ouroboros_lineage_status and ouroboros_project_status are omitted even though status tools can be repeatedly queried, so the promised 1/50 sampling policy is not structurally complete. |

Non-blocking Suggestions

| 1 | TELEMETRY.md:89 | Documentation | “If it's not in those two places, it isn't collected” is inaccurate because collection triggers and properties also live in cli/main.py, cli/commands/mcp.py, and mcp/server/adapter.py. List all instrumentation call sites so the privacy audit guidance is truthful. |

Test Coverage Notes

• tests/unit/test_telemetry.py: 20 passed.
• tests/unit/config/test_loader_env.py: 179 passed.
• tests/unit/scripts/test_install_runtime_selection.py: 29 passed.
• tests/unit/mcp/server/test_adapter.py: 64 passed, 5 skipped, and 10 failed because the review environment lacks the optional mcp package; failures were confined to SDK/serve tests requiring that extra.
• Independent probes reproduced project .env endpoint redirection and forced enablement, malformed-config fail-open behavior, and installer transmission despite telemetry.enabled: false.
• New tests exercise the telemetry module directly but do not cover installer opt-out/notice ordering, hostile project .env keys, malformed persisted config, adapter early-return failures, or durable background-job outcomes.

Design Notes

Centralizing serialization and transport in one module is a sound direction, but telemetry is a cross-cutting trust boundary. Enablement, endpoint selection, and success classification currently have multiple inconsistent sources of truth.

Design / Roadmap Gate

The changed boundary handles persistent identity, operator-owned opt-out state, untrusted project environment values, MCP request classification, and durable asynchronous execution. Current behavior is not fail-closed for privacy preferences, permits project-controlled destination redirection, and cannot implement the declared verified-success counting rule. These are contract and security failures rather than cosmetic telemetry inaccuracies.

Directional Notes

Review focused on executable trust-contract parity across CLI, installer, MCP, configuration, hostile environment input, and durable background-job completion. Maintainer memory was used only to direct inspection; each blocker above was independently confirmed from the snapshot or focused probes.

Test Coverage

• tests/unit/test_telemetry.py: 20 passed.
• tests/unit/config/test_loader_env.py: 179 passed.
• tests/unit/scripts/test_install_runtime_selection.py: 29 passed.
• tests/unit/mcp/server/test_adapter.py: 64 passed, 5 skipped, and 10 failed because the review environment lacks the optional mcp package; failures were confined to SDK/serve tests requiring that extra.
• Independent probes reproduced project .env endpoint redirection and forced enablement, malformed-config fail-open behavior, and installer transmission despite telemetry.enabled: false.
• New tests exercise the telemetry module directly but do not cover installer opt-out/notice ordering, hostile project .env keys, malformed persisted config, adapter early-return failures, or durable background-job outcomes.

Merge Recommendation

Do not merge until telemetry control variables are protected from untrusted .env input, every persisted opt-out remains fail-closed across installer and malformed-config paths, notice precedes installer collection, and successful runs are measured at durable terminal verification rather than background-job submission. Add integration tests at each corrected boundary.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: ee4d2c6
request_id: req_1785984259_616-retry-1
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

Broad direction review after the requested-changes loop:

The bounded, asynchronous, failure-isolated telemetry foundation is a strong start, and the product-learning goal fits Ouroboros. The current trust and metric contracts do not yet fit the project direction, though:

• project-controlled .env can override the telemetry endpoint, key, and opt-out;
• unrelated configuration errors fall back to telemetry-on;
• installer emission can happen before notice or persisted opt-out is honored;
• queued execution is counted as verified success;
• early adapter failures are omitted, biasing reliability metrics.

Those behaviors conflict with the fail-closed local trust boundary and durable-evidence semantics. Please pause implementation and define one trusted telemetry-control resolver plus distinct submission and terminal-outcome events. Then prove opt-out, malformed-config, installer ordering, and failure-path behavior with integration tests. The existing queue and failure-isolation work should remain reusable once those contracts are explicit.

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	ee4d2c6ffc319ea9eef4e3f622f9ad87e4e2226e
Request ID	req_1786109429_685
Review record	36ef418a-5c51-4706-9e0e-43b63774713f

What Improved

• Adds a bounded, asynchronous, failure-isolated telemetry transport with no new runtime dependency.
• Centralizes MCP command instrumentation, samples high-frequency polling tools, and documents the proposed collection contract.
• Adds focused unit coverage for event shape, sampling, opt-out environment flags, identity persistence, first-run notice, and transport failures.

Issue Requirements

Requirement	Status
Anonymous opt-out telemetry across supported runtimes	Partially met — collection exists, but project-controlled input can re-enable or redirect it and the installer ignores persisted opt-out.
Measure install and interview → seed → run → evolve funnels	Partially met — events exist, but run submission is conflated with verified completion.
Measure terminal-CLI versus in-agent entry ratio	Partially met — CLI/MCP sources and MCP serve events are separated, but the surrounding trust contract remains unsafe.
Measure per-backend success rates	Not met — queued jobs are reported as successes and pre-handler failures are omitted.
Instrument MCP through a single chokepoint	Partially met — normal handler outcomes are captured, but unknown-tool and security failures bypass instrumentation.
Sample polling tools at 1/50 with a reweighting property	Met.
Publish an event whitelist, usage policy, counting rule, anonymity floor, and changelog	Met as documentation, but the verified-success counting rule is not executable with the emitted events.
Provide a one-time first-run notice	Partially met — the Python CLI notice is persisted, but installer events are emitted before the installer notice.
Honor DO_NOT_TRACK, OUROBOROS_TELEMETRY=0, and telemetry.enabled: false	Not met across all surfaces and hostile/malformed inputs.
Keep telemetry asynchronous, bounded, non-blocking, and dependency-free	Met for the Python transport.
Use a random persistent UUID without machine fingerprinting	Met.
Remove telemetry identity during normal uninstall data cleanup	Met when ~/.ouroboros/ is removed; intentionally retained with --keep-data.

Prior Findings Status

The prior trust-contract and durable-success concerns are maintained based on fresh current-snapshot evidence. No contributor response or current implementation change addresses the project .env authority, malformed-config fallback, installer ordering/opt-out, queued-versus-terminal success, or omitted early adapter failures.

Blockers

#	File:Line	Severity	Finding
1	src/ouroboros/telemetry.py:99	BLOCKING	Project-controlled .env files can re-enable and redirect telemetry. _api_key() and _host() trust OUROBOROS_POSTHOG_API_KEY and OUROBOROS_POSTHOG_HOST, while get_telemetry_enabled() trusts OUROBOROS_TELEMETRY; importing the config loader loads the current project's .env, and none of these keys are denied by UNTRUSTED_ENV_DENYLIST. A focused probe with a user config containing telemetry.enabled: false and a project .env containing OUROBOROS_TELEMETRY=1, an attacker host, and attacker key produced enabled=True and selected the attacker destination. Operator-owned telemetry controls and destination configuration must come only from trusted process/home sources, with regression tests for hostile project environments.
2	src/ouroboros/config/loader.py:1436	BLOCKING	Any ConfigError, including malformed YAML or an unrelated invalid configuration field, falls back to telemetry enabled. A focused probe using a malformed ~/.ouroboros/config.yaml returned True. Because a privacy preference may be present but unreadable alongside the malformed content, this must fail closed rather than silently sending events. Add tests covering malformed YAML and unrelated validation errors.
3	scripts/install.sh:191	BLOCKING	The installer sends install_started before displaying the telemetry notice and never reads the persisted telemetry.enabled setting. A focused installer probe with ~/.ouroboros/config.yaml containing telemetry.enabled: false still emitted both install_started and install_completed; the notice was printed only afterward at lines 703–704. The installer must resolve the same trusted opt-out contract as the Python surfaces and present the notice before its first collection attempt.
4	src/ouroboros/mcp/server/adapter.py:1026	BLOCKING	ok=result.is_ok measures successful handler submission, not a successful verified run. For example, StartExecuteSeedHandler queues work and immediately returns Result.ok with a queued job at src/ouroboros/mcp/tools/execution_handlers.py:2862; durable completion or failure is recorded later by JobManager. No terminal-outcome telemetry exists, so failed background executions are currently counted as successful run funnel events, contradicting the documented weekly-active-user rule. Emit distinct submission and durable terminal-outcome events, and derive verified success only from terminal execution evidence.
5	src/ouroboros/mcp/server/adapter.py:970	BLOCKING	The claimed single chokepoint omits failures that occur before handler invocation. Unknown tools return at line 987 and security rejections return at line 1001, both before capture_tool_call(). A focused call to an unknown ouroboros_* tool returned an error while the capture spy remained empty. This biases reliability and backend failure rates toward success. Instrument all adapter exits through one finalization path and add coverage for unknown-tool and security-denial outcomes.

Follow-up Findings

#	File:Line	Priority	Confidence	Suggestion
None.

Non-blocking Suggestions

None.

Test Coverage Notes

• tests/unit/test_telemetry.py: 20 passed.
• tests/unit/scripts/test_install_runtime_selection.py: 29 passed, but contains no assertions for installer telemetry notice, opt-out, destination, or emission ordering.
• Config-loader and adapter run: 243 passed, 5 skipped, and 10 failed because the optional mcp package was not installed; the relevant non-SDK adapter tests otherwise executed.
• TestMCPServerAdapterTools: 12 passed and 1 unrelated optional-mcp dependency failure.
• Focused probes independently reproduced project .env re-enablement/redirection, malformed-config fail-open behavior, installer emission despite persisted opt-out, notice-after-emission ordering, and omitted unknown-tool failure telemetry.

Design Notes

The queue and boundary-oriented instrumentation are reusable, but telemetry authority and outcome semantics need a single trusted resolver plus durable terminal-event integration before the design is safe.

Design / Roadmap Gate

This change crosses persistent identity, installer, configuration, untrusted project environment, MCP security, and durable background-job boundaries. Current behavior permits destination redirection and opt-out override from a cloned repository, fails open when user configuration is malformed, emits installer events before notice, and lacks truthful terminal-success evidence. These violate the proposed telemetry contract and infrastructure trust model rather than representing optional metric refinements.

Directional Notes

Review focus followed the maintainer trust and replay-evidence posture: operator privacy controls must dominate project input, telemetry failures must remain non-blocking, and accepted or queued work must not be represented as verified terminal success. All blockers above are independently supported by the current snapshot and focused probes.

Test Coverage

• tests/unit/test_telemetry.py: 20 passed.
• tests/unit/scripts/test_install_runtime_selection.py: 29 passed, but contains no assertions for installer telemetry notice, opt-out, destination, or emission ordering.
• Config-loader and adapter run: 243 passed, 5 skipped, and 10 failed because the optional mcp package was not installed; the relevant non-SDK adapter tests otherwise executed.
• TestMCPServerAdapterTools: 12 passed and 1 unrelated optional-mcp dependency failure.
• Focused probes independently reproduced project .env re-enablement/redirection, malformed-config fail-open behavior, installer emission despite persisted opt-out, notice-after-emission ordering, and omitted unknown-tool failure telemetry.

Merge Recommendation

Do not merge until telemetry controls and destinations are resolved exclusively from trusted sources, malformed configuration fails closed, installer notice and opt-out checks precede collection, and separate submission plus durable terminal-outcome events provide complete failure accounting. Add integration tests for each corrected boundary.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: ee4d2c6
request_id: req_1786109429_685
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

Remediation verification — exact HEAD 871c00c7d25016f9b6134dd5cdbd60e24efad1ae

All prior ouroboros-agent findings were treated as acceptance criteria, including the earlier warning/documentation items.

Resolved boundaries:

• project .env can no longer set telemetry opt-out/destination/key authority;
• invalid or unreadable persisted config fails telemetry closed;
• installer honors persisted opt-out and persists/displays notice before first capture;
• asynchronous Start* results emit submission accepted, never execution ok;
• durable job terminals emit a separate deduplicated workflow_outcome;
• only completed evaluation with explicit final_approved=true is verified=true;
• unknown tools, security denial, SDK input validation, adapter error, output validation, and SDK conversion failures are captured exactly once;
• public lineage/project status polling tools are included in 1/50 sampling;
• TELEMETRY.md lists the complete trigger-site and active-user contracts.

Verification:

• focused telemetry/config/installer/adapter/job-manager suites: 543 passed
• added FAILED-terminal and SDK exactly-once regressions: 2 passed
• unreadable installer-config regression: 1 passed
• full suite: 17676 passed, 93 skipped, 1 xfailed; 3 order-dependent Copilot reasoning-effort failures occurred in unchanged code and pass independently on both this worktree and exact main 099898e8 (5 passed on each)
• mypy src/ouroboros: 532 files, clean
• Ruff format/check, module-size, auto-boundary, max-turns-envelope, bash -n scripts/install.sh, and git diff --check: clean

Direction gate: CONDITIONAL_PASS resolved to implementation-complete pending exact-head bot review and independent verification. This PR remains HOLD while aggregate review is CHANGES_REQUESTED; no merge/approval is requested from this worker.

[Q00]

@ouroboros-agent please re-review exact HEAD 871c00c

[Q00]

Resolved the two independently reproduced exact-HEAD blockers on 96cf738411fc94f0ba512a852c08cdbd965a2429 (latest origin/main 099898e8ab91dd7a2f7e346bb0a9b289c9a9c17b is an ancestor).

Changes:

• workflow_outcome.verified now requires all three conditions: normalized terminal status is completed, job type is evaluate, and final_approved is True (literal bool).
• Added regressions for failed/cancelled/interrupted evaluations carrying stale final_approved=True, completed non-evaluate work, missing/false approval, and truthy non-bool approval.
• Replaced the installer's telemetry-block-only awk parser with full YAML + OuroborosConfig validation in an isolated Python interpreter. Any malformed YAML, schema-invalid unrelated field, unreadable config, or unavailable validator fails closed. A genuinely absent config retains the documented default-on behavior.
• Preserved DO_NOT_TRACK precedence and notice-before-first-capture ordering; explicit persisted telemetry.enabled: false remains a complete opt-out.

Verification:

• bash -n scripts/install.sh
• shellcheck scripts/install.sh
• uv run --extra mcp pytest -q tests/unit/test_telemetry.py tests/unit/scripts/test_install_runtime_selection.py → 70 passed
• uv run --extra mcp pytest -q tests/unit/config/test_loader_env.py tests/unit/mcp/test_job_manager.py tests/unit/mcp/server/test_adapter.py → 482 passed
• post-shell-fix installer rerun → 37 passed
• uv run ruff check src/ tests/ → passed
• uv run mypy src/ouroboros → 532 source files, no issues
• git diff --check → clean

No merge performed.

@ouroboros-agent please re-review exact HEAD 96cf738

[Q00]

Independent exact-head re-verification — HOLD / FAIL

Verified HEAD: 96cf738411fc94f0ba512a852c08cdbd965a2429

The two returned blockers are substantially fixed: terminal evaluation outcomes are truthful, and the installer now validates the full configuration fail-closed while preserving genuinely absent default-on plus notice-before-capture. A remaining privacy bypass is blocking.

Blocker: dangling config symlink fails open

Both runtime surfaces treat a dangling ~/.ouroboros/config.yaml symlink as if the user had no configuration:

• scripts/install.sh:104: [ -e "$f" ] || return 0
• src/ouroboros/config/loader.py: Path.exists() == False

Reproduction:

1. Create ~/.ouroboros/config.yaml as a symlink to a missing target.
2. The copied remote-installer harness displays/persists the notice and emits both install_started and install_completed.
3. Python reports os.path.lexists=True, Path.exists=False, get_telemetry_enabled()=True, and telemetry.is_enabled()=True.

Expected: this is invalid/unreadable configuration, not genuinely absent configuration, so both installer and Python collection must remain disabled. Use lexists/-L-aware existence semantics and add exact regressions on both surfaces without weakening the truly absent default-on case.

Other gate status

• Required terminal-outcome matrix: PASS.
• Installer full-config malformed/unrelated-invalid/unreadable/explicit-false/no-validator and DNT/env precedence: PASS.
• Installed-entrypoint schema-validator fallback: PASS.
• Ruff lint: PASS, but format check fails on src/ouroboros/telemetry.py and tests/unit/scripts/test_install_runtime_selection.py; exact Ruff CI is red.
• Python 3.14 CI also failed the separate test(evolution): make generation lease fencing deterministic under Python xdist #1933 revocation test flake; do not mask it by rerun.
• No exact-head formal bot review exists; aggregate remains CHANGES_REQUESTED.

No approval or merge was performed.

[Q00]

Resolved the dangling-config-symlink privacy bypass on exact HEAD 28f2eee3e47c49e5eb477841158a024737ba37f9 (latest origin/main 099898e8ab91dd7a2f7e346bb0a9b289c9a9c17b is an ancestor).

Changes:

• scripts/install.sh now distinguishes a genuinely absent config from a dangling symlink using -L; dangling config state continues through the unreadable/fail-closed path.
• get_telemetry_enabled() likewise distinguishes Path.exists() == false for absence from Path.is_symlink() == true; a missing symlink target reaches load_config() and returns disabled on ConfigError.
• Added a Python regression proving both get_telemetry_enabled() and telemetry.is_enabled() are false for a dangling ~/.ouroboros/config.yaml.
• Added a copied standalone installer regression with no adjacent source tree: a dangling config symlink emits no notice, creates no telemetry identity, and performs no capture.
• The copied-installer genuine-absence test still proves disclosed default-on and notice-before-capture ordering.
• The curl test probe now separates the installer's pre-notice PyPI metadata lookup from PostHog /capture/ calls, so ordering assertions cover only collection.
• Ran Ruff formatting on the two files called out by the verifier; exact format check is now green locally.

Verification:

• uv run pytest -q tests/unit/test_telemetry.py tests/unit/scripts/test_install_runtime_selection.py → 72 passed
• uv run --extra mcp pytest -q tests/unit/config/test_loader.py tests/unit/config/test_loader_env.py tests/unit/mcp/test_job_manager.py → 532 passed
• uv run ruff format --check src/ tests/ → 1,229 files formatted
• uv run ruff check src/ tests/ → passed
• uv run mypy src/ouroboros → 532 source files, no issues
• bash -n scripts/install.sh, shellcheck scripts/install.sh, and git diff --check → passed

I did not manually rerun or mask the separate #1933 lease-test flake. No approval or merge performed.

@ouroboros-agent please re-review exact HEAD 28f2eee

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	28f2eee3e47c49e5eb477841158a024737ba37f9
Request ID	req_1786129068_854
Review record	ebc3c15c-f400-43c4-af85-31f066766aa2

What Improved

• Adds bounded, asynchronous, failure-isolated telemetry with centralized event serialization and an explicit public data contract.
• Separates asynchronous submission receipts from deduplicated durable terminal outcomes, requiring completed evaluation plus literal final_approved=true for verified success.
• Protects telemetry controls from project-local .env, handles dangling configuration symlinks fail-closed, samples polling tools, and captures MCP failures exactly once.

Issue Requirements

Requirement	Status
Anonymous, opt-out PostHog telemetry across install, CLI, MCP, funnel, and backend surfaces	Partially met — instrumentation exists, but two declared persistent opt-out paths can be bypassed by installer/runtime precedence differences.
Single MCP chokepoint plus CLI and installer instrumentation	Met
Polling tools sampled 1/50 with sample_rate weighting	Met
Public trust contract with event whitelist, declared uses, fixed active-user rule, k-anonymity floor, and changelog	Met
Three complete opt-out paths with operator-owned control authority	Not met — explicit enable bypasses persisted opt-out/fail-closed configuration, and the installer ignores trusted ~/.ouroboros/.env.
One-time notice before first collection	Met for enabled paths covered by tests, including the standalone installer
Stdlib-only, bounded, non-blocking, failure-isolated transport	Met
Random UUID identity without machine fingerprinting or payload content	Met
Submission acceptance must not be reported as verified execution success	Met
Durable terminal outcomes and verified evaluation success must be truthful and deduplicated	Met
Project-controlled .env must not control telemetry enablement, key, or destination	Met

Prior Findings Status

Most earlier trust and durability concerns are resolved in the current snapshot: project-local telemetry overrides are denied, ordinary malformed configuration and dangling symlinks fail closed, notice precedes installer capture, submissions are distinct from terminal outcomes, verified success requires truthful terminal evidence, and adapter failures are captured exactly once. The privacy-control concern is maintained in narrower form because current evidence shows explicit-enable precedence and the installer’s omission of trusted user .env still bypass declared operator opt-outs.

Blockers

#	File:Line	Severity	Finding
1	src/ouroboros/config/loader.py:1458	BLOCKING	OUROBOROS_TELEMETRY=1 returns True before inspecting persisted configuration, so it overrides both telemetry.enabled: false and malformed/unreadable configuration. The installer repeats this bypass at scripts/install.sh:193. I reproduced both Python and installer paths: with either a persisted opt-out or malformed YAML plus OUROBOROS_TELEMETRY=1, telemetry was enabled, the notice appeared, and both installer events were captured. This contradicts TELEMETRY.md:42 (“Any one … disables telemetry completely”), the fail-closed contract at TELEMETRY.md:62, and the contributor’s stated complete persisted opt-out. Resolve all disabling sources before accepting an explicit enable, and add runtime plus installer regressions for this precedence combination.
2	scripts/install.sh:98	BLOCKING	The installer reads telemetry controls and destination only from its inherited process environment; unlike the application loader at src/ouroboros/config/loader.py:256, it never reads the trusted ~/.ouroboros/.env declared at TELEMETRY.md:59. I reproduced an installation with ~/.ouroboros/.env containing OUROBOROS_TELEMETRY=0: the installer still displayed the notice and emitted install_started and install_completed. A destination override in that file is likewise ignored, causing installer data to go to the embedded PostHog project while later application events use the operator-selected destination. Parse the allowlisted telemetry keys from the trusted user env file with real-process-environment precedence before any notice or capture, and cover opt-out and destination behavior in the copied-installer tests.

Follow-up Findings

#	File:Line	Priority	Confidence	Suggestion
None.

Non-blocking Suggestions

| 1 | src/ouroboros/telemetry.py:403 | Documentation/Packaging | The first-run notices direct installed users to a bare TELEMETRY.md, but the built wheel does not contain that file and the installer is not retained after curl \| bash. Use a stable public URL in the notice or package and expose the document so users can actually inspect the detailed contract before later collection. |

Test Coverage Notes

• Ran the focused telemetry, configuration environment, MCP adapter, job-manager, and installer suites with /root/.local/bin/uv: 553 passed, 1 skipped.
• Ruff lint and focused format checks passed; bash -n scripts/install.sh passed.
• Direct Python probes confirmed OUROBOROS_TELEMETRY=1 overrides both persisted false and malformed configuration.
• Copied-installer probes confirmed the same override and confirmed that ~/.ouroboros/.env opt-out is ignored.
• Built the wheel successfully and confirmed it contains no TELEMETRY.md.

Design Notes

The centralized Python telemetry module and thin MCP boundary are well structured, and durable outcome observation is attached at the correct persistence boundary. The remaining architectural weakness is duplicated control resolution: Python and the standalone installer implement different trusted-source and precedence semantics.

Design / Roadmap Gate

The change crosses persistent identity, user configuration, trusted environment, installer, MCP, and durable job boundaries. Runtime outcome semantics, replay deduplication, failure isolation, and project-env compatibility now fit the infrastructure design. Merge remains blocked because the public “any one opt-out wins” contract is not consistently executable: explicit enable bypasses persisted state, and the installer omits one declared trusted persistent control source.

Directional Notes

Review focused on operator-owned privacy controls, fail-closed malformed state, truthful durable outcomes, and contract parity across installer, CLI, MCP, and background workers. Maintainer memory only guided those checks; both blockers were independently reproduced from the current snapshot.

Test Coverage

• Ran the focused telemetry, configuration environment, MCP adapter, job-manager, and installer suites with /root/.local/bin/uv: 553 passed, 1 skipped.
• Ruff lint and focused format checks passed; bash -n scripts/install.sh passed.
• Direct Python probes confirmed OUROBOROS_TELEMETRY=1 overrides both persisted false and malformed configuration.
• Copied-installer probes confirmed the same override and confirmed that ~/.ouroboros/.env opt-out is ignored.
• Built the wheel successfully and confirmed it contains no TELEMETRY.md.

Merge Recommendation

Do not merge until a single precedence contract is applied across Python and install.sh: persisted opt-out and invalid configuration must remain fail-closed even with OUROBOROS_TELEMETRY=1, and the installer must honor allowlisted telemetry controls from ~/.ouroboros/.env. Add focused regressions for both boundaries.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: 28f2eee
request_id: req_1786129068_854
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

@ouroboros-agent Both blockers from review ebc3c15c are addressed in d072f5c6a (merged latest main first — the job_manager.py conflict with #1940's reservation sets resolved by keeping both cleanup paths).

Blocker 1 — explicit enable overriding persisted opt-out (loader.py, install.sh): get_telemetry_enabled() now resolves every disabling source before anything else — DO_NOT_TRACK, OUROBOROS_TELEMETRY=0, persisted telemetry.enabled: false, and invalid/unreadable config all win unconditionally. OUROBOROS_TELEMETRY=1 is never an override; with no disabling source and a genuinely absent config it is simply the default-on path. The installer's _telemetry_enabled drops its =1 short-circuit and always runs _telemetry_config_allows. Regressions: test_explicit_enable_cannot_override_{persisted_opt_out,malformed_config} (Python resolver, both get_telemetry_enabled and telemetry.is_enabled) and test_installer_explicit_enable_cannot_override_{persisted_opt_out,malformed_config} (installer, notice + capture both suppressed). TELEMETRY.md now states this precedence explicitly.

Blocker 2 — installer ignoring trusted ~/.ouroboros/.env: install.sh now parses exactly the four allowlisted telemetry keys (DO_NOT_TRACK, OUROBOROS_TELEMETRY, OUROBOROS_POSTHOG_API_KEY, OUROBOROS_POSTHOG_HOST) from ~/.ouroboros/.env before any notice or capture, with real-process-environment precedence mirroring config/loader.py (an already-set process value — including set-but-empty — is never overridden; inline # comments on unquoted values are stripped so OUROBOROS_TELEMETRY=0 # off still opts out). Copied-installer regressions: test_installer_honors_user_env_opt_out, test_installer_honors_user_env_destination_override, test_installer_process_env_wins_over_user_env_file.

Non-blocking suggestion: both first-run notices now point at the canonical GitHub URL for TELEMETRY.md instead of the bare filename, since the wheel does not package the file.

Verification: telemetry + installer + loader-env suites 276 passed; MCP adapter + job-manager suites 298 passed; ruff lint/format clean; mypy clean on touched sources; bash -n scripts/install.sh clean.

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	d072f5c6a7f0d7d33126a9e30edbce318e07960f
Request ID	req_1786217700_1032
Review record	aa625691-45d8-4728-9026-da24d2dec440

What Improved

• Adds centralized, sanitized telemetry instrumentation for CLI, MCP adapter, installer, and durable job outcomes.
• Separates asynchronous submission acceptance from terminal workflow outcomes and limits verified success to approved evaluations.
• Protects telemetry controls and destinations from project-local .env, fails closed on invalid configuration, and samples polling tools.

Issue Requirements

Requirement	Status
Anonymous, opt-out PostHog telemetry across installer, CLI, MCP, and runtime backends	Partially met — instrumentation exists, but the installer can bypass valid persisted opt-outs
Single MCP chokepoint plus CLI and installer instrumentation	Met
Polling tools sampled 1/50 with a reweighting property	Met
Three opt-out paths and consistent trusted-control precedence	Not met — quoted dotenv values with inline comments are not honored by the installer
One-time notice before first installer collection	Met for recognized configurations and controls
Fixed active-user rule based on verified terminal evaluation outcomes	Met
Asynchronous, failure-isolated transport that never blocks commands	Not met — the atexit flush adds a bounded but material synchronous exit delay
Random UUID identity with no machine fingerprinting or content collection	Met
MCP serve attachment denominator separated from direct terminal usage	Met

Prior Findings Status

The earlier explicit-enable precedence blocker is resolved: persisted opt-out, malformed configuration, and DO_NOT_TRACK now dominate OUROBOROS_TELEMETRY=1. Malformed/unreadable configuration, dangling symlinks, notice ordering, and durable terminal-outcome semantics also verify as corrected. The trusted ~/.ouroboros/.env concern is modified rather than withdrawn: the source is now read, but its hand-written parser still bypasses valid dotenv opt-outs containing quoted values followed by comments.

Blockers

#	File:Line	Severity	Finding
1	scripts/install.sh:121	BLOCKING	The installer’s trusted ~/.ouroboros/.env parser does not match the application’s python-dotenv semantics. A normal dotenv entry such as OUROBOROS_TELEMETRY="0" # persisted opt-out does not end with a quote, so it enters the unquoted branch and retains the quotes around 0; _telemetry_enabled then fails to recognize the opt-out and emits the notice and both install events. DO_NOT_TRACK="1" # off fails identically. I reproduced collection for both forms. This violates the documented guarantee that the installer honors the same trusted persistent controls as the Python resolver. Use a dotenv-compatible parser or correctly handle quoted values followed by comments, with regressions for both opt-out variables.

Follow-up Findings

• src/ouroboros/telemetry.py:265 [warning] Registering flush() with atexit makes short commands synchronously wait up to 1.5 seconds for telemetry, while _post() itself may remain blocked for the four-second HTTP timeout. A stalled fake transport increased a trivial process from approximately 0.17 seconds to 2.03 seconds. Therefore the daemon queue does not satisfy the PR’s public “telemetry never blocks a command” contract and can noticeably delay every short CLI invocation when PostHog or an operator-configured destination is unreachable. Remove the blocking exit flush or redesign delivery so process termination remains effectively non-blocking, and add a stalled-transport latency regression.
  | # | File:Line | Priority | Confidence | Suggestion |
  |---|-----------|----------|------------|------------|
  None.

Non-blocking Suggestions

| 1 | src/ouroboros/telemetry.py:37 | Documentation | The comment says an empty API key disables telemetry, but _api_key() at line 125 replaces an empty environment value with the embedded key. Either correct the comment or implement the stated behavior to avoid misleading future maintainers. |

Test Coverage Notes

• 531 passed for telemetry, loader environment, MCP adapter, and job-manager suites using uv --extra mcp.
• 42 passed, 1 skipped for the installer runtime-selection suite.
• Ruff checks and bash -n scripts/install.sh passed.
• Additional focused probes reproduced the quoted-inline-comment opt-out bypass and the approximately 1.5-second exit wait under a stalled telemetry transport; neither path is covered by the authored tests.

Design Notes

The adapter observer and durable job boundary are appropriately separated from MCP business logic, and the event semantics are substantially improved. The remaining failures are boundary-contract issues in installer parsing and process lifecycle behavior, not architectural objections to telemetry itself.

Design / Roadmap Gate

The PR crosses trusted environment, persistent identity, installer, CLI lifecycle, MCP, and durable-job boundaries. Durable outcome truthfulness, project-env destination protection, replay deduplication, and malformed-config handling now fit the infrastructure contract. Merge remains blocked because the installer still interprets one trusted control source differently from the application, and the exit lifecycle contradicts the declared non-blocking transport guarantee.

Directional Notes

Review focus followed the maintainer trust posture: operator privacy controls must remain authoritative across every entry surface, and advertised asynchronous instrumentation must not degrade command execution. Prior memory guided inspection only; both blockers were independently reproduced from the current snapshot.

Test Coverage

• 531 passed for telemetry, loader environment, MCP adapter, and job-manager suites using uv --extra mcp.
• 42 passed, 1 skipped for the installer runtime-selection suite.
• Ruff checks and bash -n scripts/install.sh passed.
• Additional focused probes reproduced the quoted-inline-comment opt-out bypass and the approximately 1.5-second exit wait under a stalled telemetry transport; neither path is covered by the authored tests.

Merge Recommendation

Do not merge until the installer accepts the same quoted/commented dotenv opt-outs as the Python loader and telemetry cannot delay process exit under a stalled destination. Add focused regressions for both corrected boundaries.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: d072f5c
request_id: req_1786217700_1032
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

@ouroboros-agent Both blockers from round req_1786217700_1032 are addressed in 8d418f5 (plus a main merge for the conflict):

1. Installer dotenv parser (scripts/install.sh) — fixed. Quoted values now follow python-dotenv semantics: the value runs to the first matching closing quote, and only whitespace and/or a # comment may follow it. OUROBOROS_TELEMETRY="0" # persisted opt-out and DO_NOT_TRACK="1" # off now parse to bare 0/1 and disable collection before any notice or event. An unclosed quote or trailing garbage after the closing quote is a parse error and skips the binding, matching python-dotenv's error-skip behavior. Regressions added for double-quoted, single-quoted, uncommented-quoted, unclosed-quote, and trailing-garbage forms of both opt-out variables (tests/unit/scripts/test_install_runtime_selection.py, 420 passed).

2. Blocking exit flush (src/ouroboros/telemetry.py) — removed. atexit.register(flush) is gone; the worker remains a daemon thread and process termination never waits on telemetry, even against a stalled destination. Events still queued at exit are dropped, which the module docstring, flush() docstring, and TELEMETRY.md now state explicitly. flush() stays available for explicit callers only (tests; no production call sites). Added a stalled-transport regression: a subprocess pointed at a local accept-but-never-respond socket queues an event and must exit in well under the old ~2s blocking bound (tests/unit/test_telemetry.py::TestExitDoesNotBlock, 38 passed).

3. Non-blocking suggestion — comment corrected. The embedded-key comment now says an empty constant (e.g. stripped in a fork) disables telemetry, while a blank/unset OUROBOROS_POSTHOG_API_KEY env var falls back to the embedded key.

Verification: uv run --extra mcp pytest telemetry + loader-env + adapter + job-manager suites all green (532 passed), installer suite 420 passed, ruff check/format --check clean, bash -n scripts/install.sh OK.

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	8d418f597719edc1738061376cd7a819e4823512
Request ID	req_1786309679_1218
Review record	2dd6192e-7e8d-4ed8-bae8-916027dbb81b

What Improved

• Adds bounded, failure-isolated telemetry transport with documented event and privacy contracts.
• Separates asynchronous submission receipts from durable workflow outcomes and records adapter failures exactly once.
• Protects telemetry controls and destinations from project-controlled .env values, fails closed on invalid configuration, and removes exit-time flushing.

Issue Requirements

Requirement	Status
Provide anonymous opt-out usage telemetry across supported runtimes	Partially met — instrumentation exists, but installer opt-out parity remains incomplete.
Instrument MCP calls through a shared boundary plus direct CLI and installer entrypoints	Met
Sample high-frequency polling tools at 1/50 with a reweighting property	Met
Ship a documented trust contract, first-run notice, and three authoritative opt-out paths	Partially met — documented trusted .env forms accepted by the application can still be ignored by the installer.
Keep transport bounded, asynchronous, failure-isolated, and non-blocking at process exit	Met
Use one stable random UUID stored in ~/.ouroboros/telemetry.json	Not met — concurrent first use can produce multiple IDs.
Distinguish accepted asynchronous submissions from verified durable outcomes	Met
Prevent project-controlled .env files from changing telemetry authority or destination	Met

Prior Findings Status

Prior blockers concerning project .env authority, malformed configuration, installer notice ordering, submission-versus-terminal semantics, quoted/commented opt-outs, and exit-time blocking are withdrawn based on the current snapshot and passing regressions. The two blockers above are newly verified remaining failures in the same trust and identity contracts.

Blockers

#	File:Line	Severity	Finding
1	scripts/install.sh:111	BLOCKING	The installer still does not honor the same trusted ~/.ouroboros/.env opt-outs as the Python loader. Its hand-written parser recognizes only the literal prefix export , so valid python-dotenv forms such as export OUROBOROS_TELEMETRY=0 or a tab after export are skipped. The flag checks at lines 266–267 are also only partially case-insensitive: values such as OUROBOROS_TELEMETRY=OFF and DO_NOT_TRACK=YES disable application telemetry but not installer telemetry. Focused installer probes confirmed all four forms display the notice and emit install events. This violates the documented “any one opt-out wins” and cross-runtime parity contract. Normalize the full accepted grammar and flag casing, with installer regressions for each form.

Follow-up Findings

• src/ouroboros/telemetry.py:164 [warning] First-use identity creation is protected only by a process-local lock. Concurrent processes can both observe a missing state file, generate different UUIDs, overwrite telemetry.json, and retain different IDs in their process caches. A synchronized 64-process probe returned multiple distinct IDs while only one survived in the file. Concurrent MCP sessions or installer/application startup can therefore count one installation as multiple active users and violate the stated stable-identity contract. Use cross-process atomic creation or locking, then reload the winning state before caching it; coordinate the installer writer with the same protocol and add concurrent creation tests.
  | # | File:Line | Priority | Confidence | Suggestion |
  |---|-----------|----------|------------|------------|
  None.

Non-blocking Suggestions

| 1 | tests/unit/test_telemetry.py:404 | Test hygiene | The stalled-transport regression passes but produces PytestUnhandledThreadExceptionWarning because the acceptor thread calls accept() after the socket is closed. Catch the shutdown OSError or stop and join the thread before closing the socket so telemetry tests remain warning-clean. |

Test Coverage Notes

• Ran SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 /root/.local/bin/uv run --extra mcp python -m pytest -q tests/unit/test_telemetry.py tests/unit/config/test_loader_env.py tests/unit/mcp/server/test_adapter.py tests/unit/mcp/test_job_manager.py tests/unit/scripts/test_install_runtime_selection.py: 584 passed, 1 skipped, 1 warning.
• Focused installer probes reproduced collection for valid trusted-env opt-outs using multiple-space/tab export, OFF, and YES.
• A synchronized 64-process identity probe reproduced multiple UUIDs from one initially absent telemetry.json.

Design Notes

The telemetry boundary is appropriately thin, and durable outcome instrumentation fits the event-driven architecture. The remaining issues are cross-entrypoint trust parity and atomic ownership of persistent identity, both foundational to truthful metrics.

Design / Roadmap Gate

The changed boundary spans installer parsing, trusted environment configuration, persistent identity, MCP request observation, and durable job completion. Request and outcome semantics now preserve submission/completion distinctions and replay deduplication. Merge remains blocked because valid persisted opt-outs are still interpreted differently by the installer and application, while identity persistence is not atomic across the concurrent processes expected in multi-agent/MCP use.

Directional Notes

Review focus followed the advisory trust posture: operator opt-outs must dominate every entry surface, and published adoption metrics require a genuinely stable identity. Both blockers are grounded in current-source probes rather than prior review or memory claims.

Test Coverage

• Ran SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 /root/.local/bin/uv run --extra mcp python -m pytest -q tests/unit/test_telemetry.py tests/unit/config/test_loader_env.py tests/unit/mcp/server/test_adapter.py tests/unit/mcp/test_job_manager.py tests/unit/scripts/test_install_runtime_selection.py: 584 passed, 1 skipped, 1 warning.
• Focused installer probes reproduced collection for valid trusted-env opt-outs using multiple-space/tab export, OFF, and YES.
• A synchronized 64-process identity probe reproduced multiple UUIDs from one initially absent telemetry.json.

Merge Recommendation

Do not merge until installer opt-out resolution matches the Python loader for all accepted trusted-env forms and telemetry identity creation is atomic across processes and the installer. Add focused regressions for both boundaries; the existing telemetry and durable-outcome foundation can otherwise remain.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: 8d418f5
request_id: req_1786309679_1218
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

@ouroboros-agent Both findings from round req_1786309679_1218 are addressed in a8bb826:

1. Installer opt-out grammar/casing parity (scripts/install.sh) — fixed. The trusted-env parser now accepts export followed by any run of spaces/tabs (export[[:space:]]* + re-trim), and _telemetry_enabled lowercases and trims both flag values before matching, using exactly the application resolver's sets (1|true|on|yes for DO_NOT_TRACK, 0|false|off|no for OUROBOROS_TELEMETRY). All four reproduced forms — multi-space export, tab export, OUROBOROS_TELEMETRY=OFF, DO_NOT_TRACK=YES — now disable installer telemetry before any notice or event, plus a combined export DO_NOT_TRACK="1" # off regression. 6 new installer regressions; suite now 58 passed.

2. Concurrent first-use identity (src/ouroboros/telemetry.py) — made atomic on both surfaces. Python: _publish_new_state writes the candidate to a same-directory temp file and publishes with os.link (atomic create-if-not-exists, content complete before the link), falling back to O_CREAT|O_EXCL where hard links are unsupported, then re-reads the file (bounded 3×20ms retry for the fallback's partial-write window) and caches whichever distinct_id actually won — never its local candidate. Installer: _telemetry_distinct_id uses the same tmp + ln publish and adopts the winner from the file on re-read. New regression: 16 subprocesses synchronized on a sentinel file against one fresh HOME converge on a single UUID that matches the file (tests/unit/test_telemetry.py::TestConcurrentIdentityCreation).

3. Test-hygiene suggestion — fixed. The stalled-transport acceptor now catches shutdown OSError and is joined before the server socket closes; the telemetry suite runs clean under -W error::pytest.PytestUnhandledThreadExceptionWarning.

Verification: telemetry + loader-env + adapter + job-manager suites 533 passed, installer suite 58 passed, ruff check/format clean, bash -n scripts/install.sh OK.

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	a8bb826d91ad760a16815e5a4b5802b18b85a383
Request ID	req_1786311128_1225
Review record	e0dd3f24-241c-436f-94c9-5bc78640410f

What Improved

• Adds bounded, daemon-thread telemetry transport with failure isolation and no exit-time flush.
• Establishes operator-owned opt-out precedence across process environment, trusted user environment, persisted configuration, installer, CLI, and MCP paths.
• Separates asynchronous submission receipts from durable terminal outcomes and requires explicit evaluation approval for verified success.
• Adds atomic first-use identity publication, MCP failure-boundary instrumentation, polling sampling, first-run disclosure, and extensive regression coverage.

Issue Requirements

Requirement	Status
Anonymous opt-out telemetry for install, funnel conversion, entry-surface ratios, and backend outcomes	Partially met — instrumentation exists, but unrestricted serialization can violate the anonymity contract and corrupt identity state makes user-level metrics unreliable.
Single MCP request chokepoint plus CLI and installer instrumentation	Met — adapter, SDK, CLI, serve, installer, and durable-job boundaries were traced and covered by targeted tests.
Polling tools sampled 1/50 with a reweighting property	Met
Event whitelist that excludes code, prompts, file contents, paths, arguments, and environment variables	Not met — capture() and set_context() enforce no event or property allowlist.
Fixed verified weekly-active-user counting rule with installs, submissions, retries, and CI excluded	Partially met — durable outcome semantics and CI marking are implemented, but malformed state causes one installation to generate multiple anonymous identities.
Three authoritative opt-out paths, including fail-closed malformed configuration	Met
Installer honors trusted persistent controls and shows/persists notice before collection	Met
Stdlib-only, bounded, failure-isolated, non-blocking transport	Met
Stable random UUID identity in ~/.ouroboros/telemetry.json	Not met — malformed existing state is never repaired and produces a new unpersisted UUID per process.
MCP serve attachment denominator without inflating direct terminal-CLI usage	Met
In-process automation and detached workers do not inflate MCP request ratios	Met
Telemetry disabled suite-wide with isolated fake-transport tests	Met

Prior Findings Status

The specific prior blockers around trusted dotenv parsing, flag casing/grammar parity, blocking exit flush, absent-file concurrent identity creation, and stalled-test-thread cleanup are withdrawn: current source and focused tests show those changes are present. The earlier classification standard remains applicable—privacy declarations must be enforced at runtime and identity must remain stable across persistent-state failure. Applying that standard to current HEAD reveals the new allowlist-enforcement and malformed-state-repair blockers above.

Blockers

#	File:Line	Severity	Finding
1	src/ouroboros/telemetry.py:358	BLOCKING	Enforce the documented event and property whitelist at the serialization boundary. capture() accepts any event name and forwards every caller-provided property unchanged, while set_context() similarly accepts unrestricted global properties; both are exposed through the module's public exports. A focused fake-transport probe successfully emitted not_whitelisted with prompt and file_path properties, directly contradicting the PR's event-whitelist requirement and TELEMETRY.md's promise that serialization uses allowlisted properties and can never collect prompts or paths. Auditing today's call sites does not make that privacy contract executable; reject unknown events/properties or construct each supported event from a fixed schema before queueing it.
2	src/ouroboros/telemetry.py:159	BLOCKING	Repair malformed identity state instead of minting an unpersisted UUID. When telemetry.json exists but cannot be decoded, _load_state() generates a candidate, but _publish_new_state() only uses create-if-absent operations; both os.link() and O_EXCL refuse the existing corrupt file, the rereads fail, and the local candidate is returned without replacing the file. I reproduced two fresh processes against one corrupt file returning different UUIDs while the file remained unchanged. This permanently fragments the promised stable identity and corrupts retention/weekly-active metrics; _write_state() also uses a non-atomic truncate-and-write, so interruption can create this state internally. Atomically replace invalid state, make subsequent state updates atomic, and test stability across process reloads after corruption.

Follow-up Findings

#	File:Line	Priority	Confidence	Suggestion
1	src/ouroboros/telemetry.py:484	Medium	High	Consider making the notice check-and-set cross-process atomic. Concurrent first-use CLI/MCP processes can all read notice_shown=false and each display the advertised one-time notice before independently writing true. This does not expose data because each process discloses before capture, but it can produce repeated notices during simultaneous agent-session startup.

Non-blocking Suggestions

None.

Test Coverage Notes

• Ran SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 /root/.local/bin/uv run --extra mcp python -m pytest -q tests/unit/test_telemetry.py tests/unit/config/test_loader_env.py tests/unit/mcp/server/test_adapter.py tests/unit/mcp/test_job_manager.py tests/unit/scripts/test_install_runtime_selection.py: 590 passed, 1 skipped.
• Ran Ruff against the changed telemetry, configuration, MCP, installer-test, and telemetry-test surfaces: all checks passed.
• Installer syntax is exercised by the targeted installer suite.
• Independently reproduced permanent identity churn from malformed persisted state.
• Independently demonstrated that unrestricted event names, prompts, and file paths pass through capture() to the transport.
• No full repository test suite was run.

Design Notes

The request/outcome separation, trusted-control precedence, and thin MCP observation boundary now fit Ouroboros's infrastructure design. The remaining problems are both SSOT failures: privacy policy is documented but not enforced by the serializer, and identity persistence has no authoritative recovery path for invalid durable state.

Design / Roadmap Gate

This PR crosses persistent identity, trusted and untrusted environment loading, installer disclosure, CLI lifecycle, MCP request handling, and durable job-terminal boundaries. Opt-out precedence, submission-versus-terminal semantics, failure isolation, replay deduplication, and absent-state concurrent creation now satisfy the intended contracts. Merge remains blocked because the transport accepts data categories explicitly forbidden by the public contract and the identity state machine cannot recover from malformed durable state, allowing permanent metric fragmentation.

Directional Notes

Review focused on the maintainer trust posture: operator privacy controls must dominate project input, public telemetry declarations must match executable behavior, and published success/adoption metrics require durable terminal evidence plus a stable identity. Maintainer memory and prior reviews guided inspection only; both blockers were independently reproduced from the current snapshot.

Test Coverage

• Ran SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 /root/.local/bin/uv run --extra mcp python -m pytest -q tests/unit/test_telemetry.py tests/unit/config/test_loader_env.py tests/unit/mcp/server/test_adapter.py tests/unit/mcp/test_job_manager.py tests/unit/scripts/test_install_runtime_selection.py: 590 passed, 1 skipped.
• Ran Ruff against the changed telemetry, configuration, MCP, installer-test, and telemetry-test surfaces: all checks passed.
• Installer syntax is exercised by the targeted installer suite.
• Independently reproduced permanent identity churn from malformed persisted state.
• Independently demonstrated that unrestricted event names, prompts, and file paths pass through capture() to the transport.
• No full repository test suite was run.

Merge Recommendation

Do not merge until telemetry events and properties are schema-allowlisted before queueing, and malformed or interrupted identity state is atomically repaired to one stable persisted UUID. Add regressions proving forbidden fields cannot reach the transport and repeated/concurrent processes converge after a corrupt state file.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: a8bb826
request_id: req_1786311128_1225
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

@ouroboros-agent Both blockers from round req_1786311128_1225 are addressed in 2b786fc:

1. Event/property whitelist enforced at serialization (src/ouroboros/telemetry.py) — fixed. capture() now drops any event not in _EVENT_ALLOWLIST (command_run / workflow_outcome / mcp_serve_started) before it can touch the queue, and sanitizes properties against that event's allowlisted keys plus the base (app_version, os, python_version, frontdoor, ci) and context sets: unknown keys, non-scalar values, and strings longer than 200 chars are dropped (dropped, not truncated). set_context() accepts only the four resolved-backend keys with the same scalar/length gate. Your probe shapes are regressions now: capture("not_whitelisted", {"prompt": ...}) never reaches the transport, and prompt/file_path on a valid event are stripped while allowlisted keys survive (tests/unit/test_telemetry.py::TestPropertyAllowlist, 5 tests).

2. Malformed identity state atomically repaired — fixed on both surfaces. _load_state() distinguishes absent (FileNotFoundError → unchanged create-if-absent _publish_new_state) from invalid (exists but unparseable / no distinct_id → new _repair_state): the repairer takes an O_EXCL repair lock, re-validates under the lock, atomically replaces the corrupt file via tmp + os.replace, and re-reads/adopts the survivor; losers wait bounded (10×20ms) and adopt, with a stale-lock takeover fallback. _write_state() itself is now tmp + os.replace (the non-atomic truncate-and-write you flagged as the internal corruption source is gone). scripts/install.sh mirrors the distinction: ln create-if-absent when the file is missing, mv atomic repair when it exists but has no extractable distinct_id, re-read-and-adopt either way. Regressions: sequential two-process convergence after not-json{{{ corruption, 8-process synchronized concurrent convergence (all ids equal the persisted file), and an installer test proving install events are sent under the repaired persisted id.

3. Medium follow-up (repeated notices) — fixed. show_first_run_notice() claims an O_EXCL marker (~/.ouroboros/telemetry.notice) so exactly one of N simultaneous first-use processes prints; notice_shown is still persisted in telemetry.json via the now-atomic write. Regression: 8 synchronized subprocesses → notice appears exactly once.

Verification: telemetry + loader-env + adapter + job-manager suites 541 passed under -W error::PytestUnhandledThreadExceptionWarning, installer suite 59 passed, ruff clean, mypy clean, bash -n scripts/install.sh OK.

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	2b786fc84028d8e1622736715a9825b3f96bea67
Request ID	req_1786312333_1233
Review record	13ce810f-35ef-4c2d-9715-4a3de51c115c

What Improved

• Adds a centralized, bounded, failure-isolated telemetry transport with event/property allowlists.
• Protects telemetry controls and destinations from project-local .env input and fails closed on invalid user configuration.
• Separates asynchronous submission receipts from durable terminal outcomes and only marks explicitly approved evaluations as verified.
• Adds broad regression coverage for opt-out precedence, installer disclosure, adapter failures, job outcomes, identity creation, and malformed state.

Issue Requirements

Requirement	Status
Anonymous opt-out telemetry for installation, funnel conversion, entry-surface ratios, and backend success rates	Partially met — instrumentation exists, but stable identity and terminal-outcome delivery remain unreliable
Cover all supported runtime entry surfaces through the MCP chokepoint plus CLI and installer instrumentation	Met
Sample high-frequency polling tools at 1/50 with a re-weighting property	Met
Ship a truthful privacy contract, first-run notice, and three authoritative opt-out paths	Partially met — controls are substantially aligned, but a stale notice claim permits collection without completed disclosure
Use a stdlib-only, bounded, asynchronous transport that never blocks command execution	Met
Persist one stable random anonymous UUID across sessions	Not met — concurrent installer repair of corrupt state can emit and persist different identities
Keep MCP serve attachments and internal automation from inflating terminal-versus-agent usage ratios	Met
Distinguish accepted asynchronous submissions from verified durable terminal outcomes	Partially met — event semantics are correct, but detached-worker exit can drop the durable outcome before transport
Exclude installs, retries, and CI from the published active-user counting rule	Partially met — the declared predicate is implemented, but missing terminal delivery and split identities make the resulting count unreliable

Prior Findings Status

The prior event/property-whitelist blocker is resolved in current source. Malformed Python identity state is now atomically repaired, but the prior stable-identity concern is modified rather than withdrawn because the installer’s corrupt-state repair still permits concurrent identity splitting. The stale notice marker and detached-worker terminal-delivery issues are newly established from current-source evidence.

Blockers

#	File:Line	Severity	Finding
1	scripts/install.sh:323	BLOCKING	Corrupt identity repair is still not concurrency-safe in the installer. Every process that initially sees an invalid telemetry.json sets file_existed=true and later performs an unconditional mv of its own UUID before immediately adopting the current file. Concurrent repairers can therefore each emit under a different ID while the last writer becomes the persisted identity. A deterministic two-process probe against this snapshot returned IDs ending in ...001 and ...002, with ...002 persisted. This fragments the stable identity and active-user metrics. Use the same exclusive repair ownership/adopt-winner protocol across Python and the installer, and add a concurrent corrupt-state installer/cross-surface regression.
2	src/ouroboros/telemetry.py:692	BLOCKING	The exclusive notice marker has no crash/staleness recovery. If a process creates telemetry.notice and terminates before setting notice_shown and printing, every later process returns on FileExistsError, while telemetry remains enabled and collection proceeds without disclosure. This was reproduced by creating the reachable post-crash state (notice_shown=false plus an existing marker): show_first_run_notice() printed nothing and left the flag false. The notice claim must be recoverable or atomically tied to completed disclosure, with a crash-state regression.
3	src/ouroboros/telemetry.py:519	BLOCKING	The only event qualifying a user under the published counting rule is emitted from JobManager._append_event, but it is merely queued to a daemon thread. A detached worker returns after observing the terminal row at src/ouroboros/mcp/detached_worker.py:251 and never calls flush; the telemetry contract explicitly says queued or in-flight events are dropped at process exit. If the job task has already released, the worker has no grace period for DNS/TLS/HTTP completion, so terminal outcomes can be systematically lost even though submission events from the long-lived MCP parent survive. Add a bounded worker-local delivery handoff/flush after durable terminalization and an integration test that verifies a detached worker actually transports workflow_outcome before exit.

Follow-up Findings

#	File:Line	Priority	Confidence	Suggestion
None.

Non-blocking Suggestions

None.

Test Coverage Notes

• Dependency-complete focused suite passed: 599 passed, 1 skipped across telemetry, installer, config loader, MCP adapter, and job-manager tests using uv run --extra mcp.
• Focused identity/notice/outcome regressions passed: 5 passed.
• ruff check passed for changed Python paths, and bash -n scripts/install.sh passed.
• Independent probes reproduced concurrent installer identity divergence and the stale-marker disclosure failure.
• Existing tests cover concurrent Python identity creation and a single installer repair, but not concurrent corrupt-state installer/cross-surface repair, notice-claim crash recovery, or terminal-event transport from a detached worker.

Design Notes

The telemetry policy boundary is now appropriately centralized and sanitized, and durable outcome semantics are much clearer. The remaining defects are state-machine and process-lifecycle failures at installer concurrency, disclosure recovery, and detached-worker delivery boundaries.

Design / Roadmap Gate

This PR crosses trusted environment loading, persistent identity, installer execution, CLI disclosure, MCP request handling, and durable job completion. Configuration precedence, schema allowlisting, submission-versus-outcome semantics, and Python absent-state creation now fit the intended architecture. Merge remains blocked because corrupt identity repair is not one cross-runtime transaction, the disclosure marker has an unrecoverable intermediate state, and detached terminal producers do not guarantee a bounded delivery opportunity for the event that defines verified active usage.

Directional Notes

Review focus followed the maintainer trust posture: operator disclosure and opt-outs must survive failure paths, published adoption metrics require a stable identity, and verified outcomes must reach telemetry from the runtime that produces them. Prior memory and earlier classifications guided inspection only; each blocker was independently verified from the current snapshot or a focused probe.

Test Coverage

• Dependency-complete focused suite passed: 599 passed, 1 skipped across telemetry, installer, config loader, MCP adapter, and job-manager tests using uv run --extra mcp.
• Focused identity/notice/outcome regressions passed: 5 passed.
• ruff check passed for changed Python paths, and bash -n scripts/install.sh passed.
• Independent probes reproduced concurrent installer identity divergence and the stale-marker disclosure failure.
• Existing tests cover concurrent Python identity creation and a single installer repair, but not concurrent corrupt-state installer/cross-surface repair, notice-claim crash recovery, or terminal-event transport from a detached worker.

Merge Recommendation

Do not merge until concurrent corrupt-state repair converges across installer and Python processes, notice claims recover safely after interruption, and detached workers provide tested bounded delivery of durable terminal outcomes. The focused suite is green, but it does not exercise these critical failure boundaries.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: 2b786fc
request_id: req_1786312333_1233
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

@ouroboros-agent The error-taxonomy blocker from round req_1786326905_1382 is addressed in dda0ca1:

Closed output vocabulary, no provenance trust (src/ouroboros/mcp/telemetry_boundary.py). _safe_error_type no longer reads type(error).__module__ at all — both of your attack surfaces are gone by construction. The class NAME is serialized only when it belongs to an audited set assembled from trusted sources on OUR side: every builtin exception enumerated at import from the interpreter's own vars(builtins) (71 names — an enumeration CPython ships, not a claim the object makes), the nine literal ouroboros.mcp.errors classes (the complete hierarchy, traced against what actually reaches the boundary: not-found, security, verbatim MCPServerError re-raise, and wrap-all branches), plus CancelledError/JSONDecodeError. Everything else folds to the fixed literal ExtensionError. The key property: only audited strings can ever be serialized, so even a class spoofing an audited name (__module__ = "builtins", or naming itself ValueError) leaks nothing — the output is a closed vocabulary.

Total failure isolation. The helper is wrapped in try/except returning ExtensionError, so hostile metaclasses (raising __name__, non-string names) can never raise into the command path; and the caller-path audit found one more gap of the same class — the round-13 getattr(value, "is_error", False) read only defends against a MISSING attribute, not a hostile property that raises — now wrapped in _is_logical_error with the same totality, used at both call sites.

Regressions (typed + SDK + direct-unit): spoofed __module__="builtins", the ouroboros_acme_private.errors prefix collision that defeated the old startswith check, non-string __module__ (int) asserting BOTH the fold and that the handler's original Result.err object survives untouched (your contract-violation reproduction), and a hostile-__name__ metaclass against the helper directly. Audited names (MCPToolError, ValueError) stay verbatim; jsonschema's ValidationError keeps folding. Provenance note: the four type(...).__name__ structured-logging sites in adapter.py were git-blame-verified to predate this PR (#1622, present verbatim on origin/main) — local logs only, never PostHog — and were deliberately left untouched per the PR's scope.

Verification: adapter + evaluate-telemetry + telemetry suites 296 passed under -W error::PytestUnhandledThreadExceptionWarning; ruff/format/mypy clean.

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	dda0ca15953d578fc1ee2037070c39935253ed0b
Request ID	req_1786328585_1387
Review record	7d3bcdc4-8b8e-4eac-841f-c4028463e208

What Improved

• Adds bounded, failure-isolated telemetry transport with strict per-event property allowlists.
• Establishes operator-owned opt-out precedence across configuration, trusted environment, and installer paths.
• Separates asynchronous submission receipts from durable terminal outcomes and verified evaluation success.
• Sanitizes MCP tool names, job types, and error classifications at extension boundaries.
• Adds extensive regression coverage for identity persistence, replay deduplication, installer ordering, adapters, and job outcomes.

Issue Requirements

Requirement	Status
Add anonymous PostHog usage telemetry for install, CLI/MCP funnel, and backend reliability measurement	Met
Cover shared MCP, direct CLI, and installer entry surfaces with polling sampled 1/50	Met
Provide three authoritative opt-out paths and prevent project .env from controlling telemetry	Met
Never collect values that identify a user or project	Not met — dynamic installed-plugin names are serialized as CLI commands
Count active users only from non-CI, explicitly verified evaluation terminal outcomes	Met
Distinguish asynchronous submission acceptance from durable terminal success	Met
Use a random durable UUID rather than machine fingerprinting	Met
Keep transport bounded, asynchronous, non-blocking, and failure-isolated	Met
Display and persist a one-time first-run notice before collection	Met
Document the exact event/property whitelist and public aggregate-statistics policy	Partially met — the documented privacy exclusion does not match dynamic CLI behavior

Prior Findings Status

The prior error-taxonomy and hostile-object concern is withdrawn: current source uses a closed error-name vocabulary and failure-isolated logical-error access, with focused tests passing. The overall request-changes posture remains, but for a newly verified cross-entrypoint gap: MCP extension names are normalized while dynamic CLI plugin names are still transmitted verbatim.

Blockers

#	File:Line	Severity	Finding
1	src/ouroboros/cli/main.py:181	BLOCKING	The new CLI chokepoint forwards ctx.invoked_subcommand directly to telemetry, but _PluginAwareGroup resolves arbitrary installed plugin names as top-level commands. capture_cli_command() then serializes that name verbatim at src/ouroboros/telemetry.py:1049, unlike the audited MCP extension mapping. I reproduced a successful dynamic command named acme-private-project producing command: "acme-private-project" in the queued PostHog event. Plugin namespaces commonly identify organizations or projects, violating TELEMETRY.md’s promise that identifying project data is never collected. Map dynamic plugin commands to a fixed value such as extension_command, permit verbatim names only from an audited built-in CLI set, and add an actual Click dynamic-dispatch regression.

Follow-up Findings

#	File:Line	Priority	Confidence	Suggestion
None.

Non-blocking Suggestions

None.

Test Coverage Notes

• Reviewed telemetry, configuration, adapter, job-manager, detached-worker, evaluation-chain, and installer tests.
• SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 /root/.local/bin/uv run --extra mcp python -m pytest ... completed with 792 passed, 8 skipped.
• An initial run without the mcp extra had 10 dependency-related adapter failures; all passed when rerun with the declared extra.
• Focused Click and queue probes independently reproduced the untested dynamic-plugin command disclosure.

Design Notes

The centralized transport, durable-outcome boundary, opt-out resolver, and MCP metadata normalization are sound. CLI instrumentation needs the same closed-vocabulary treatment already applied to MCP tools and background job types.

Design / Roadmap Gate

This PR crosses persistent identity, trusted configuration, installer disclosure, MCP extension registration, dynamic CLI plugin dispatch, and durable terminal-state boundaries. Replay deduplication, opt-out precedence, outcome semantics, and MCP extension sanitization now fit the intended trust model. Merge remains blocked because the CLI side of the same public extension contract lacks canonicalization and can disclose plugin-controlled identifiers contrary to the published privacy contract.

Directional Notes

Review focused on executable privacy parity across CLI, plugin, MCP, installer, configuration, and durable job paths. Maintainer memory guided attention toward operator-owned opt-outs and truthful outcomes, but the blocker is independently established from current source and runtime probes.

Test Coverage

• Reviewed telemetry, configuration, adapter, job-manager, detached-worker, evaluation-chain, and installer tests.
• SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 /root/.local/bin/uv run --extra mcp python -m pytest ... completed with 792 passed, 8 skipped.
• An initial run without the mcp extra had 10 dependency-related adapter failures; all passed when rerun with the declared extra.
• Focused Click and queue probes independently reproduced the untested dynamic-plugin command disclosure.

Merge Recommendation

Do not merge until dynamic plugin CLI commands are represented by a fixed non-identifying category and a Click-level regression proves plugin names cannot reach serialized telemetry. The focused implementation suites otherwise pass.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: dda0ca1
request_id: req_1786328585_1387
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

@ouroboros-agent The dynamic-CLI-command blocker from round req_1786328585_1387 is addressed in 50ff06d, completing the closed identifier vocabulary across every entry surface:

CLI commands audited (src/ouroboros/telemetry.py). capture_cli_command serializes a subcommand verbatim only when it belongs to _CANONICAL_CLI_COMMANDS — the 27 statically registered built-in ooo subcommands, enumerated from the real typer app via typer.main.get_command(app).commands (the same conversion the production entrypoint uses; this caught two hidden aliases — monitor, dispatch — that a source grep would have missed). Anything _PluginAwareGroup resolves dynamically (installed plugin names, which commonly identify organizations/projects) folds to the fixed literal extension_command with is_funnel=false. _CLI_SKIP remains a separate product gate, and the funnel tuple/_CLI_FUNNEL are asserted subsets of the audited set.

Actual Click dynamic-dispatch regression. A new test stages a genuine installed plugin named acme-private-project (manifest + lockfile + trust-store, mirroring the plugin-dispatch suite's staging) and invokes the REAL ouroboros_app through CliRunner so resolution goes through the genuine _PluginAwareGroup.get_command() path — the captured event reads command == "extension_command" with the plugin name absent from the full event repr. A control test proves a canonical built-in (status) still captures verbatim through the same real callback path, and a sync guard asserts every statically registered command is a member of the audited set, failing CI if a future built-in is missed. The unit layer covers the direct hostile-name, canonical pass-through, and subset assertions.

With this round, every caller-influenced identifier channel — MCP tool names (ouroboros_extension_tool/ouroboros_unknown_tool), durable job types (extension_job), error classes (ExtensionError), and CLI commands (extension_command) — is a closed audited vocabulary, and TELEMETRY.md's allowlist note covers all four in one place.

Verification: telemetry + new CLI-telemetry + existing plugin-dispatch suites 106 passed under -W error::PytestUnhandledThreadExceptionWarning (3× no-flake on the new file); ruff/format/mypy clean.

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	50ff06d99cbf1ae1c1ed0643f4ad49b1b23ad125
Request ID	req_1786330235_1392
Review record	e7be8e27-cb5f-4fc2-b556-ef48a9d3c350

What Improved

• Adds a bounded asynchronous telemetry transport with durable anonymous identity and exact per-event property allowlists.
• Separates asynchronous submission receipts from deduplicated durable workflow_outcome events and reserves verified=true for explicitly approved evaluations.
• Enforces fail-closed malformed configuration, protects telemetry controls from project .env, and aligns installer notice and opt-out ordering.
• Normalizes dynamic CLI commands, MCP tools, job types, and error classes into audited non-identifying vocabularies.

Issue Requirements

Requirement	Status
Anonymous opt-out PostHog usage telemetry across supported runtime entrypoints	Partially met — central instrumentation exists, but project-controlled CI classification can alter measured eligibility.
Instrument MCP, direct CLI, and installer entry surfaces without counting internal automation as user activity	Met
Sample high-frequency polling tools at 1/50 with a reweighting property	Met
Provide three effective opt-out paths and fail closed for invalid persisted configuration	Met
Protect telemetry controls and destinations from project-controlled .env input	Met
Publish an exact event/property and weekly-active counting contract	Partially met — the documented rule exists, but untrusted project input can force the ci exclusion.
Distinguish submission acceptance from durable terminal and verified outcomes	Met
Ensure telemetry never raises, terminates, or changes command behavior	Not met — hostile BaseException values escape the new telemetry boundary.
Persist a random non-fingerprinted UUID and display a one-time notice before collection	Met
Provide critical-path regressions for privacy, adapters, persistence, installer behavior, and outcome semantics	Partially met — broad coverage passes, but the two reproduced boundary failures lack tests.

Prior Findings Status

The prior dynamic-plugin CLI identifier concern is withdrawn: current code folds noncanonical commands to extension_command, and the real static command roster matches the audited set. Earlier opt-out, destination, malformed-config, installer-ordering, durable-outcome, tool-name, job-type, and ordinary exception-taxonomy concerns are also resolved by current evidence. The prior total failure-isolation concern remains in modified form because the remediation handles hostile Exception values but not hostile BaseException values. The project-controlled CI classification issue is newly identified.

Blockers

#	File:Line	Severity	Finding
1	src/ouroboros/mcp/telemetry_boundary.py:106	BLOCKING	The advertised total failure isolation still catches only Exception. Both _safe_error_type() and _is_logical_error() allow KeyboardInterrupt, SystemExit, or another BaseException raised by an extension-controlled metaclass/property to escape. A focused current-snapshot probe showed _safe_error_type() propagating a hostile KeyboardInterrupt, and observe_adapter_tool_call() replacing a completed Result.ok with another hostile KeyboardInterrupt from is_error. Telemetry can therefore still change or terminate command behavior through the public extension boundary. Catch BaseException inside these sanitization-only helpers and add typed-adapter regressions for hostile BaseException subclasses.

Follow-up Findings

• src/ouroboros/telemetry.py:834 [warning] The ci exclusion used by the fixed weekly-active counting rule trusts CI and GITHUB_ACTIONS after the configuration loader has imported the project-controlled .env. Neither key is denied for untrusted project input. A current-snapshot subprocess probe with only .env: CI=1 produced a normal local command_run carrying ci=true; the same behavior excludes genuine local workflow_outcome events from the published active-user rule. This lets cloned repositories silently alter telemetry semantics. Treat CI classification as operator/runtime-owned—for example, deny these keys from project .env while preserving pre-existing process values—and add an integration regression.
  | # | File:Line | Priority | Confidence | Suggestion |
  |---|-----------|----------|------------|------------|
  None.

Non-blocking Suggestions

None.

Test Coverage Notes

• Ran the focused telemetry, configuration, CLI, MCP adapter, job manager, detached worker, evaluation, and installer suites with SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 and the mcp-test dependency group: 748 passed, 8 skipped.
• Independently derived the real Typer command and MCP-error rosters: all 27 static CLI commands and all current Ouroboros MCP exception classes match their audited telemetry sets.
• Added runtime probes outside the PR-authored tests that reproduced both blockers: escaping hostile BaseException values and project .env forcing ci=true.

Design Notes

The centralized boundary, closed metadata vocabularies, durable outcome ownership, and fail-closed privacy resolver are directionally sound. The remaining issues are trust-boundary completeness failures rather than structural objections to the telemetry architecture.

Design / Roadmap Gate

This PR crosses persistent identity, trusted and untrusted environment resolution, installer disclosure, public CLI/MCP extension points, and durable terminal-state measurement. Replay deduplication, opt-out precedence, canonical identifier transport, and outcome ownership now fit the intended design. Merge remains blocked because sanitization is not total for the same BaseException domain the observer handles, and the fixed active-user eligibility rule still consumes a project-controlled environment signal.

Directional Notes

Review focus followed the advisory maintainer posture that operator privacy and measurement controls must dominate project input, while telemetry must remain unable to affect runtime behavior. Prior review history guided inspection only; both blockers were independently reproduced from the current source snapshot.

Test Coverage

• Ran the focused telemetry, configuration, CLI, MCP adapter, job manager, detached worker, evaluation, and installer suites with SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 and the mcp-test dependency group: 748 passed, 8 skipped.
• Independently derived the real Typer command and MCP-error rosters: all 27 static CLI commands and all current Ouroboros MCP exception classes match their audited telemetry sets.
• Added runtime probes outside the PR-authored tests that reproduced both blockers: escaping hostile BaseException values and project .env forcing ci=true.

Merge Recommendation

Do not merge until telemetry sanitizers cannot propagate hostile BaseException values and CI eligibility is derived only from trusted process/runtime state rather than project .env. Add focused regressions for both reproduced paths; the broader targeted suites otherwise pass.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: 50ff06d
request_id: req_1786330235_1392
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

@ouroboros-agent Both findings from round req_1786330235_1392 are addressed in f3cf009:

1. Total BaseException isolation (src/ouroboros/mcp/telemetry_boundary.py) — fixed. _safe_error_type and _is_logical_error now catch BaseException, closing your reproduced escapes (hostile metaclass __name__ raising KeyboardInterrupt; hostile is_error property raising through the observer and replacing a completed Result.ok). Swallowing these inside sanitization-only helpers is correct — the raise originates from an extension-controlled dunder read, not a genuine interrupt — and the rationale is documented inline. A full sweep of the file's except clauses: the two observation wrappers already caught-and-reraised BaseException (correct as-is), and record_direct_evaluation_outcome deliberately keeps except Exception since its inputs are typed primitives with no hostile-object attack surface. Regressions: KeyboardInterrupt/SystemExit from __name__ and is_error properties at the direct-unit layer, plus typed-adapter integration — a hostile is_error raising SystemExit completes the real call with the original Result.ok value untouched and ok=true, and a raised KeyboardInterrupt-subclassed hostile error propagates with unchanged semantics while the captured event folds to ExtensionError with the identifying string absent.

2. CI classification is operator/runtime-owned (src/ouroboros/config/untrusted_env.py) — fixed. CI and GITHUB_ACTIONS join the untrusted-project denylist alongside the telemetry-control cluster: a cloned repository shipping CI=1 in its .env could silently deregister genuine local users from the published weekly-active rule's ci!=true exclusion. Real CI runners set these in the actual process environment, which the loader never overrides, so genuine CI detection is unchanged — asserted by regression. Tests: neither key reaches os.environ from untrusted input (parametrized + membership pinning), your exact integration shape as a real import-order subprocess (project .env CI=1 → env clean and the captured command_run carries no ci property), and process-env CI=true still stamps ci=true. TELEMETRY.md's denied-key sentence updated.

Verification: telemetry + loader-env + adapter + evaluate-telemetry + CLI-telemetry suites 519 passed under -W error::PytestUnhandledThreadExceptionWarning; ruff/format/mypy clean.

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: APPROVE

Metadata

Field	Value
PR	#1908
HEAD checked	f3cf009ee5d15a3ce125e5c67e971c09c42a4ade
Request ID	req_1786332563_1397
Review record	fcc7731f-a2fd-48da-af97-bc73c5f03a52

What Improved

• Adds bounded, asynchronous, failure-isolated telemetry with explicit event/property allowlists and durable anonymous identity handling.
• Separates asynchronous submission receipts from terminal workflow outcomes, counting only explicitly approved evaluations as verified.
• Protects telemetry controls, destinations, and CI classification from project-controlled .env values.
• Normalizes extension-controlled CLI commands, MCP tools, job types, and exception classes to fixed non-identifying categories.
• Adds broad regression coverage across configuration, installer, CLI, typed/SDK MCP adapters, detached workers, durable jobs, and evaluation outcomes.

Issue Requirements

Requirement	Status
Add anonymous, opt-out PostHog telemetry for installation, funnel conversion, entrypoint ratios, and backend reliability	Met
Instrument MCP calls through the shared adapter boundary, plus direct CLI and installer entrypoints	Met
Sample high-frequency polling tools at 1/50 and include a reweighting property	Met
Publish a telemetry trust contract covering event scope, uses, active-user counting, anonymity, aggregation, and changes	Met
Support DO_NOT_TRACK, OUROBOROS_TELEMETRY=0, and persisted telemetry.enabled: false, with any disable winning	Met
Prevent project-controlled environment files from overriding telemetry controls, destinations, or CI classification	Met
Show a one-time disclosure before collection	Partially met — structurally enforced when python3 is available; the explicitly documented text-only fallback can suppress it from nested JSON on uv-first systems without python3
Count verified terminal evaluation outcomes rather than queued submissions	Met
Keep telemetry non-blocking, bounded, offline-safe, and failure-isolated	Met
Preserve stable random identity without machine fingerprinting or ephemeral IDs	Partially met — fully enforced in Python and parser-backed installer paths; the acknowledged no-python3 installer fallback is text-structural rather than JSON-structural

Prior Findings Status

Prior blocking concerns are withdrawn on current-source evidence. Operator opt-outs and destinations are protected from project .env; malformed configuration fails closed; installer notice precedes capture; submissions and terminal outcomes are distinct; job outcomes are deduplicated; logical payload errors affect success classification; extension-controlled identifiers are normalized; hostile telemetry sanitization catches BaseException; and CI classification is operator/runtime-owned. The contributor’s explicit explanation of the no-python3 installer fallback is retained as a non-blocking follow-up rather than re-raised as a blocker.

Blockers

No in-scope blocking findings remained after policy filtering.

Follow-up Findings

#	File:Line	Priority	Confidence	Suggestion
1	scripts/install.sh:380	Medium	High	Consider failing telemetry closed when python3 is unavailable instead of using the text-only identity and notice fallbacks. On an otherwise supported uv-first installation, valid JSON containing only nested distinct_id/notice_shown fields is accepted by the regex paths: a focused probe returned the nested UUID and the fallback notice check suppressed disclosure. This limitation was explicitly acknowledged by the contributor as a best-effort compatibility fallback, so it is not re-raised as a blocker, but deferring collection until a structural JSON parser is available would make the installer fully match the Python trust contract.

Non-blocking Suggestions

| 1 | scripts/install.sh:537 | Documentation | The source documents the no-python3 structural-validation gap, but TELEMETRY.md presents identity and first-run disclosure without this qualification. Either eliminate the fallback gap or disclose it in the public contract. |

Test Coverage Notes

• Reviewed telemetry, CLI, untrusted-environment, installer, adapter, job-manager, detached-worker, direct-evaluation, and start-evaluation tests.
• 567 passed, 8 skipped for the focused non-SDK suites with SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 and unhandled thread warnings promoted to errors.
• 198 passed for tests/unit/mcp/server/test_adapter.py using uv run --extra mcp.
• The initial default-environment adapter run lacked the optional mcp package; rerunning with the declared extra resolved all ten environment-related failures.
• Independently reproduced the documented no-python3 installer fallback behavior described in the follow-up.

Design Notes

The implementation now centralizes policy in a small telemetry module and a shared MCP boundary while keeping handlers thin. Exact property allowlists, closed identifier vocabularies, fail-closed controls, and durable terminal-event ownership provide a coherent trust and measurement model.

Design / Roadmap Gate

The affected boundary spans persistent identity, installer disclosure, trusted/untrusted configuration, CLI and MCP entrypoints, SDK conversion, extension registration, durable job terminals, replay deduplication, and evaluation ownership. Current code preserves opt-out precedence, sanitizes caller-controlled metadata, distinguishes submission from completion, flushes detached terminal outcomes, and avoids duplicate direct/job-backed evaluation events. The remaining no-parser installer fallback is explicitly documented and previously explained; it is a compatibility hardening opportunity rather than a newly unresolved merge blocker.

Directional Notes

Review focus followed the advisory trust-first and replay-safe maintainer posture: privacy controls must dominate project input, telemetry must not alter command behavior, identifiers must remain non-identifying across extension boundaries, and success metrics must derive from terminal verification rather than acceptance. All merge conclusions are grounded in the current snapshot and focused execution, not memory artifacts.

Test Coverage

• Reviewed telemetry, CLI, untrusted-environment, installer, adapter, job-manager, detached-worker, direct-evaluation, and start-evaluation tests.
• 567 passed, 8 skipped for the focused non-SDK suites with SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 and unhandled thread warnings promoted to errors.
• 198 passed for tests/unit/mcp/server/test_adapter.py using uv run --extra mcp.
• The initial default-environment adapter run lacked the optional mcp package; rerunning with the declared extra resolved all ten environment-related failures.
• Independently reproduced the documented no-python3 installer fallback behavior described in the follow-up.

Merge Recommendation

Merge is supported. Current-source inspection and 765 focused passing tests provide positive evidence that the previously blocking privacy, runtime, extension, and durable-outcome contracts are addressed. Track the no-python3 installer fallback as follow-up hardening.

Review-Metadata:
verdict: APPROVE
head_sha: f3cf009
request_id: req_1786332563_1397
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: APPROVE

Metadata

Field	Value
PR	#1908
HEAD checked	3caae4aed365e9409ffe4461de8970a90ba4eacf
Request ID	req_1786334417_1410-retry-1-retry-2
Review record	3d6e40da-f80e-4cb9-8f57-8346e4e0e175

What Improved

• Adds bounded, asynchronous, failure-isolated anonymous telemetry with explicit event/property allowlists.
• Establishes operator-owned opt-out precedence across process environment, trusted user environment, persisted configuration, CLI, MCP, and installer surfaces.
• Separates asynchronous submission receipts from durable terminal outcomes and only marks completed, explicitly approved evaluations as verified.
• Canonicalizes extension-controlled CLI commands, MCP tools, job types, and exception classes to prevent identifying metadata disclosure.
• Adds replay-deduplicated job outcomes, detached-worker flushing, atomic identity repair, and one-time disclosure handling.

Issue Requirements

Requirement	Status
Add anonymous, opt-out PostHog telemetry for install, workflow funnel, entry-surface ratio, and backend reliability measurement	Met
Cover supported MCP host entrypoints through a shared request boundary while excluding internal automation from CLI-versus-agent counts	Met
Sample polling tools at 1/50 and include a reweighting property	Met
Provide a documented privacy contract with an exact event/property whitelist, fixed active-user rule, aggregate-use disclosure, k-anonymity floor, and changelog policy	Met
Support DO_NOT_TRACK, OUROBOROS_TELEMETRY=0, and persisted telemetry.enabled: false opt-outs	Met
Keep telemetry non-blocking and failure-isolated using stdlib transport and a bounded queue	Met
Use a random persistent UUID without machine fingerprinting or PII	Met
Distinguish background submission acceptance from verified terminal success	Met
Preserve durable terminal outcomes across detached-worker exit and replay without duplicate counting	Met
Display and persist first-run disclosure before installer collection	Met

Prior Findings Status

Prior blocking concerns are withdrawn on current-source evidence. The snapshot independently preserves operator-owned privacy controls, fail-closed malformed configuration handling, notice-before-capture ordering, canonical extension metadata, total observer isolation, truthful submission/terminal semantics, durable outcome deduplication, and trusted CI classification. The prior approval classification is maintained; the documented no-parser installer fallback remains a non-blocking hardening opportunity.

Blockers

No in-scope blocking findings remained after policy filtering.

Follow-up Findings

#	File:Line	Priority	Confidence	Suggestion
1	scripts/install.sh:325	Low	High	Replace the documented no-python3 raw-text identity/notice fallback with a structured parser or fail collection closed when parsing is unavailable. Raw grep/sed cannot distinguish top-level fields from nested or duplicate JSON keys, although normal installer environments and all Python-backed paths enforce the intended schema.

Non-blocking Suggestions

None.

Test Coverage Notes

• Reviewed telemetry, CLI, trusted/untrusted configuration, installer, MCP adapter, detached-worker, job-manager, and direct/job-backed evaluation tests.
• Broad targeted run produced 790 passed and 23 skipped; its ten failures were caused by running adapter tests without the optional MCP SDK dependency.
• Rerunning the affected adapter, detached-worker, and evaluation telemetry suites with --extra mcp produced 218 passed, including all ten previously dependency-blocked tests.
• ruff check passed for the changed source and representative tests.
• bash -n scripts/install.sh passed; shellcheck was unavailable in the review environment.

Design Notes

The implementation centralizes Python telemetry policy and serialization while keeping MCP instrumentation at thin request and durable-state boundaries. Installer duplication is substantial but explicitly aligned with the same opt-out, identity, disclosure, and fail-closed contracts.

Design / Roadmap Gate

The affected boundary spans persistent identity, installer disclosure, trusted and untrusted environment resolution, CLI and MCP entrypoints, SDK conversion, extension registration, durable job terminals, replay deduplication, and direct evaluation ownership. Current code preserves compatibility and opt-out precedence, sanitizes caller-controlled metadata, distinguishes submission from completion, flushes detached terminal outcomes, and prevents duplicate direct/job-backed evaluation events. No current blocking contract, persistence, replay, or runtime-boundary failure was found.

Directional Notes

Review focus followed the advisory trust-first and replay-safe maintainer posture: privacy controls must dominate project input, telemetry must never alter command behavior, extension metadata must remain non-identifying, and success metrics must derive from verified terminal evidence. All merge conclusions are independently grounded in the current snapshot and focused execution.

Test Coverage

• Reviewed telemetry, CLI, trusted/untrusted configuration, installer, MCP adapter, detached-worker, job-manager, and direct/job-backed evaluation tests.
• Broad targeted run produced 790 passed and 23 skipped; its ten failures were caused by running adapter tests without the optional MCP SDK dependency.
• Rerunning the affected adapter, detached-worker, and evaluation telemetry suites with --extra mcp produced 218 passed, including all ten previously dependency-blocked tests.
• ruff check passed for the changed source and representative tests.
• bash -n scripts/install.sh passed; shellcheck was unavailable in the review environment.

Merge Recommendation

Merge is supported. Current-source inspection, focused runtime verification, and static checks provide positive evidence that the telemetry privacy, command-stability, extension, and durable-outcome contracts are satisfied. Track the no-python3 installer fallback as follow-up hardening.

Review-Metadata:
verdict: APPROVE
head_sha: 3caae4a
request_id: req_1786334417_1410-retry-1-retry-2
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	c2e1036369141836c6c6cfd4d10e046ff8832adc
Request ID	req_1786336261_1417
Review record	ca704649-e819-488d-8950-1037d5b816cc

What Improved

• Adds a documented, opt-out telemetry contract with exact event/property declarations, anonymous durable identity, and one-time disclosure.
• Separates asynchronous submission receipts from durable terminal outcomes, including replay deduplication and verified-evaluation semantics.
• Protects telemetry controls, destinations, and CI classification from project-controlled .env input.
• Canonicalizes extension-controlled CLI, MCP tool, job, and error identifiers to non-identifying fixed values.

Issue Requirements

Requirement	Status
Provide anonymous, opt-out PostHog telemetry across supported entry surfaces and runtimes	Partially met — instrumentation exists, but installer serialization can emit undeclared properties
Measure install and interview → seed → run → evolve/evaluate conversion without counting submissions as verified success	Met
Instrument MCP, direct CLI, installer, and durable background-job boundaries	Met
Sample polling tools at 1/50 with a re-weighting property	Met
Ship the telemetry whitelist, public counting rule, aggregate-use policy, k-anonymity floor, and changelog	Partially met — documented accurately, but the installer does not enforce the exact whitelist structurally
Provide one-time first-run disclosure and DO_NOT_TRACK, OUROBOROS_TELEMETRY=0, and persisted-config opt-outs	Met
Keep telemetry asynchronous, bounded, failure-isolated, and dependency-light	Met
Use a random durable UUID without PII, fingerprinting, paths, prompts, code, or arbitrary project identifiers	Partially met — identity handling is sound, but unescaped installer values can introduce arbitrary data
Exclude host-spawned MCP serves from direct-terminal CLI counts and internal automation from front-door ratios	Met

Prior Findings Status

Prior privacy, opt-out, identifier-canonicalization, durable-outcome, replay, and failure-isolation concerns are resolved in the current snapshot and were not re-raised. Prior rounds had reached approval; this review introduces one newly reproduced installer serialization blocker not present in the supplied prior discussion.

Blockers

#	File:Line	Severity	Finding
1	scripts/install.sh:611	BLOCKING	_telemetry_ping constructs JSON through unescaped string concatenation, so values can alter the declared property structure or make the payload invalid. For example, placing an executable named uname earlier on PATH that returns Linux\",\"leaked\":\"project-secret causes the real payload to contain the undeclared property "leaked":"project-secret". The same issue applies to every value appended at line 615. This violates TELEMETRY.md’s exact-property and “never collect arbitrary identifying data” contract at the installer boundary. Serialize values with a real JSON encoder when available and a strict shell JSON-escaping/canonicalization fallback, constrain os/arch to a bounded vocabulary or safe token format, and add an installer regression using quote/control-character-bearing command output.

Follow-up Findings

#	File:Line	Priority	Confidence	Suggestion
None.

Non-blocking Suggestions

None.

Test Coverage Notes

• Reviewed the telemetry, installer, CLI, configuration, MCP adapter, detached-worker, job-manager, and evaluation telemetry tests.
• Focused run without the optional MCP extra: 828 passed, 23 skipped; 10 adapter transport failures were solely due to the missing mcp package.
• Required-extra rerun: 339 passed, 5 skipped with --extra mcp and PytestUnhandledThreadExceptionWarning promoted to an error.
• Ruff check and format verification passed for all changed Python files; bash -n scripts/install.sh passed. ShellCheck was unavailable.
• Added a focused runtime probe independently demonstrating undeclared installer-property injection through unescaped uname output; no existing regression covers this path.

Design Notes

The shared Python telemetry boundary is well isolated and enforces closed event, property, and identifier vocabularies. The installer independently reimplements serialization but lacks equivalent structural enforcement, creating a cross-runtime trust-contract mismatch.

Design / Roadmap Gate

Persistence, replay deduplication, terminal outcome ownership, malformed-config fail-closed behavior, detached-worker flushing, extension identifier folding, and CLI/MCP compatibility are supported by current source and focused tests. The remaining affected boundary is installer transport serialization: unlike the Python serializer, it permits property-shape mutation through unescaped command-derived values, directly violating the PR’s public exact-property and privacy contracts.

Directional Notes

Review focus followed the advisory trust-first posture: operator opt-outs must dominate project input, success must reflect terminal verification, and every telemetry surface must enforce the published privacy vocabulary. Prior review history and maintainer memory guided inspection only; the blocker is grounded in the current installer source and a focused runtime reproduction.

Test Coverage

• Reviewed the telemetry, installer, CLI, configuration, MCP adapter, detached-worker, job-manager, and evaluation telemetry tests.
• Focused run without the optional MCP extra: 828 passed, 23 skipped; 10 adapter transport failures were solely due to the missing mcp package.
• Required-extra rerun: 339 passed, 5 skipped with --extra mcp and PytestUnhandledThreadExceptionWarning promoted to an error.
• Ruff check and format verification passed for all changed Python files; bash -n scripts/install.sh passed. ShellCheck was unavailable.
• Added a focused runtime probe independently demonstrating undeclared installer-property injection through unescaped uname output; no existing regression covers this path.

Merge Recommendation

Do not merge until installer telemetry uses structurally safe JSON serialization and a regression proves hostile quote/control-character-bearing values cannot add properties, disclose arbitrary data, or corrupt the payload. The remaining telemetry architecture and focused suites otherwise provide strong positive evidence.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: c2e1036
request_id: req_1786336261_1417
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

@ouroboros-agent The installer serialization blocker from round req_1786336261_1417 is addressed in 7666d94:

Structural encoding (scripts/install.sh _telemetry_ping). Unescaped string concatenation is gone. Three layers: (1) os/arch are token-constrained at the source — fullmatch [A-Za-z0-9._-]{1,32} or the literal unknown — which defuses your hostile-uname probe regardless of encoder availability; (2) with python3 on PATH (the same soft dependency the identity/notice readers use), the ENTIRE payload is built by json.dumps with event, distinct_id, api_key, and every key=value passed via argv — never interpolated into the -c source — so a hostile value can only ever become the string contents of its own declared property, and keys are defensively whitelisted to [A-Za-z0-9_]+ (a failing key is dropped, never stringified); (3) the no-python3 fallback pushes every value (including api_key/event/distinct_id) through a strict safe-token filter (tr -cd 'A-Za-z0-9._-', 64-char cap, unknown for empty) before concatenation — every legitimate installer value fits that charset losslessly, and no quote/backslash/control character can survive to mutate structure, consistent with the documented NO_PYTHON3 best-effort posture elsewhere in the file.

Regressions drive the real _telemetry_ping via the extracted-function driver: your exact probe — a PATH-earlier uname emitting Linux\",\"leaked\":\"project-secret — produces a payload that json.loads cleanly with NO leaked key and os == unknown, on BOTH the python3 path and the forced no-python3 path; and a normal ping asserts the exact declared property structure (parsed JSON, exact key-set match), doubling as the installer-side structural whitelist regression. Installer suite 75 passed, bash -n OK, ruff clean.

CI note: all 14 checks are green on the merge-resolved head (the previous module-size failure was fixed in 3caae4a by moving the PR's additions out of the grandfathered adapter module).

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	7666d9404fd092f90b3a397bf9c6883ad1dd7ec4
Request ID	req_1786337804_1429
Review record	6b45f1eb-9954-4fa2-b88d-55c7087f08cf

What Improved

• Adds bounded, asynchronous telemetry with strict event/property allowlists and stable random UUID identities.
• Separates asynchronous submission receipts from verified durable workflow outcomes, including replay deduplication.
• Protects telemetry controls, destinations, and CI classification from project-controlled .env input.
• Sanitizes extension-controlled command, tool, job, and error identifiers across CLI and MCP boundaries.
• Fixes installer JSON serialization so hostile values cannot mutate the payload structure.

Issue Requirements

Requirement	Status
Anonymous opt-out PostHog telemetry for installation, workflow funnels, entrypoint ratios, and runtime success rates	Partially met — instrumentation exists, but the installer can ignore a valid duplicate-key persisted opt-out
Shared MCP/CLI/install instrumentation with polling sampled 1/50 and reweighting metadata	Met
Published event whitelist, aggregate-use policy, fixed active-user rule, k-anonymity floor, and changelog	Met
One-time first-run notice and three authoritative opt-out paths	Partially met — ordinary paths work, but trusted .env duplicate-key semantics diverge between installer and application
Stdlib-only bounded, asynchronous, failure-isolated transport	Met
Stable random UUID identity without machine fingerprinting or PII	Met
Distinguish submission acceptance from terminal and verified outcomes	Met
Avoid internal automation and MCP serve boots inflating terminal CLI usage	Met
Disable telemetry during tests and provide focused telemetry regressions	Partially met — broad coverage exists, but duplicate trusted-control precedence is missing

Prior Findings Status

The prior installer structural-serialization blocker is withdrawn: current source uses JSON encoding on the Python path, constrained fallback serialization without Python, and focused hostile-value coverage. Earlier privacy, identifier-sanitization, durable-outcome, replay, and adapter-isolation concerns are also addressed in the current snapshot. Approval cannot be maintained because current verification found a new trusted .env precedence mismatch that can bypass an installer opt-out.

Blockers

#	File:Line	Severity	Finding
1	scripts/install.sh:172	BLOCKING	_telemetry_load_user_env makes duplicate keys in the trusted ~/.ouroboros/.env file first-wins because it exports the first value and then skips every later occurrence once the variable is set. The application resolver delegates to dotenv_values, where duplicate keys are last-wins (src/ouroboros/config/loader.py:207). I reproduced OUROBOROS_TELEMETRY=1 followed by OUROBOROS_TELEMETRY=0: the installer reports telemetry enabled with value 1, while the application resolves value 0 and disables telemetry. Consequently, a valid persisted opt-out can be ignored by install.sh, which then displays the notice and emits install telemetry contrary to the published same-controls contract. Preserve real process-environment precedence, but resolve all assignments within the trusted file using the application’s last-wins semantics; add an installer regression covering an enabling entry followed by a disabling duplicate for both OUROBOROS_TELEMETRY and DO_NOT_TRACK.

Follow-up Findings

#	File:Line	Priority	Confidence	Suggestion
None.

Non-blocking Suggestions

None.

Test Coverage Notes

• Reviewed the telemetry, installer, configuration, CLI, MCP adapter, durable-job, detached-worker, and evaluation telemetry tests.
• Initial focused run completed with 778 passed and 23 skipped; 10 adapter failures were caused solely by running without the optional MCP dependency.
• Rerunning the relevant adapter/job/evaluation suites with --extra mcp produced 389 passed.
• Evaluation/Ralph and run/evaluate chaining suites produced 53 passed.
• bash -n scripts/install.sh, Ruff on the changed Python files, and mypy on 12 changed source files passed.
• Existing installer tests cover ordinary trusted .env opt-outs and quoting, but do not cover duplicate telemetry-control assignments; the reproduced first-wins/last-wins divergence remains untested.

Design Notes

The telemetry boundary is now largely well-factored: serialization and privacy vocabulary are centralized, MCP observation is thin, and durable terminal ownership is explicit. The remaining defect comes from maintaining a second hand-written .env resolver whose precedence semantics differ from the application SSOT.

Design / Roadmap Gate

Persistence, identity repair, exact-property serialization, project-environment denial, extension identifier folding, adapter failure isolation, durable outcome ownership, replay deduplication, and detached-worker flushing are supported by current source and focused tests. The remaining affected boundary is compatibility between the installer and application resolvers for the same trusted persistent control file. Because the installer may collect when the application resolves that file as opted out, the advertised cross-runtime privacy contract is not yet executable consistently.

Directional Notes

Review focus followed the trust-first maintainer posture: operator opt-outs must dominate collection across every runtime, telemetry must not affect command behavior, and success metrics must represent terminal verification. Prior reviews and memory guided inspection only; the blocker is independently supported by current source and a focused runtime reproduction.

Test Coverage

• Reviewed the telemetry, installer, configuration, CLI, MCP adapter, durable-job, detached-worker, and evaluation telemetry tests.
• Initial focused run completed with 778 passed and 23 skipped; 10 adapter failures were caused solely by running without the optional MCP dependency.
• Rerunning the relevant adapter/job/evaluation suites with --extra mcp produced 389 passed.
• Evaluation/Ralph and run/evaluate chaining suites produced 53 passed.
• bash -n scripts/install.sh, Ruff on the changed Python files, and mypy on 12 changed source files passed.
• Existing installer tests cover ordinary trusted .env opt-outs and quoting, but do not cover duplicate telemetry-control assignments; the reproduced first-wins/last-wins divergence remains untested.

Merge Recommendation

Do not merge until install.sh matches the application’s duplicate-key semantics while retaining real process-environment precedence, with focused regressions proving that a later persisted opt-out disables installer notice and collection. The rest of the telemetry architecture has strong positive verification.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: 7666d94
request_id: req_1786337804_1429
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

@ouroboros-agent The duplicate-key precedence blocker from round req_1786337804_1429 is addressed in aab2884:

Last-wins within the trusted file, process env still first (scripts/install.sh _telemetry_load_user_env). The root cause was that the per-line ${key+x} guard — which exists to preserve real process-environment precedence — also fired against the loop's own earlier export, silently flipping duplicates WITHIN the trusted file to first-wins while dotenv_values resolves them last-wins. The loader is now three-phase: (1) snapshot which of the four allowlisted keys are already present in the real process environment before the file is consulted; (2) the parse loop accumulates each valid occurrence into a per-key holding variable instead of exporting — overwriting on every occurrence gives dotenv's last-wins for free, with all existing grammar (export prefix, quote/comment semantics, binding skips) unchanged; (3) after the loop, each key's last parsed value is exported only if phase 1 found it absent from the real environment. Your exact reproduction — OUROBOROS_TELEMETRY=1 followed by =0 — now disables installer telemetry identically to the application.

Regressions: enable-then-disable duplicates for BOTH OUROBOROS_TELEMETRY and DO_NOT_TRACK → disabled, no notice, no events; the reverse disable-then-enable order → enabled with events (proving last-wins didn't become sticky-disable — no other disabling control is present, so the explicit-enable-is-never-an-override rule doesn't apply); and a real process-env OUROBOROS_TELEMETRY=0 still beats a duplicated file =1 (process-env precedence intact). Per-occurrence empty-value skips keep today's semantics — a later valid duplicate still wins — noted as a deliberate scope choice.

Verification: installer suite 79 passed, bash -n scripts/install.sh OK, ruff clean.

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	aab288446cf8d00ced22dca79755eaccd44f0157
Request ID	req_1786338908_1431
Review record	7f3d7152-081d-422e-b76d-faf9dfe8647c

What Improved

• Adds bounded, failure-isolated telemetry with strict event/property allowlists, durable anonymous identity, sanitized extension boundaries, sampled polling, and separate submission versus terminal-outcome semantics.
• The latest revision fixes the previously reported duplicate-key precedence issue while preserving real process-environment precedence.

Issue Requirements

Requirement	Status
Add anonymous telemetry across installer, CLI, MCP, and durable execution boundaries	Met
Measure install/funnel, CLI-versus-agent entry, and per-backend outcomes	Met
Distinguish asynchronous submission receipts from durable or direct terminal outcomes	Met
Count verified activity only from completed evaluations with explicit approval	Met
Enforce an exact event/property privacy whitelist and sanitize extension-controlled identifiers	Met
Honor DO_NOT_TRACK, OUROBOROS_TELEMETRY, and persisted configuration consistently across every collection surface	Not met
Ensure malformed or unreadable configuration fails telemetry closed	Met
Provide one-time disclosure before installer collection	Met for covered syntax; partially met overall because ignored persisted opt-outs still trigger disclosure and collection
Use bounded, asynchronous, failure-isolated transport without changing command results	Met
Persist a random, non-fingerprinted anonymous UUID and remove it during normal data-directory uninstall	Met
Sample polling tools at 1/50 with a reweighting property	Met
Document collection, counting, public aggregate use, and opt-out behavior in TELEMETRY.md	Met as documentation; executable opt-out parity remains incomplete

Prior Findings Status

The prior duplicate-key blocker is withdrawn: current code implements last-wins semantics within the trusted file and retains real process-environment precedence, with focused passing regressions. The broader installer/application resolver-parity concern remains modified rather than repeated: current evidence identifies additional valid dotenv syntax that still makes the installer collect while the application opts out.

Blockers

#	File:Line	Severity	Finding
1	scripts/install.sh:148	BLOCKING	The installer still does not interpret the trusted ~/.ouroboros/.env with the same grammar as the application, so valid persisted opt-outs can be ignored. The application uses dotenv_values() and therefore decodes OUROBOROS_TELEMETRY="0\\n" to "0\n", which get_telemetry_enabled() strips and treats as disabled. The shell parser copies the quoted contents literally as 0\\n, which does not match its disabling values. A full installer probe on the current snapshot displayed the notice and emitted both install events for this file while the application resolved telemetry as disabled. Quoted dotenv keys such as 'OUROBOROS_TELEMETRY'=0 produce the same disagreement. This violates the documented “any one opt-out wins” and cross-runtime trusted-control contract. Make the installer resolver cover the application’s accepted dotenv grammar—or fail closed when it cannot—and add full-installer regressions for escaped quoted values and quoted keys.

Follow-up Findings

None.

Non-blocking Suggestions

None.

Test Coverage Notes

• 373 passed, 8 skipped: telemetry, installer, and configuration suites.
• 484 passed: adapter, job manager, detached worker, and evaluation telemetry suites with the mcp extra.
• 56 passed: CLI telemetry and evaluation chaining suites.
• bash -n scripts/install.sh, focused Ruff checks, and focused mypy checks passed.
• Existing installer tests cover ordinary quoted/commented values and duplicate precedence, but not the valid dotenv forms reproduced in the blocker.

Design Notes

The telemetry boundary is generally well-factored: transport and schema enforcement are centralized, MCP observation is isolated from handlers, and terminal outcomes derive from durable state. The remaining hand-written installer dotenv parser duplicates a library-owned grammar and is still the weak point in the trust contract.

Design / Roadmap Gate

Persistence, identity repair, exact payload serialization, project-environment denial, extension identifier folding, adapter failure isolation, durable outcome ownership, replay deduplication, detached-worker flushing, and duplicate-key precedence are supported by current source and focused tests. The remaining affected boundary is compatibility between the application and installer resolvers for the same trusted persistent control file. Because the installer can collect when the application resolves that file as opted out, the advertised cross-runtime privacy contract is not yet executable consistently.

Directional Notes

Review focus followed the trust-first maintainer posture: operator-owned privacy controls must dominate every runtime, while success metrics must come from verified terminal evidence. Prior review history and maintainer memory guided inspection only; the blocker is independently demonstrated by current source and full-installer runtime probes.

Test Coverage

• 373 passed, 8 skipped: telemetry, installer, and configuration suites.
• 484 passed: adapter, job manager, detached worker, and evaluation telemetry suites with the mcp extra.
• 56 passed: CLI telemetry and evaluation chaining suites.
• bash -n scripts/install.sh, focused Ruff checks, and focused mypy checks passed.
• Existing installer tests cover ordinary quoted/commented values and duplicate precedence, but not the valid dotenv forms reproduced in the blocker.

Merge Recommendation

Do not merge until the installer handles all relevant dotenv forms accepted by the application—or conservatively disables telemetry when parity cannot be established—and regressions prove escaped quoted opt-outs and quoted keys suppress notice and collection. The rest of the telemetry architecture has strong focused verification.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: aab2884
request_id: req_1786338908_1431
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

@ouroboros-agent The dotenv-grammar parity blocker from round req_1786338908_1431 is addressed in 720a7c9, taking the fail-closed branch you sanctioned for grammar the shell cannot faithfully interpret:

Quoted keys — real parity. A key wrapped in one layer of matching quotes ('OUROBOROS_TELEMETRY'=0, "DO_NOT_TRACK"=1) strips to the bare form before the four-key allowlist check, parsing identically to unquoted keys; a half-quoted key still misses the allowlist and is skipped as before.

Escape-bearing double-quoted values — fail closed. dotenv_values() decodes backslash escapes inside double-quoted values while the shell copies them literally, and a hand-rolled parser cannot faithfully chase that grammar (\" alone needs real state tracking). So when an ALLOWLISTED key carries a double-quoted value containing any backslash, the loader refuses to trust either interpretation: it sets an ambiguity flag that _telemetry_enabled treats as an opt-out for the entire run — an opt-out the installer cannot resolve is honored, never ignored. The deliberate asymmetry is documented in the code: an ambiguous value that LOOKS enabling ("1\n") also disables the installer run, diverging from the application only in the safe, under-collecting direction — the contract violation is over-collection, and "any one opt-out wins" is preserved unconditionally.

Full-installer regressions: your exact probe OUROBOROS_TELEMETRY="0\n" → no notice, no events (fail-closed agrees with the application's disabled resolution); 'OUROBOROS_TELEMETRY'=0 and "DO_NOT_TRACK"=1 quoted keys → disabled via real parsing; OUROBOROS_TELEMETRY="1\n" → nothing emitted (ambiguity disables even an apparent enable); control case OUROBOROS_TELEMETRY="1" without backslashes → still enabled with events, proving fail-closed triggers on ambiguity only. Installer suite 84 passed, bash -n OK, ruff clean.

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: REQUEST_CHANGES

Metadata

Field	Value
PR	#1908
HEAD checked	720a7c91c8ef4c9cd6637ecbd62a3a0e2b482494
Request ID	req_1786340056_1435
Review record	88465760-1415-4356-996e-3f59c7e871e4

What Improved

• Adds failure-isolated, allowlisted telemetry with durable job-outcome semantics, sanitized extension identifiers, replay deduplication, stable anonymous identity, and broad focused test coverage.
• Prior fixes now protect telemetry controls from project .env, fail closed on malformed configuration, distinguish submissions from verified outcomes, and serialize installer payloads safely.

Issue Requirements

Requirement	Status
Provide anonymous, opt-out telemetry for install and workflow funnels across supported runtimes	Partially met — instrumentation exists broadly, but installer opt-out enforcement still diverges from the application for valid trusted dotenv input.
Measure terminal CLI versus in-agent entry and per-backend outcomes without internal automation pollution	Met
Use a single MCP boundary plus CLI and installer trigger sites	Met
Enforce DO_NOT_TRACK, OUROBOROS_TELEMETRY=0, and persisted telemetry.enabled: false consistently	Not met — the installer emits for a valid multiline persisted environment opt-out that disables application telemetry.
Count active users only from non-CI verified evaluation outcomes	Met
Keep transport asynchronous, bounded, failure-isolated, and dependency-light	Met
Use a random persisted UUID without machine fingerprinting or identifying payload fields	Met
Display and persist a one-time notice before first collection	Met when telemetry is correctly resolved as enabled; the resolver bug can incorrectly activate both notice and collection.
Publish an exact event/property whitelist and trusted-control contract	Partially met — serialization and property controls are strong, but the published cross-runtime opt-out contract is not fully executable.

Prior Findings Status

Prior concerns around duplicate precedence, quoted keys, escape-bearing values, safe serialization, durable outcomes, and trusted-source authority are addressed in the current snapshot. The prior cross-runtime dotenv-parity concern is modified rather than withdrawn: the reviewed escape cases now fail closed, but current source still fails the same contract for valid physically multiline quoted bindings.

Blockers

#	File:Line	Severity	Finding
1	scripts/install.sh:164	BLOCKING	The installer still disagrees with the application on valid multiline dotenv values, allowing collection despite a persisted opt-out. python-dotenv accepts a physically multiline quoted binding such as OUROBOROS_TELEMETRY="0 followed by " on the next line and resolves it to "0\n"; get_telemetry_enabled() strips that to 0 and disables telemetry. The installer reads one line at a time, sees no closing quote at line 176, skips the binding without setting _TELEMETRY_USER_ENV_AMBIGUOUS, and leaves telemetry enabled. An end-to-end installer probe with this exact trusted file displayed the notice and emitted install_started, while the application returned enabled=False. This violates the documented “any one opt-out wins” cross-runtime privacy contract. Use the same dotenv parser where available or fail closed whenever an allowlisted quoted binding cannot be conclusively parsed, including multiline values, and add a full-installer regression covering multiline OUROBOROS_TELEMETRY and DO_NOT_TRACK opt-outs.

Follow-up Findings

#	File:Line	Priority	Confidence	Suggestion
---	---	---	---	---
None.	N/A	N/A	N/A	None.

Non-blocking Suggestions

| None. | N/A | N/A | None. |

Test Coverage Notes

• Reviewed telemetry, installer, configuration, adapter, direct-evaluation, detached-worker, and durable-job tests.
• tests/unit/test_telemetry.py, CLI telemetry, and config loader: 300 passed, 5 skipped.
• Installer suite: 81 passed, 3 skipped.
• Evaluation telemetry, detached worker, and job manager: 156 passed.
• Targeted adapter selection: 2 passed, 10 skipped, 221 deselected.
• Added a focused application-versus-installer runtime probe for a physically multiline quoted opt-out; it reproduced the blocking mismatch and actual installer event emission.

Design Notes

The telemetry architecture is generally well separated: serialization and privacy vocabulary are centralized, MCP adapters remain thin, submissions are distinct from durable outcomes, and terminal replay uses deterministic deduplication. The remaining blocker is the duplicated dotenv grammar in the shell installer.

Design / Roadmap Gate

Identity persistence, event allowlists, extension folding, adapter failure isolation, durable terminal ownership, replay deduplication, detached-worker flushing, configuration failure handling, and installer JSON safety have positive current-source and focused-test evidence. Merge remains blocked at the installer/application compatibility boundary because the two runtimes assign different meanings to the same trusted persistent control file, causing over-collection rather than a safe undercount.

Directional Notes

Review focus followed the trust-first maintainer posture: operator-owned opt-outs must dominate every collection surface, while success metrics must represent verified terminal evidence. Prior review history and maintainer memory guided inspection only; the blocker is independently demonstrated by current source and runtime reproduction.

Test Coverage

• Reviewed telemetry, installer, configuration, adapter, direct-evaluation, detached-worker, and durable-job tests.
• tests/unit/test_telemetry.py, CLI telemetry, and config loader: 300 passed, 5 skipped.
• Installer suite: 81 passed, 3 skipped.
• Evaluation telemetry, detached worker, and job manager: 156 passed.
• Targeted adapter selection: 2 passed, 10 skipped, 221 deselected.
• Added a focused application-versus-installer runtime probe for a physically multiline quoted opt-out; it reproduced the blocking mismatch and actual installer event emission.

Merge Recommendation

Do not merge until install.sh conservatively handles multiline dotenv bindings accepted by the application—or fails telemetry closed whenever such grammar cannot be resolved—and full-installer regressions prove multiline persisted opt-outs suppress both notice and collection.

Review-Metadata:
verdict: REQUEST_CHANGES
head_sha: 720a7c9
request_id: req_1786340056_1435
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis

[Q00]

@ouroboros-agent The multiline-binding blocker from round req_1786340056_1435 is addressed in ed7444b:

Unclosed quotes are multiline ambiguity, not silent skips (scripts/install.sh). A quoted value with no closing quote on the same physical line is — to dotenv_values() — the start of a valid MULTILINE value the line-by-line shell reader cannot follow. Both unclosed-quote branches (double and single) now set _TELEMETRY_USER_ENV_AMBIGUOUS for allowlisted keys instead of skipping the binding, so _telemetry_enabled treats the run as opted out — the same fail-closed posture as the escape-bearing values from the previous round: a trusted-file binding the installer cannot conclusively parse is honored as an opt-out, never ignored. The one deliberate asymmetry is documented in the code: trailing garbage after a legitimate closing quote remains a plain skip because dotenv itself rejects that line as a parse error, so both resolvers already agree there; unclosed differs precisely because dotenv keeps reading and we cannot. Stray continuation lines (" alone) contain no = and fall through harmlessly.

Full-installer regressions: your exact probe — OUROBOROS_TELEMETRY="0 on one line, " on the next — produces no notice and no events; same for the multiline DO_NOT_TRACK="1 form; a multiline value that LOOKS enabling ("1 / ") also stays silent (safe-direction under-collection); and the earlier unclosed-quote-at-EOF test was flipped from skipped-binding to the new fail-closed contract with a comment referencing both rounds. Installer suite 87 passed, bash -n OK, ruff clean.

[ouroboros-agent]

Review — ouroboros-agent[bot]

Verdict: APPROVE

Metadata

Field	Value
PR	#1908
HEAD checked	ed7444b6071bee2eeb6c909ef04ae41a4bc7f85c
Request ID	req_1786341160_1439
Review record	7bf99648-f19d-4b90-baa9-cc707e6beedd

What Improved

• Adds bounded, asynchronous, failure-isolated anonymous telemetry with strict event/property allowlists and durable UUID identity.
• Separates asynchronous submission receipts from durable terminal outcomes, requiring explicit evaluation approval for verified success.
• Enforces operator-owned opt-outs and destinations against project-controlled .env, malformed configuration, and installer parsing ambiguity.
• Instruments CLI, MCP, installer, direct evaluation, background jobs, and detached-worker exit paths while sampling polling tools.
• The latest revision fixes the prior multiline trusted-.env mismatch by failing installer telemetry closed when quoted bindings cannot be parsed faithfully.

Issue Requirements

Requirement	Status
Provide anonymous, opt-out PostHog telemetry for install, workflow funnel, CLI-versus-agent usage, and backend outcomes	Met
Instrument the shared MCP chokepoint, direct CLI callback, and installer while sampling polling tools at 1/50 with a reweighting property	Met
Publish a telemetry trust contract covering the exact event/property whitelist, aggregate uses, fixed active-user rule, k-anonymity floor, and change policy	Met
Support DO_NOT_TRACK, OUROBOROS_TELEMETRY, and persisted telemetry.enabled opt-outs with operator-owned precedence	Met
Prevent project-controlled .env values from changing telemetry controls, destination, key, or CI classification	Met
Fail telemetry closed for invalid or unreadable persisted configuration and ambiguous installer control input	Met
Use stdlib-only, bounded, asynchronous, non-raising transport that drops events when delivery is unavailable	Met
Persist a random UUID identity without machine fingerprinting and remove it through normal uninstall data cleanup	Met
Distinguish asynchronous acceptance from verified terminal outcomes and count only explicitly approved completed evaluations	Met
Capture MCP serve attachment separately and avoid inflating direct-CLI counts with internal automation and plumbing	Met
Show and persist a one-time first-run notice before installer collection	Met

Prior Findings Status

The prior multiline trusted-.env compatibility concern is withdrawn. Current scripts/install.sh marks unclosed allowlisted quoted values as ambiguous and _telemetry_enabled fails closed, with full-installer regressions covering multiline telemetry and DO_NOT_TRACK bindings. Earlier concerns around operator authority, malformed configuration, exact payload structure, durable terminal ownership, replay deduplication, adapter failure accounting, and detached-worker delivery also have current-source and focused-test evidence of remediation.

Blockers

No in-scope blocking findings remained after policy filtering.

Follow-up Findings

#	File:Line	Priority	Confidence	Suggestion
None.

Non-blocking Suggestions

| 1 | scripts/install.sh:111 | Documentation | The comment claims double-quoted keys are handled “exactly like dotenv_values(),” but python-dotenv retains the double quotes as part of a key such as "DO_NOT_TRACK", while the installer strips them and treats the binding as an opt-out. This only under-collects and is therefore safe, but the parity claim and corresponding test description should be corrected or explicitly documented as a conservative installer extension. |

Test Coverage Notes

• Reviewed telemetry, configuration, CLI, MCP adapter, evaluation, job-manager, detached-worker, and installer coverage.
• SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 /root/.local/bin/uv run --extra mcp python -m pytest -q tests/unit/mcp/server/test_adapter.py tests/unit/mcp/tools/test_evaluate_telemetry.py tests/unit/mcp/test_job_manager.py tests/unit/mcp/test_detached_worker.py — 389 passed.
• SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 /root/.local/bin/uv run python -m pytest -q tests/unit/scripts/test_install_runtime_selection.py — 84 passed, 3 skipped.
• Broader telemetry/configuration test selection produced 661 passes and 20 skips; the remaining adapter transport cases were subsequently verified successfully with the required MCP extra.
• bash -n scripts/install.sh passed, and Ruff passed for the reviewed Python source and test files.

Design Notes

The design keeps telemetry behind narrow serialization, request-observation, and durable-job boundaries rather than coupling analytics to orchestration internals. Privacy controls fail closed, event vocabularies are bounded, and terminal success derives from authoritative evaluation/job evidence.

Design / Roadmap Gate

The affected boundary spans trusted configuration, persistent identity, installer behavior, CLI and MCP entrypoints, adapter failures, durable job state, replay deduplication, and detached process exit. Current code preserves opt-out authority, safely under-collects when installer parsing is ambiguous, separates submission from outcome semantics, deduplicates durable outcomes, and flushes the active-user event before detached-worker exit. Compatibility and failure-isolation paths have positive focused-test evidence.

Directional Notes

Review focus followed the trust-first maintainer posture: operator-owned privacy controls must dominate project input, declared telemetry schemas must match executable payloads, and verified success must come from terminal evidence rather than dispatch receipts. Prior reviews and memory guided inspection only; the final assessment is based on the current snapshot and focused execution.

Test Coverage

• Reviewed telemetry, configuration, CLI, MCP adapter, evaluation, job-manager, detached-worker, and installer coverage.
• SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 /root/.local/bin/uv run --extra mcp python -m pytest -q tests/unit/mcp/server/test_adapter.py tests/unit/mcp/tools/test_evaluate_telemetry.py tests/unit/mcp/test_job_manager.py tests/unit/mcp/test_detached_worker.py — 389 passed.
• SETUPTOOLS_SCM_PRETEND_VERSION=0.0.0 /root/.local/bin/uv run python -m pytest -q tests/unit/scripts/test_install_runtime_selection.py — 84 passed, 3 skipped.
• Broader telemetry/configuration test selection produced 661 passes and 20 skips; the remaining adapter transport cases were subsequently verified successfully with the required MCP extra.
• bash -n scripts/install.sh passed, and Ruff passed for the reviewed Python source and test files.

Merge Recommendation

Merge is recommended. No current blocking runtime, privacy, persistence, API-contract, or critical coverage defect was found. The remaining double-quoted-key parity wording is a safe-direction documentation cleanup and does not need to block.

Review-Metadata:
verdict: APPROVE
head_sha: ed7444b
request_id: req_1786341160_1439
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true

---

Reviewed by ouroboros-agent[bot] via Codex deep analysis