Skip to content

Add introspection tools for internal agent - #543

Merged
isiddharthsingh merged 17 commits into
mainfrom
feature/introspection-tools
Jun 26, 2026
Merged

Add introspection tools for internal agent#543
isiddharthsingh merged 17 commits into
mainfrom
feature/introspection-tools

Conversation

@OlivierTrudeau

@OlivierTrudeau OlivierTrudeau commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Gives the internal agent (Slack, web chat, RCA) direct read access to incidents, services, actions, metrics, and findings — same data MCP clients get, without HTTP round-trips. Also fixes Slack leaving a stuck "Thinking..." message when the input guardrail blocks a request.

Test plan

  • Mention Aurora in Slack with a normal question — get a reply
  • Send a prompt that trips the guardrail — thinking message updates with the refusal text
  • Ask Aurora to list incidents / pull metrics in web chat

Made with Cursor

Summary by CodeRabbit

  • New Features
    • Added internal introspection tools to surface incidents, service graphs, actions/runs, metrics, RCA findings, postmortems, knowledge base memory, and Grafana alerts.
  • Bug Fixes
    • Improved Slack/Google Chat fallback messaging when guardrails block a response.
    • Refined RCA sub-agent tool-call history rendering for more consistent command/output excerpts.
    • Standardized UTC timestamp formatting and UUID validation across key endpoints.
  • Maintenance
    • Improved cloud tool caching by action/incident context.
    • Broadened and tightened RCA triggering rules; updated Notion postmortem export property handling.

OlivierTrudeau and others added 3 commits June 20, 2026 13:45
Persist a refusal message when input rails block background chats so Slack replaces the thinking placeholder instead of hanging. Colocate each introspection Args schema with its tool function.

Co-authored-by: Cursor <cursoragent@cursor.com>
@OlivierTrudeau
OlivierTrudeau requested a review from a team as a code owner June 20, 2026 20:47
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR adds shared helpers for validation, timestamp formatting, truncation, metrics periods, and tool history, then updates routes and agent tools to use them. It also adds introspection tools, wires them into cloud tooling, refactors Notion postmortem action-item handling, and changes background guardrail message delivery.

Changes

Shared utilities, introspection tools, and agent/route refactoring

Layer / File(s) Summary
Create shared utility foundation
server/utils/validation.py, server/utils/query_helpers.py, server/utils/text/text_utils.py, server/utils/metrics_periods.py, server/chat/backend/agent/utils/tool_call_history.py
Creates shared helpers and constants for UUID validation, UTC serialization, query shaping, truncation, metric periods, and tool-call history rendering.
Adopt shared tool history serialization
server/chat/backend/agent/orchestrator/sub_agent.py, server/chat/backend/agent/utils/tool_context_capture.py, server/routes/incidents_findings.py
Replaces local tool-history command, excerpt, and row-shaping logic with shared history/truncation helpers.
Implement introspection tool suite
server/chat/backend/agent/tools/introspection_tools.py
Adds introspection tool infrastructure and 19 tool functions for incidents, services, actions, metrics, RCA findings, postmortems, knowledge base memory, and Grafana alerts.
Wire introspection tools and RCA gating
server/chat/backend/agent/tools/cloud_tools.py, server/chat/backend/agent/tools/trigger_rca_tool.py
Registers introspection tools in cloud tooling, expands tool cache keys, broadens trigger_rca registration, and changes the background RCA guard condition.
Refactor routes and postmortem tool to shared helpers
server/routes/actions/actions_routes.py, server/routes/artifact_routes.py, server/routes/audit_routes.py, server/routes/incident_feedback/routes.py, server/routes/incidents_routes.py, server/routes/metrics_routes.py, server/routes/postmortem_routes.py, server/chat/backend/agent/tools/postmortem_tool.py
Switches UUID validation and timestamp serialization from local helpers to shared utilities across the route and postmortem modules.
Enhance Notion postmortem property coercion
server/chat/backend/agent/tools/notion/postmortem.py
Adds schema-aware Notion property coercion and merge helpers, updates action-item counts, and switches incident ID validation to the shared UUID helper.
Add guardrail fallback delivery for background channels
server/chat/background/task.py
Adds fallback guardrail message handling for Slack and Google Chat, including action-specific session messages for blocked action-triggered runs.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • Arvo-AI/aurora#156: Both PRs touch RCA-related tool output handling, including shared truncation and excerpt logic in tool_context_capture.py.
  • Arvo-AI/aurora#202: Both PRs modify trigger_rca availability and background-session behavior in cloud_tools.py and trigger_rca_tool.py.
  • Arvo-AI/aurora#379: Both PRs involve postmortem tooling and the get_postmortem / save_postmortem flow used for postmortem management.

