Skip to content

fix(logging): redact agent registry exception telemetry - #1709

Draft
seonghobae wants to merge 3 commits into
fix/structured-exception-telemetryfrom
fix/agent-registry-redacted-telemetry
Draft

seonghobae wants to merge 3 commits into
fix/structured-exception-telemetryfrom
fix/agent-registry-redacted-telemetry

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Refs #1698.

Current authority

Verified finding

The agent registry loader still used ordinary exc_info=True for OSError and json.JSONDecodeError, and included the registry path in those exception-bearing log messages. Normal Python logging can render the original exception value and traceback, so provider/OS failures can expose credentials, connection strings or internal path material into the normal log sink. This is one of the remaining sink-by-sink migrations required by #1698; it is not a reason to fork #1700's telemetry contract.

The FileNotFoundError branch is intentionally unchanged because it attaches no exception information and reports the deterministic missing registration file.

Minimal causal repair

The two exception-bearing branches now consume #1700's core.safe_logging.redacted_exception_info(error) helper. The rendered record keeps a bounded operation label, exception class and one-way failure-site fingerprint while dropping raw exception value, raw traceback and the registry path from these failure records. No public/API response behavior or registry parsing semantics change.

Focused regressions drive a real logging.Formatter and inject API-key-shaped data, a PostgreSQL connection string, token-bearing malformed JSON and secret-bearing paths. They require the operation label, exception type and exception_fingerprint= while rejecting the injected values and paths from rendered output.

Doctoring records the scope, rejected alternatives and CWE-532 / OWASP Logging Cheat Sheet traceability. No helper copy or second telemetry contract is introduced.

Independent review

CodeRabbit reviewed exactly 2d6800aea110ac04eeedcd5faa91479f62877408 → fc56d609045e47093fdf455234121eb7cfe31715, selected all three effective files, produced no review thread, and submitted formal APPROVED review PRR_kwDOSNjZ2s8AAAABN2-vow at 2026-09-16T15:51:58Z. This is exact-current-head review evidence only; it does not substitute for hosted required contexts or protected-lineage integration.

Evidence boundary

Exact fc56d609... still has zero PR-triggered repository workflow runs because this PR is stacked on #1700 and the repository-local stacked-PR trigger defect remains owned by #1691. Test source and exact-head independent review are GREEN, but no hosted GREEN is claimed. Parent #1700 workflow/review evidence does not transfer.

Keep Draft until the canonical exception-redaction/telemetry lineage reaches protected ancestry and the unchanged final head has every then-live required context terminal-success plus qualifying post-last-push independent review. No temporary develop retarget, copied workflow, dummy/no-op evidence commit, synthetic status, self-approval, force push, destructive rebase, or gate weakening.

Summary by CodeRabbit

  • Bug Fixes

    • Improved error logging for agent registry read and JSON parsing failures by hiding sensitive exception details and file paths while retaining useful diagnostics.
  • Documentation

    • Added documentation describing the updated exception telemetry behavior and its verification coverage.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: e97fe326-e82a-4036-ac5c-564d3aec57df

📥 Commits

Reviewing files that changed from the base of the PR and between 2d6800a and fc56d60.

📒 Files selected for processing (3)
  • backend/services/agent_registry.py
  • backend/tests/test_agent_registry.py
  • docs/doctoring/agent-registry-exception-telemetry.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The registry now uses redacted exception metadata for read and JSON parsing failures. Tests verify that logs retain failure type and fingerprint without exposing exception contents or paths. Documentation records the logging boundary and unchanged missing-file handling.

Changes

Agent registry exception telemetry

Layer / File(s) Summary
Redacted registry failure logging
backend/services/agent_registry.py
OSError and JSONDecodeError handlers now pass redacted_exception_info(exc) to logger.debug. Both handlers still return {}.
Telemetry verification and documentation
backend/tests/test_agent_registry.py, docs/doctoring/agent-registry-exception-telemetry.md
Tests verify rendered logs without exception contents, secrets, credentials, or paths. Documentation records the telemetry boundary and unchanged FileNotFoundError handling.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to fc56d

The registry continues to recover from read and parse failures without logging raw exception details, payloads, or paths. No actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: redacting agent registry exception telemetry in logging.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/agent-registry-redacted-telemetry

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Exact review target: 2d6800aea110ac04eeedcd5faa91479f62877408..fc56d609045e47093fdf455234121eb7cfe31715. Please review only the three-file agent-registry telemetry delta against the #1700 structured exception contract; parent evidence must not transfer.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='2d6800aea110ac04eeedcd5faa91479f62877408'
head='fc56d609045e47093fdf455234121eb7cfe31715'

echo '=== exact changed files ==='
git diff --name-status "$base" "$head"

echo '=== exact diff ==='
git diff --no-ext-diff --unified=80 "$base" "$head" -- \
  backend/services/agent_registry.py \
  backend/tests/test_agent_registry.py \
  docs/doctoring/agent-registry-exception-telemetry.md

echo '=== agent registry outline ==='
ast-grep outline backend/services/agent_registry.py --items all || true

echo '=== redacted_exception_info definition and local call sites ==='
rg -n -A35 -B10 --glob '*.py' '\bdef\s+redacted_exception_info\s*\(|\bredacted_exception_info\s*\(' core backend

echo '=== helper module outline ==='
helper=$(fd -t f 'safe_logging\.py' . | head -n 1)
if [ -n "$helper" ]; then
  ast-grep outline "$helper" --items all || true
fi

Length of output: 52876


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='2d6800aea110ac04eeedcd5faa91479f62877408'
head='fc56d609045e47093fdf455234121eb7cfe31715'

echo '=== exact diff whitespace validation ==='
git diff --check "$base" "$head" -- \
  backend/services/agent_registry.py \
  backend/tests/test_agent_registry.py \
  docs/doctoring/agent-registry-exception-telemetry.md

echo '=== safe logging module ==='
cat -n backend/core/safe_logging.py

echo '=== test framework configuration candidates ==='
fd -t f -a 'pyproject.toml|pytest.ini|tox.ini|setup.cfg' . -d 3 | while IFS= read -r file; do
  echo "--- $file"
  rg -n -C2 'pytest|testpaths|pythonpath|addopts' "$file" || true
done

echo '=== exact commit parent relationship ==='
git merge-base --is-ancestor "$base" "$head"
printf 'base_is_ancestor=yes\n'
printf 'merge_base='
git merge-base "$base" "$head"
printf 'changed_file_count='
git diff --name-only "$base" "$head" | wc -l

Length of output: 4115


@seonghobae Reviewed only 2d6800aea110ac04eeedcd5faa91479f62877408..fc56d609045e47093fdf455234121eb7cfe31715.

I found no issues in the three-file agent-registry telemetry delta.

backend/services/agent_registry.py uses the canonical core.safe_logging.redacted_exception_info(exc) helper for both exception-bearing sinks. The helper emits the exception class and exception_fingerprint= without the original exception value or traceback. The revised log messages do not include the registry path.

The focused tests format real log records and reject the injected secret values and paths. The documentation matches the implementation and the stated sink-specific scope.

Static validation passed for the exact diff. I did not execute repository tests. I did not use or transfer parent workflow or approval evidence.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae seonghobae added bug Something isn't working priority: high High-priority or P1 work labels Sep 19, 2026 — with ChatGPT Codex Connector

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: high High-priority or P1 work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant