fix(agent): add case artifact triggers#2806
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bb155edf9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ArtifactBinding( | ||
| tool_names=("core.cases.get_case_metrics",), | ||
| op="upsert", | ||
| build=_build_case_metrics_artifacts, |
There was a problem hiding this comment.
Handle metric requests that return no rows
This binding builds artifacts only from metric output rows, but core.cases.get_case_metrics can successfully validate requested cases and still return no row for a case (for example an open case, a case with no completed durations, or a workspace with no duration definitions; _format_time_series skips incomplete durations). In those cases a valid case_ids input produces no side effect, so the agent never surfaces the case artifact after a metrics call. Please fall back to the input case_ids for any requested case missing from the metric rows and resolve those identities like the other case-id based bindings.
Useful? React with 👍 / 👎.
|
✅ No security or compliance issues detected. Reviewed everything up to 2bb155e. Security Overview
Detected Code Changes
|
There was a problem hiding this comment.
2 issues found across 6 files
Confidence score: 3/5
- There is a concrete regression risk in
packages/tracecat-registry/tracecat_registry/core/ee/tasks.py:delete_tasknow requirescase:readpluscase:delete, which can block delete-only roles that previously worked. tracecat/artifacts/bindings.pyhas a medium-impact correctness gap where_build_case_metrics_artifactsmay emit nothing for valid requested cases whenget_case_metricsreturns no rows (for example, open/no-duration cases), leading to incomplete artifacts.- Given one high-severity RBAC behavior change (7/10, high confidence) and one medium data-output issue (5/10), this looks mergeable only with some caution rather than low risk.
- Pay close attention to
packages/tracecat-registry/tracecat_registry/core/ee/tasks.pyandtracecat/artifacts/bindings.py- permission checks may regress delete flows, and artifact generation may silently omit expected cases.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/tracecat-registry/tracecat_registry/core/ee/tasks.py">
<violation number="1" location="packages/tracecat-registry/tracecat_registry/core/ee/tasks.py:194">
P1: `delete_task` now requires `case:read` in addition to `case:delete`, introducing an RBAC regression for delete-only actors.</violation>
</file>
<file name="tracecat/artifacts/bindings.py">
<violation number="1" location="tracecat/artifacts/bindings.py:279">
P2: `_build_case_metrics_artifacts` only emits artifacts for cases present in the output rows, but `get_case_metrics` can return no rows for valid requested cases (e.g., open cases or cases with no completed durations). Cases in the input `case_ids` that are missing from the output will never surface an artifact. Consider falling back to the input `case_ids` for any requested case absent from metric rows, resolving their identities like the other case-id-based bindings.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| """Delete a case task.""" | ||
| await get_context().cases.delete_task(task_id) | ||
| ctx = get_context() | ||
| task = await ctx.cases.get_task(task_id) |
There was a problem hiding this comment.
P1: delete_task now requires case:read in addition to case:delete, introducing an RBAC regression for delete-only actors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/tracecat-registry/tracecat_registry/core/ee/tasks.py, line 194:
<comment>`delete_task` now requires `case:read` in addition to `case:delete`, introducing an RBAC regression for delete-only actors.</comment>
<file context>
@@ -188,6 +188,9 @@ async def delete_task(
"""Delete a case task."""
- await get_context().cases.delete_task(task_id)
+ ctx = get_context()
+ task = await ctx.cases.get_task(task_id)
+ await ctx.cases.delete_task(task_id)
+ return {"case_id": str(task["case_id"])}
</file context>
|
|
||
|
|
||
| def _build_case_metrics_artifacts(ctx: ArtifactProjectionContext) -> Iterable[Artifact]: | ||
| return _case_metric_artifacts_from_output(ctx.tool_output, ctx.tool_call_id) |
There was a problem hiding this comment.
P2: _build_case_metrics_artifacts only emits artifacts for cases present in the output rows, but get_case_metrics can return no rows for valid requested cases (e.g., open cases or cases with no completed durations). Cases in the input case_ids that are missing from the output will never surface an artifact. Consider falling back to the input case_ids for any requested case absent from metric rows, resolving their identities like the other case-id-based bindings.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tracecat/artifacts/bindings.py, line 279:
<comment>`_build_case_metrics_artifacts` only emits artifacts for cases present in the output rows, but `get_case_metrics` can return no rows for valid requested cases (e.g., open cases or cases with no completed durations). Cases in the input `case_ids` that are missing from the output will never surface an artifact. Consider falling back to the input `case_ids` for any requested case absent from metric rows, resolving their identities like the other case-id-based bindings.</comment>
<file context>
@@ -227,6 +259,40 @@ def _build_deleted_case_artifact(ctx: ArtifactProjectionContext) -> Artifact | N
+
+
+def _build_case_metrics_artifacts(ctx: ArtifactProjectionContext) -> Iterable[Artifact]:
+ return _case_metric_artifacts_from_output(ctx.tool_output, ctx.tool_call_id)
+
+
</file context>
Summary
core.cases.delete_taskso post-delete artifact projection can still upsert the caseTesting
uv run pytest tests/unit/test_artifacts.py tests/unit/test_agent_stream_artifacts.py tests/registry/test_core_cases.py -quv run ruff check tracecat/artifacts/bindings.py tracecat/artifacts/resolution.py packages/tracecat-registry/tracecat_registry/core/ee/tasks.py tests/unit/test_artifacts.py tests/unit/test_agent_stream_artifacts.py tests/registry/test_core_cases.pyuv run ruff format --check tracecat/artifacts/bindings.py tracecat/artifacts/resolution.py packages/tracecat-registry/tracecat_registry/core/ee/tasks.py tests/unit/test_artifacts.py tests/unit/test_agent_stream_artifacts.py tests/registry/test_core_cases.pyuv run basedpyright tracecat/artifacts/bindings.py tracecat/artifacts/resolution.py packages/tracecat-registry/tracecat_registry/core/ee/tasks.py tests/unit/test_artifacts.py tests/unit/test_agent_stream_artifacts.py tests/registry/test_core_cases.pySummary by cubic
Add missing case artifact triggers and resolve case identity from case, comment, and task references so streamed case artifacts hydrate title, severity, and status. Also return the parent case from
core.cases.delete_taskto keep artifact upserts working after task deletion.tracecat/artifacts/bindings.py.identity_refkindscomment_idandtask_id, resolving to the parent case and hydrating title/severity/status intracecat/artifacts/resolution.py.packages/tracecat-registry/tracecat_registry/core/ee/tasks.pysocore.cases.delete_taskreturns{"case_id": ...}for post-delete projections.Written for commit 2bb155e. Summary will update on new commits.