Suggested reviewers

  • beng360
  • Zarlanx

Poem

🐇 I hopped through helpers, neat and new,
With UUIDs, timestamps, and truncation too.
I sniffed out tools in the cloudy dew,
Then passed them through with a fresher view.
Guardrails now murmur when messages stall,
And this rabbit does a happy thump for all.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 72.34% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding internal agent introspection tools.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/introspection-tools

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.

Comment thread server/chat/background/task.py Fixed
…once'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
server/chat/backend/agent/tools/notion/postmortem.py (1)

250-334: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix _create_action_items return contract to avoid runtime crashes.

_create_action_items returns 0 on the empty-items path but returns a dict elsewhere. Both callers dereference with .get(...), so the empty path triggers AttributeError and fails valid requests with no unchecked action items.

💡 Proposed fix
-def _create_action_items(
+def _create_action_items(
     client: NotionClient,
     postmortem_md: str,
     action_items_database_id: str,
-) -> int:
+) -> Dict[str, int]:
     """Create one Notion page per unchecked action item in the target DB.
 
-    Returns the number of pages successfully created.
+    Returns counts of created/failed pages.
     """
     items = parse_action_items(postmortem_md or "")
     if not items:
-        return 0
+        return {"created": 0, "failed": 0}
@@
-    return {"created": created, "failed": failed}
+    return {"created": created, "failed": failed}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/chat/backend/agent/tools/notion/postmortem.py` around lines 250 - 334,
The _create_action_items function has an inconsistent return type that causes
AttributeError in callers. When items is empty, the function returns the integer
0, but at the end of the function it returns a dictionary with "created" and
"failed" keys. Since callers invoke .get() on the return value expecting a
dictionary, the early return path fails. Change the early return statement when
items is empty to return a dictionary {"created": 0, "failed": 0} instead of
returning 0 to maintain consistency with the final return statement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@server/chat/backend/agent/tools/cloud_tools.py`:
- Around line 1324-1327: The tool registration logic at line 1326 exposes
trigger_rca to background sessions via the is_background condition, but the
trigger_rca function itself returns an error whenever state.is_background is
true, creating a contradiction. Either remove the is_background condition from
the tool registration check to keep trigger_rca UI-gated only, or modify the
guard logic inside the trigger_rca function itself to be more specific and only
block background RCA recursion rather than all background executions, ensuring
the tool behavior aligns with when it is registered.
- Around line 1821-1841: The imports in the introspection_tools section are
using names that conflict with incident.io tool schemas, specifically
ListIncidentsArgs and GetIncidentArgs. To fix this, alias the introspection tool
imports using the `as` keyword to give them distinct names that won't overwrite
the incident.io schemas (for example, prefix them with "Introspection" such as
`ListIncidentsArgs as IntrospectionListIncidentsArgs` and `GetIncidentArgs as
IntrospectionGetIncidentArgs`). Apply this aliasing consistently to all imported
Args classes from introspection_tools to ensure the incident.io tools receive
their correct schemas rather than the introspection ones.
- Around line 1329-1331: The condition on line 1331 uses `if is_background or
_action_id:` which registers trigger_action for all background sessions
regardless of whether _action_id exists, causing the wrapper to pin a None value
and ignore the caller's supplied action_id. Fix this by requiring _action_id to
actually exist before registering trigger_action, either by changing the
condition to only check `if _action_id:` (registering only when an action id is
explicitly provided), or to `if is_background and _action_id:` (registering only
for background sessions with a pinned action id), depending on the intended
behavior for unpinned background sessions.

