Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHAGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ Keep proposed and unmerged changes out of released sections.

## [Unreleased]

- Proposed [PR #1051](https://github.com/BitterSecurity/Decepticon/pull/1051):
structured tool errors — the ops tools report failures as
`{is_error, tool, cause, recovery}` envelopes built by
`decepticon.tools.errors.tool_error`, and the TUI routes tool results
that carry that envelope to the structured error display (cause and
recovery rendered separately) instead of dumping raw JSON. Removes an
internal design note accidentally committed at the repository root.
Source only, unreleased.
- Proposed [PR #1038](https://github.com/BitterSecurity/Decepticon/pull/1038):
TUI slash-command UX parity — completion, suggestions, /help grouping.
Path completions for `/file` keep the typed directory prefix, so
Expand Down
55 changes: 55 additions & 0 deletions clients/cli/src/components/EventItem.structured.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// @vitest-environment jsdom
import React from "react";
import { cleanup, render } from "@testing-library/react";
import { afterEach, expect, it, vi } from "vitest";
import { EventItem } from "./EventItem.js";
import type { AgentEvent } from "../types.js";

vi.mock("ink", async () => {
const React = await import("react");
return {
Box: ({ children }: { children: React.ReactNode }) => React.createElement("div", null, children),
Text: ({ children }: { children: React.ReactNode }) => React.createElement("span", null, children),
useInput: () => {},
};
});
vi.mock("../shell/ScreenContext.js", () => ({ useScreen: () => "transcript" }));
vi.mock("./BashResult.js", () => ({ BashResult: () => null }));
vi.mock("./TodoList.js", () => ({ TodoList: () => null, parseTodos: () => [] }));

afterEach(() => cleanup());

const structuredEnvelope = JSON.stringify({
is_error: true,
tool: "ops_start",
cause: "opscontrol_unreachable: missing socket",
recovery: "bring it up via `decepticon start`",
});

function toolResultEvent(content: string): AgentEvent {
return {
id: "e-1",
type: "tool_result",
content,
toolName: "ops_start",
toolArgs: { workload: "ad" },
status: "success",
timestamp: 0,
};
}

it("renders a structured tool error with cause and recovery", () => {
const view = render(<EventItem event={toolResultEvent(structuredEnvelope)} />);
const text = view.container.textContent ?? "";
expect(text).toContain("error>");
expect(text).toContain("[ops_start]");
expect(text).toContain("opscontrol_unreachable");
expect(text).toContain("Recovery:");
});

it("keeps plain tool results on the raw tool display", () => {
const view = render(<EventItem event={toolResultEvent('{"state":"running"}') as AgentEvent} />);
const text = view.container.textContent ?? "";
expect(text).not.toContain("Recovery:");
expect(text).toContain('{"state":"running"}');
});
6 changes: 6 additions & 0 deletions clients/cli/src/components/EventItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { BashResult } from "./BashResult.js";
import { UserMessage } from "./messages/UserMessage.js";
import { AIMessage } from "./messages/AIMessage.js";
import { ToolCallMessage } from "./messages/ToolCallMessage.js";
import { ErrorMessage, parseStructured } from "./messages/ErrorMessage.js";
import { DelegateMessage } from "./messages/DelegateMessage.js";
import { SystemMessage } from "./messages/SystemMessage.js";
import { QAQuestionMessage, QAAnswerMessage } from "./messages/QAMessage.js";
Expand Down Expand Up @@ -34,6 +35,11 @@ export const EventItem = React.memo(function EventItem({
);

case "tool_result":
// Tools signal failure with a structured error envelope (tools/errors.py);
// those render as cause + recovery instead of a raw tool result.
if (parseStructured(event.content)) {
return <ErrorMessage content={event.content} />;
}
return <ToolCallMessage event={event} />;

case "delegate":
Expand Down
40 changes: 40 additions & 0 deletions clients/cli/src/components/messages/ErrorMessage.render.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import React from "react";
import { describe, expect, it } from "vitest";
import { render } from "ink-testing-library";
import { ErrorMessage } from "./ErrorMessage.js";
import { ScreenProvider } from "../shell/ScreenContext.js";

describe("ErrorMessage — real Ink rendering", () => {
it("renders tool, cause, recovery, and dismiss hint", () => {
const content = JSON.stringify({
is_error: true,
tool: "Bash",
cause: "sandbox HTTP daemon not responding",
recovery: "Run decepticon doctor --deep to diagnose",
});
const rendered = render(
<ScreenProvider value="prompt">
<ErrorMessage content={content} onDismiss={() => undefined} />
</ScreenProvider>,
);
const frame = rendered.lastFrame() ?? "";
expect(frame).toContain("[Bash]");
expect(frame).toContain("sandbox HTTP daemon not responding");
expect(frame).toContain("Recovery: Run decepticon doctor --deep");
expect(frame).toContain("esc to dismiss");
rendered.unmount();
});

it("preserves plain-text error fallback", () => {
const rendered = render(
<ScreenProvider value="prompt">
<ErrorMessage content="Some plain error from the agent." />
</ScreenProvider>,
);
const frame = rendered.lastFrame() ?? "";
expect(frame).toContain("error>");
expect(frame).toContain("Some plain error from the agent.");
expect(frame).not.toContain("Recovery:");
rendered.unmount();
});
});
45 changes: 45 additions & 0 deletions clients/cli/src/components/messages/ErrorMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
*
* Claude Code pattern: MAX_API_ERROR_CHARS = 1000, head/tail split,
* red color coding, "(ctrl+o to expand)" hint on truncation.
* When the tool result is a structured error (from tools/errors.py),
* cause and recovery are displayed separately for readability.
Comment on lines +6 to +7

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Record this user-visible change in CHAGELOG.md

This commit changes operator-visible TUI error presentation, but CHAGELOG.md is absent from the changed files. Add a proposed, PR-linked entry under Unreleased before merge as required by the repository's versioned-change policy.

AGENTS.md reference: AGENTS.md:L3-L8

Useful? React with 👍 / 👎.

* Esc dismisses the banner when onDismiss is provided.
*/

Expand All @@ -13,6 +15,31 @@ import { useScreen } from "../shell/ScreenContext.js";

const MAX_ERROR_CHARS = 1000;

interface StructuredError {
is_error: true;
tool: string;
cause: string;
recovery?: string;
}

export function parseStructured(content: string): StructuredError | null {
try {
const parsed = JSON.parse(content);
if (
parsed &&
typeof parsed === "object" &&
parsed.is_error === true &&
typeof parsed.tool === "string" &&
typeof parsed.cause === "string"
) {
return parsed as StructuredError;
}
} catch {
// not JSON
}
return null;
}

interface Props {
content: string;
/** When provided, esc dismisses the banner. */
Expand All @@ -38,6 +65,24 @@ export const ErrorMessage = React.memo(function ErrorMessage({
{ isActive: onDismiss !== undefined },
);

const structured = parseStructured(display);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Route structured tool results through their renderer

When a tool returns the new tool_error() payload, useAgent.ts records it as a tool_result, and EventItem.tsx sends that event to ToolCallMessage; ErrorMessage is instantiated only for the separate top-level agent.error state. Consequently, this parser never receives the structured tool results created by this commit, so operators continue to see the raw payload rather than separated cause and recovery text. Parse these payloads in ToolCallMessage or route structured error events to this component.

Useful? React with 👍 / 👎.


if (structured) {
return (
<Box flexDirection="column" marginTop={1}>
<Text>
<Text color="red" bold>{"error> "}</Text>
<Text bold color="yellow">{`[${structured.tool}] `}</Text>
<Text color="red" wrap="wrap">{structured.cause}</Text>
</Text>
{structured.recovery && (
<Text dimColor wrap="wrap">{` Recovery: ${structured.recovery}`}</Text>
)}
{onDismiss && <Text dimColor>{" esc to dismiss"}</Text>}
</Box>
);
}

return (
<Box flexDirection="column" marginTop={1}>
<Text>
Expand Down
47 changes: 47 additions & 0 deletions packages/decepticon/decepticon/tools/errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
"""Structured error result for tools.

Claude Code pattern: errors carry the cause and a recovery path — not
just a message string. Tools return ``tool_error(...)`` on failure; the
TUI parses the envelope and displays cause and recovery separately.

Usage in a tool implementation:

from decepticon.tools.errors import tool_error

# On failure, return a structured error instead of raising:
return tool_error("Bash", cause="sandbox HTTP daemon not responding",
recovery="Run `decepticon doctor --deep` to diagnose")
"""

from __future__ import annotations

from typing import Any

from pydantic import BaseModel


class ToolErrorResult(BaseModel):
"""Structured error returned as a tool result.

The TUI parses this to display cause and recovery separately,
mirroring Claude Code's error display pattern.
"""

is_error: bool = True
tool: str
cause: str
recovery: str = ""


def tool_error(tool: str, *, cause: str, recovery: str = "") -> dict[str, Any]:
"""Build a structured error tool result.

Args:
tool: Tool name (e.g. "Bash", "WebFetch").
cause: What went wrong.
recovery: How the operator or agent should fix it.

Returns:
A dict that the TUI parses for cause/recovery display.
"""
return ToolErrorResult(tool=tool, cause=cause, recovery=recovery).model_dump()
44 changes: 24 additions & 20 deletions packages/decepticon/decepticon/tools/ops/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

from langchain_core.tools import tool

from decepticon.tools.errors import tool_error
from decepticon.tools.ops.client import (
OpsControlClient,
OpsControlError,
Expand All @@ -39,28 +40,31 @@ def _envelope(payload: dict[str, Any]) -> str:
return json.dumps(payload, sort_keys=True, default=str)


def _diagnose_unreachable(exc: OpsControlUnreachableError) -> str:
def _diagnose_unreachable(exc: OpsControlUnreachableError, tool_name: str) -> str:
return _envelope(
{
"error": "opscontrol_unreachable",
"message": str(exc),
"hint": (
tool_error(
tool_name,
cause=f"opscontrol_unreachable: {exc}",
recovery=(
"The opscontrol daemon owns the docker socket per ADR-0006. "
"If you reached this from `make dev` / `make smoke` the stack "
"is daemon-less by design — bring it up via `decepticon start` "
"to enable workload spawning."
),
}
)
)


def _diagnose_http(exc: OpsControlError) -> str:
def _diagnose_http(exc: OpsControlError, tool_name: str) -> str:
return _envelope(
{
"error": "opscontrol_http_error",
"status_code": exc.status_code,
"body": exc.body,
}
tool_error(
tool_name,
cause=f"opscontrol_http_error {exc.status_code}: {exc.body}",
recovery=(
"The request reached the opscontrol daemon but was rejected. "
"Check the daemon logs and the workload allowlist."
),
)
)


Expand Down Expand Up @@ -118,9 +122,9 @@ def ops_start(workload: str) -> str:
try:
return _envelope(OpsControlClient().start(workload, engagement_id))
except OpsControlUnreachableError as exc:
return _diagnose_unreachable(exc)
return _diagnose_unreachable(exc, "ops_start")
except OpsControlError as exc:
return _diagnose_http(exc)
return _diagnose_http(exc, "ops_start")


@tool
Expand All @@ -135,9 +139,9 @@ def ops_stop(workload: str) -> str:
try:
return _envelope(OpsControlClient().stop(workload))
except OpsControlUnreachableError as exc:
return _diagnose_unreachable(exc)
return _diagnose_unreachable(exc, "ops_stop")
except OpsControlError as exc:
return _diagnose_http(exc)
return _diagnose_http(exc, "ops_stop")


@tool
Expand Down Expand Up @@ -175,9 +179,9 @@ def ops_cleanup_engagement() -> str:
try:
return _envelope(OpsControlClient().cleanup_engagement(engagement_id))
except OpsControlUnreachableError as exc:
return _diagnose_unreachable(exc)
return _diagnose_unreachable(exc, "ops_cleanup_engagement")
except OpsControlError as exc:
return _diagnose_http(exc)
return _diagnose_http(exc, "ops_cleanup_engagement")


@tool
Expand All @@ -202,9 +206,9 @@ def ops_status() -> str:
}
)
except OpsControlUnreachableError as exc:
return _diagnose_unreachable(exc)
return _diagnose_unreachable(exc, "ops_status")
except OpsControlError as exc:
return _diagnose_http(exc)
return _diagnose_http(exc, "ops_status")


OPS_TOOLS = [ops_start, ops_stop, ops_cleanup_engagement, ops_status]
12 changes: 8 additions & 4 deletions packages/decepticon/tests/unit/ops/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,10 @@ def test_ops_start_unreachable_diagnostic(monkeypatch) -> None:
)
out = ops_start.invoke({"workload": "ad"})
data = json.loads(out)
assert data["error"] == "opscontrol_unreachable"
assert "ADR-0006" in data["hint"]
assert data["is_error"] is True
assert data["tool"] == "ops_start"
assert "opscontrol_unreachable" in data["cause"]
assert "ADR-0006" in data["recovery"]


def test_ops_start_http_error_propagates(monkeypatch) -> None:
Expand All @@ -142,8 +144,10 @@ def start(self, *_args, **_kw):
monkeypatch.setattr("decepticon.tools.ops.tools.OpsControlClient", factory)
out = ops_start.invoke({"workload": "bogus"})
data = json.loads(out)
assert data["error"] == "opscontrol_http_error"
assert data["status_code"] == 400
assert data["is_error"] is True
assert data["tool"] == "ops_start"
assert "opscontrol_http_error 400" in data["cause"]
assert "not in allowlist" in data["cause"]


def test_ops_stop_returns_envelope(patch_client) -> None:
Expand Down
Loading
Loading