In `@server/chat/backend/agent/tools/introspection_tools.py`:
- Around line 101-116: The list_incidents function always excludes merged
incidents in the WHERE clause regardless of user input, which prevents the
advertised status="merged" filter from working. Modify the WHERE clause
construction in list_incidents to conditionally include the i.status != 'merged'
exclusion only when the status parameter is None (i.e., when no explicit status
filter is provided). When a user provides an explicit status value, omit that
exclusion so merged incidents can be returned if that's what was requested.

In `@server/chat/background/task.py`:
- Around line 1980-1983: The code iterates through the messages list and calls
.get() on each msg element without verifying it is a dictionary first. If a
legacy or corrupt row contains a non-dict item in the messages list, this will
raise an AttributeError and abort execution. Guard both the message iteration
loop starting at line 1980 (where msg.get('sender') and
msg.get('text')/msg.get('content') are called) and the similar iteration at
lines 2113-2116 by adding an isinstance check to ensure each msg is a dict
before attempting to call .get() on it. Only process the message if it passes
the dict type check.

---

Outside diff comments:
In `@server/chat/backend/agent/tools/notion/postmortem.py`:
- Around line 250-334: The _create_action_items function has an inconsistent
return type that causes AttributeError in callers. When items is empty, the
function returns the integer 0, but at the end of the function it returns a
dictionary with "created" and "failed" keys. Since callers invoke .get() on the
return value expecting a dictionary, the early return path fails. Change the
early return statement when items is empty to return a dictionary {"created": 0,
"failed": 0} instead of returning 0 to maintain consistency with the final
return statement.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e9a50961-58e5-4675-92cd-2896cab92cce

📥 Commits

Reviewing files that changed from the base of the PR and between 6f55949 and a3bca1b.

📒 Files selected for processing (20)
  • server/chat/backend/agent/orchestrator/sub_agent.py
  • server/chat/backend/agent/tools/cloud_tools.py
  • server/chat/backend/agent/tools/introspection_tools.py
  • server/chat/backend/agent/tools/notion/postmortem.py
  • server/chat/backend/agent/tools/postmortem_tool.py
  • server/chat/backend/agent/utils/tool_call_history.py
  • server/chat/backend/agent/utils/tool_context_capture.py
  • server/chat/background/task.py
  • server/routes/actions/actions_routes.py
  • server/routes/artifact_routes.py
  • server/routes/audit_routes.py
  • server/routes/incident_feedback/routes.py
  • server/routes/incidents_findings.py
  • server/routes/incidents_routes.py
  • server/routes/metrics_routes.py
  • server/routes/postmortem_routes.py
  • server/utils/metrics_periods.py
  • server/utils/query_helpers.py
  • server/utils/text/text_utils.py
  • server/utils/validation.py

Comment thread server/chat/backend/agent/tools/cloud_tools.py
Comment thread server/chat/backend/agent/tools/cloud_tools.py Outdated
Comment thread server/chat/backend/agent/tools/cloud_tools.py
Comment thread server/chat/backend/agent/tools/introspection_tools.py Outdated
Comment thread server/chat/background/task.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
server/chat/backend/agent/tools/notion/postmortem.py (1)

59-89: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Harden coercion for date/email/url/checkbox to avoid invalid or incorrect Notion writes.

_coerce_property_value currently accepts arbitrary strings for date/email/url and uses bool(value) for checkbox, which can silently convert "false" to True. This can either fail downstream at Notion API time or persist incorrect values.

Suggested fix
 def _coerce_property_value(prop_meta: Dict[str, Any], value: Any) -> Optional[Dict[str, Any]]:
@@
-        if prop_type == "date":
-            iso = value if isinstance(value, str) else str(value)
-            return {"date": {"start": iso}}
+        if prop_type == "date":
+            if hasattr(value, "isoformat"):
+                iso = value.isoformat()
+            elif isinstance(value, str):
+                iso = value.strip()
+                if not iso:
+                    return None
+            else:
+                return None
+            return {"date": {"start": iso}}
@@
-        if prop_type == "checkbox":
-            return {"checkbox": bool(value)}
+        if prop_type == "checkbox":
+            if isinstance(value, bool):
+                return {"checkbox": value}
+            if isinstance(value, str):
+                normalized = value.strip().lower()
+                if normalized in {"true", "1", "yes", "y"}:
+                    return {"checkbox": True}
+                if normalized in {"false", "0", "no", "n"}:
+                    return {"checkbox": False}
+                return None
+            if isinstance(value, (int, float)):
+                return {"checkbox": bool(value)}
+            return None
@@
-        if prop_type == "url":
-            return {"url": str(value)}
-        if prop_type == "email":
-            return {"email": str(value)}
+        if prop_type == "url":
+            url = str(value).strip()
+            if not url:
+                return None
+            return {"url": url}
+        if prop_type == "email":
+            email = str(value).strip()
+            if not _looks_like_email(email):
+                return None
+            return {"email": email}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/chat/backend/agent/tools/notion/postmortem.py` around lines 59 - 89,
The _coerce_property_value function needs to add proper validation for date,
email, url, and checkbox property types. For the date property type, validate
that the ISO string is in a valid date format. For email and url property types,
add validation to ensure the values match expected email and URL formats rather
than accepting arbitrary strings. For the checkbox property type, replace the
simple bool(value) call with logic that correctly handles string representations
of booleans (such as "false" which should evaluate to False, not True as bool()
would return).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@server/chat/backend/agent/tools/cloud_tools.py`:
- Around line 1329-1332: The tool-cache key generation at lines 1034 and 1036
does not include the trigger_action_id, but the tool registration logic at line
1331 uses _action_id to conditionally add the trigger_action tool. This mismatch
causes cached tools from one action to be reused for a different action, pinning
trigger_action to a stale action id. Include the trigger_action_id value
(obtained from state_context if available, similar to how _action_id is
obtained) as part of the cache key computation at both cache key generation
locations to ensure separate cache entries for different action IDs.

---

Outside diff comments:
In `@server/chat/backend/agent/tools/notion/postmortem.py`:
- Around line 59-89: The _coerce_property_value function needs to add proper
validation for date, email, url, and checkbox property types. For the date
property type, validate that the ISO string is in a valid date format. For email
and url property types, add validation to ensure the values match expected email
and URL formats rather than accepting arbitrary strings. For the checkbox
property type, replace the simple bool(value) call with logic that correctly
handles string representations of booleans (such as "false" which should
evaluate to False, not True as bool() would return).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0ee2ae8c-ee3f-4c39-ae95-902a3a53447d

📥 Commits

Reviewing files that changed from the base of the PR and between 2baf879 and 70c1996.

📒 Files selected for processing (5)
  • server/chat/backend/agent/tools/cloud_tools.py
  • server/chat/backend/agent/tools/introspection_tools.py
  • server/chat/backend/agent/tools/notion/postmortem.py
  • server/chat/backend/agent/tools/trigger_rca_tool.py
  • server/chat/background/task.py

Comment thread server/chat/backend/agent/tools/cloud_tools.py
OlivierTrudeau and others added 7 commits June 22, 2026 12:08
Persist a refusal message when input rails block background chats so Slack replaces the thinking placeholder instead of hanging. Colocate each introspection Args schema with its tool function.

Co-authored-by: Cursor <cursoragent@cursor.com>
…once'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
@OlivierTrudeau
OlivierTrudeau force-pushed the feature/introspection-tools branch from 711bf64 to 4fefbeb Compare June 22, 2026 17:06
…key conflicts

Keep bool return from early Slack send, fallback_text for guardrail blocks,
and stable capture_tag cache keys with action_id differentiation.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread server/chat/backend/agent/tools/trigger_rca_tool.py Outdated
Comment thread server/chat/backend/agent/tools/cloud_tools.py Outdated
Comment thread server/chat/backend/agent/tools/cloud_tools.py
Comment thread server/chat/backend/agent/tools/introspection_tools.py

@arvo-ai-staging arvo-ai-staging Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aurora Risk Review

Verdict: RISKY

The new introspection tools are well-structured and the Slack guardrail fix is clean. However, two queries in introspection_tools.py — list_action_runs and get_action — omit an explicit org_id filter, relying solely on RLS to enforce tenant isolation. Every other query in the same file adds AND org_id = %s as defense-in-depth; these two do not, creating a cross-tenant data leak risk if action_runs or actions lack row-level security on the pooled connection type used here.

Findings

# Severity File Finding
1 HIGH server/chat/backend/agent/tools/introspection_tools.py:310 list_action_runs query has no org_id filter — cross-tenant run history exposure
2 HIGH server/chat/backend/agent/tools/introspection_tools.py:340 get_action query has no org_id filter — cross-tenant action config exposure

Aurora reviews PRs for incident prevention. This is advisory only and does not block merge.

Comment thread server/chat/backend/agent/tools/introspection_tools.py
Comment thread server/chat/backend/agent/tools/introspection_tools.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@server/chat/background/task.py`:
- Around line 1474-1479: Move the early Slack response path in
background/task.py so guardrail block handling happens before calling
_send_response_to_slack in the trigger_metadata/slack branch. Make sure the
logic around _append_block_message and the later post-processing path cannot be
skipped for blocked sessions, and pass fallback_text through the early send path
if needed so the placeholder message is updated even when
_send_response_to_slack returns False. Update the related slack send flow used
in the later block as well so both paths behave consistently.
- Around line 488-499: The dedup lock in the task execution path is being
removed on every exit, which allows an acks_late redelivery to repeat side
effects after a successful run. Update the dedup handling around the background
chat task logic in task.py so the key created with celery_app.backend.client.set
stays in Redis after successful completion and is only released safely on
failure using an owner token and compare-delete if cleanup is needed. Keep the
existing dedup flow tied to self.request.id, the _dedup_key/_dedup_acquired
flags, and the duplicate-execution skip path, but avoid unconditional deletion
in the finally block.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 60954936-e5df-4bfd-b9e6-f893628bde5e

📥 Commits

Reviewing files that changed from the base of the PR and between 70c1996 and 52cabbf.

📒 Files selected for processing (4)
  • server/chat/backend/agent/tools/cloud_tools.py
  • server/chat/backend/agent/tools/introspection_tools.py
  • server/chat/backend/agent/tools/trigger_rca_tool.py
  • server/chat/background/task.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@server/chat/background/task.py`:
- Around line 1474-1479: Move the early Slack response path in
background/task.py so guardrail block handling happens before calling
_send_response_to_slack in the trigger_metadata/slack branch. Make sure the
logic around _append_block_message and the later post-processing path cannot be
skipped for blocked sessions, and pass fallback_text through the early send path
if needed so the placeholder message is updated even when
_send_response_to_slack returns False. Update the related slack send flow used
in the later block as well so both paths behave consistently.
- Around line 488-499: The dedup lock in the task execution path is being
removed on every exit, which allows an acks_late redelivery to repeat side
effects after a successful run. Update the dedup handling around the background
chat task logic in task.py so the key created with celery_app.backend.client.set
stays in Redis after successful completion and is only released safely on
failure using an owner token and compare-delete if cleanup is needed. Keep the
existing dedup flow tied to self.request.id, the _dedup_key/_dedup_acquired
flags, and the duplicate-execution skip path, but avoid unconditional deletion
in the finally block.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 60954936-e5df-4bfd-b9e6-f893628bde5e

📥 Commits

Reviewing files that changed from the base of the PR and between 70c1996 and 52cabbf.

📒 Files selected for processing (4)
  • server/chat/backend/agent/tools/cloud_tools.py
  • server/chat/backend/agent/tools/introspection_tools.py
  • server/chat/backend/agent/tools/trigger_rca_tool.py
  • server/chat/background/task.py
🛑 Comments failed to post (2)
server/chat/background/task.py (2)

488-499: 🗄️ Data Integrity & Integration | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
python - <<'PY'
from pathlib import Path
p = Path("server/chat/background/task.py")
s = p.read_text()
finally_block = s[s.find("finally:"):s.find("# Safety net")]
print("owner token present:", "_dedup_token" in s)
print("atomic compare-delete present:", ".eval(" in finally_block and "redis.call('get'" in finally_block)
print("success is not deleted:", "not completed_successfully" in finally_block)
PY

Repository: Arvo-AI/aurora

Length of output: 246


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
p = Path("server/chat/background/task.py")
s = p.read_text().splitlines()
for start, end in [(480, 510), (930, 950)]:
    print(f"\n--- {start}-{end} ---")
    for i in range(start-1, min(end, len(s))):
        print(f"{i+1}: {s[i]}")
PY

Repository: Arvo-AI/aurora

Length of output: 2792


Keep the dedup marker through successful completion.
finally deletes celery:dedup:{self.request.id} on every exit, so an acks_late redelivery after a crash or lost ack can run the same side effects again. Keep successful markers until TTL expiry, and use an owner token/compare-delete if cleanup must happen on failure.

🧰 Tools
🪛 Ruff (0.15.18)

[warning] 497-497: Logging statement uses f-string

(G004)

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

In `@server/chat/background/task.py` around lines 488 - 499, The dedup lock in the
task execution path is being removed on every exit, which allows an acks_late
redelivery to repeat side effects after a successful run. Update the dedup
handling around the background chat task logic in task.py so the key created
with celery_app.backend.client.set stays in Redis after successful completion
and is only released safely on failure using an owner token and compare-delete
if cleanup is needed. Keep the existing dedup flow tied to self.request.id, the
_dedup_key/_dedup_acquired flags, and the duplicate-execution skip path, but
avoid unconditional deletion in the finally block.

1474-1479: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Handle guardrail blocks before the early Slack send.

The early Slack send runs before _append_block_message(), and it does not pass fallback_text; blocked sessions can return False with no placeholder update if the outer post-processing path does not run.

Suggested fix
+        guardrail_blocked = getattr(state, "guardrail_blocked", False)
+        if guardrail_blocked:
+            source = (trigger_metadata or {}).get("source", "")
+            block_text = (
+                _ACTION_GUARDRAIL_USER_MSG
+                if source == "action"
+                else _GUARDRAIL_USER_MSG
+            )
+            _append_block_message(session_id, user_id, block_text)
+
         # Send Slack response immediately — don't wait for post-processing
         _slack_early_sent = False
         if trigger_metadata and trigger_metadata.get('source') in ['slack', 'slack_button']:
             try:
-                _slack_early_sent = _send_response_to_slack(user_id, session_id, trigger_metadata)
+                _slack_early_sent = _send_response_to_slack(
+                    user_id,
+                    session_id,
+                    trigger_metadata,
+                    fallback_text=_GUARDRAIL_USER_MSG if guardrail_blocked else None,
+                )
             except Exception:
                 logger.exception("[BackgroundChat] Failed early Slack send")
@@
-        guardrail_blocked = getattr(state, "guardrail_blocked", False)
-        if guardrail_blocked:
-            # Streaming copies are stripped at end of workflow; persist a real bot
-            # message so Slack/Google Chat can replace the "Thinking..." placeholder.
-            source = (trigger_metadata or {}).get("source", "")
-            block_text = (
-                _ACTION_GUARDRAIL_USER_MSG
-                if source == "action"
-                else _GUARDRAIL_USER_MSG
-            )
-            _append_block_message(session_id, user_id, block_text)

Also applies to: 1553-1564

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

In `@server/chat/background/task.py` around lines 1474 - 1479, Move the early
Slack response path in background/task.py so guardrail block handling happens
before calling _send_response_to_slack in the trigger_metadata/slack branch.
Make sure the logic around _append_block_message and the later post-processing
path cannot be skipped for blocked sessions, and pass fallback_text through the
early send path if needed so the placeholder message is updated even when
_send_response_to_slack returns False. Update the related slack send flow used
in the later block as well so both paths behave consistently.

@arvo-ai-staging arvo-ai-staging Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aurora Risk Review — Latest changes

Verdict: SAFE

No new incident risk in the latest changes.


Aurora reviews PRs for incident prevention.

Resolve conflict in cloud_tools.py: keep main's cleaner
_is_background_rca call using the existing local is_background var.

Also add org_id filters to get_action and list_action_runs to fix
cross-tenant data exposure (review comments).

Co-authored-by: Cursor <cursoragent@cursor.com>

@arvo-ai-staging arvo-ai-staging Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aurora Risk Review — Latest changes

Verdict: SAFE

No new incident risk in the latest changes.


Aurora reviews PRs for incident prevention.

Comment thread server/chat/backend/agent/tools/trigger_rca_tool.py Outdated

@arvo-ai-staging arvo-ai-staging Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aurora Risk Review — Latest changes

Verdict: SAFE

No new incident risk in the latest changes.


Aurora reviews PRs for incident prevention.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
server/chat/backend/agent/tools/trigger_rca_tool.py (1)

75-84: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use the RCA predicate instead of incident_id alone.

This guard says it blocks nested RCA only, but is_background && incident_id also blocks non-RCA background sessions that happen to be incident-scoped. Reuse the same _is_background_rca(...) predicate used in cloud_tools.py so Actions/prediscovery-style background runs are not misclassified.

🐛 Proposed fix
-        from chat.backend.agent.tools.cloud_tools import get_state_context
+        from chat.backend.agent.tools.cloud_tools import get_state_context, _is_background_rca
         state = get_state_context()
         # Block nested RCA only — background Slack/Celery sessions without an
         # active incident investigation may still trigger a new RCA.
-        if (
-            state
-            and getattr(state, "is_background", False)
-            and getattr(state, "incident_id", None)
-        ):
+        if _is_background_rca(state, bool(getattr(state, "is_background", False))):
             return json.dumps({
                 "error": "Cannot trigger RCA from within a background RCA session. "
                 "Nested RCA is not supported — finish the current investigation first."
             })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/chat/backend/agent/tools/trigger_rca_tool.py` around lines 75 - 84,
The nested-RCA guard in trigger_rca_tool.py is using is_background plus
incident_id, which can wrongly block non-RCA background sessions. Update the
check in the RCA trigger path to reuse the same _is_background_rca(...)
predicate used in cloud_tools.py, and keep the existing error response only for
true background RCA sessions so Actions/prediscovery-style runs are not
misclassified.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@server/chat/backend/agent/tools/trigger_rca_tool.py`:
- Around line 75-84: The nested-RCA guard in trigger_rca_tool.py is using
is_background plus incident_id, which can wrongly block non-RCA background
sessions. Update the check in the RCA trigger path to reuse the same
_is_background_rca(...) predicate used in cloud_tools.py, and keep the existing
error response only for true background RCA sessions so
Actions/prediscovery-style runs are not misclassified.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 99a91df3-e5bb-4737-bd79-45a4eca84a35

📥 Commits

Reviewing files that changed from the base of the PR and between 52cabbf and 8fea670.

📒 Files selected for processing (3)
  • server/chat/backend/agent/tools/cloud_tools.py
  • server/chat/backend/agent/tools/introspection_tools.py
  • server/chat/backend/agent/tools/trigger_rca_tool.py

@isiddharthsingh
isiddharthsingh merged commit 424a229 into main Jun 26, 2026
19 checks passed
@isiddharthsingh
isiddharthsingh deleted the feature/introspection-tools branch June 26, 2026 17:23
@sonarqubecloud

Copy link
Copy Markdown

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants