From a1e10457ee588b266662a630d4e61e649cdf204f Mon Sep 17 00:00:00 2001 From: ericksondelacruz Date: Wed, 14 Oct 2026 14:49:13 +0800 Subject: [PATCH] feat(rules): add OBS-009..012 for agent-reliability-otel-labels label correctness OBS-006..008 prove the agent-reliability-otel-labels labels are written; OBS-009..012 prove they are right. Four repo-scope rules: - OBS-009 (medium, 0.6): a Run shared between runs (module scope, class body, Python default argument, or a literal run id). - OBS-010 (low, 0.55): tool spans labelled only on success (no mark_tool_span in finally / except / catch, an error callback, or a .finally / .catch handler). - OBS-011 (low, 0.7): every tool-span label passes a constant attempt. - OBS-012 (low, 0.55): no run-end label can record a failure. Discovery: OtelLabelsSignal gains the run_construct kind and per-call facts (ModuleScope, RunIDConst, FailureSafe, HookUnknown, ErrorPath, AttemptConst, SideEffect, ExitReason). A Python construct under `if __name__ == "__main__":` is a one-run script and a non-static TS/JS class field runs per instance, so neither is module scope. Labels inside an OpenAI Agents RunHooks / AgentHooks on_tool_end are left out as unknown. Test-file signals never count and one correct call credits the repo. Rules: four predicates (repo_otel_labels_run_shared, repo_otel_labels_tool_span_failure_unsafe, repo_otel_labels_attempt_constant, repo_otel_labels_run_end_success_only). Findings carry the offending call's file and line, so SARIF renders them as located results. The scan report gains otel_labels (not folded into ScanID). schema_version 22 -> 23 (main took 22 for LC-116). The fixture holds 338 rules and is in sync with agent-reliability-rules. Do not merge before agent-reliability-otel-labels 0.1.1 is published: the 0.1.0 README example trips OBS-009..012. --- ARCHITECTURE.md | 37 +- CLAUDE.md | 7 + COVERAGE.md | 2 +- README.md | 7 + internal/analysis/otel_labels.go | 9 +- internal/analysis/otel_labels_facts.go | 320 ++++++++++++++++ internal/analysis/otel_labels_test.go | 345 +++++++++++++++++- internal/models/models.go | 63 ++-- internal/rules/evaluator.go | 20 + internal/rules/otel_labels_evidence_test.go | 43 +++ internal/rules/policies_test.go | 81 ++++ internal/rules/predicates.go | 103 ++++++ internal/rules/predicates_test.go | 98 +++++ internal/rules/rule_detector.go | 38 ++ internal/rules/schema.go | 7 + internal/rules/schema.yaml | 22 ++ internal/rules/schema_version.go | 2 +- internal/sarif/render.go | 14 +- internal/scanner/scanner.go | 1 + internal/scanner/scanner_test.go | 251 +++++++++++++ testdata/rules-fixture/manifest.yaml | 2 +- .../rules-fixture/observability/quality.yaml | 144 ++++++++ 22 files changed, 1576 insertions(+), 40 deletions(-) create mode 100644 internal/analysis/otel_labels_facts.go create mode 100644 internal/rules/otel_labels_evidence_test.go diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 1011a4dc..ddcfc1fe 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -560,13 +560,26 @@ For each language recon cleared, do the AST work and produce a `RepoInventory`: `@trustabl/agent-reliability-otel-labels` is imported), `run_start` (`.mark_start(...)` / `.markStart(...)` — receiver required, since it is a method on `Run`), `tool_span` (`mark_tool_span` / `markToolSpan`) and `run_end` (`mark_run_end` - / `markRunEnd`). The call names are plausible names for a project's own + / `markRunEnd`). `run_construct` records a `Run()` / `new Run()` resolved through + the file's own import (never repo-wide: `Run` is too generic a name), with + `ModuleScope` (built once: no enclosing function, a Python class body, a + TypeScript/JavaScript `static` class field, or a Python default parameter; a + non-static TypeScript/JavaScript class field initializer runs per instance and + is exempt, as is a Python `__main__` guard, parenthesized or not) and `RunIDConst` + (a string-literal run id). The call names are plausible names for a project's own helpers, so they count only when the library is imported somewhere in the repo; each file's own imports are followed, so a renamed import - (`mark_tool_span as mts`, `markToolSpan as label`) still resolves. Every - signal carries `TestOrigin` when its file classifies as a test - (`pathclass.Classify`). That gate is repo-wide, not per-file: a `Run` is - routinely built in one module and started in another that imports nothing. + (`mark_tool_span as mts`, `markToolSpan as label`) still resolves. That gate + is repo-wide, not per-file: a `Run` is routinely built in one module and + started in another that imports nothing. Every signal carries `TestOrigin` + when its file classifies as a test (`pathclass.Classify`). + `tool_span` signals carry `FailureSafe` (inside `finally` / `except` / + `catch` before the enclosing function, in a named error callback, or in a + function passed to `.finally(` / `.catch(`), `HookUnknown` (an `on_tool_end` + method of a `RunHooks` / `AgentHooks` subclass, whose failure behaviour is + unverified), `AttemptConst` (an integer-literal `attempt`) and the literal + `SideEffect`. `run_end` signals carry `ErrorPath` (except / catch / error + callback / `.catch(`, never `finally`) and the literal `ExitReason`. `attr_literal` records a label key written to a span by hand (`set_attribute` / `setAttribute` first argument, or a key of the literal passed to `set_attributes` / `setAttributes`), collected whether or not the @@ -601,6 +614,20 @@ For each language recon cleared, do the AST work and produce a `RepoInventory`: do not count). The predicates prove the calls exist, not that labels land: with no current span each call is a silent no-op. + Four more repo-scope predicates back `OBS-009..012`, which check that the + labels written are right, not only present: `repo_otel_labels_run_shared` + (a `run_construct` with `ModuleScope` or `RunIDConst`), + `repo_otel_labels_tool_span_failure_unsafe` (tool-span labels exist, none is + `FailureSafe`; `HookUnknown` labels are left out), + `repo_otel_labels_attempt_constant` (every tool-span label has + `AttemptConst`) and `repo_otel_labels_run_end_success_only` (no run end has + `ErrorPath`, a literal `"error"`, or a variable reason). All four drop + test-file signals, and one correct call credits the repo. A repo finding + whose rule asserts one of them carries the offending call's file and line + (`otelLabelsEvidence` in `rule_detector.go`); every other repo finding keeps + an empty location. Hand-written `trustabl.*` keys credit the `OBS-006..008` + ladder but are not read by these four. + - **DiscoverSubagents** (`subagents.go`) — **hybrid** discovery in two passes. Pass 1 reads every `.claude/agents/*.md` component (matched at any path depth — monorepos that nest agent projects under `agent/.claude/agents/` or diff --git a/CLAUDE.md b/CLAUDE.md index 9a4b47fc..dc9c4388 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -380,6 +380,13 @@ When changing a rule (add / remove / edit severity, confidence, match, text): > OTel GenAI semconv is still Development with renamed keys), and **rank 4** (run-level correlation id — session / `group_id` — needs a > run-call kwarg predicate, tracked with the HTTP-entrypoint design ticket). See `COVERAGE.md`. > +> **agent-reliability-otel-labels correctness, Batch 1 (2026-10-08, `schema_version` 22 → 23):** the fixture now holds **338** rules +> (334 + 4; re-derive, do not trust this figure). Built in parallel with LC-116, which took 21 → 22 first, so this batch takes 23. **OBS-009** (shared `Run`) / **OBS-010** (tool span labelled only on success) / +> **OBS-011** (constant `attempt`) / **OBS-012** (run end only on success) read new facts on `OtelLabelsSignal`; test-file +> signals never count and findings point at the offending call. Hand-written `trustabl.*` keys now credit OBS-006..008. +> Spec: `.superpowers/specs/2026-10-04-otel-labels-correctness-rules-design.md`. Not built: the handoff rule (spec O7), +> the `side_effect` rules (Batch 2). OBS-009..012 must not reach production before library 0.1.1 is published (spec O1): the 0.1.0 README example trips all four. +> > **agent-reliability-otel-labels batch (2026-10-04, `schema_version` 20 → 21):** the fixture now holds **333** rules > (330 + 3; re-derive, do not trust this figure). Built in parallel with the HTTPS-only batch, which had already taken > 19 → 20, so this batch takes 21. **OBS-006** / **OBS-007** / **OBS-008** are a mutually exclusive diff --git a/COVERAGE.md b/COVERAGE.md index ed968153..18c08757 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -4,7 +4,7 @@ Coverage matrix for Trustabl's static analysis: which agent SDKs (and which languages) we currently scan, analyse, and detect against. This file is the at-a-glance reference; `ARCHITECTURE.md` has the implementation detail. -_Last reviewed: 2026-10-12 (branch `feat/langgraph-inmemory-checkpointer-server`, merged with `main` 44b2062) — **LC-116: in-memory LangGraph checkpointer run from a server entrypoint (`schema_version` 21 → 22; 1 rule, 2 predicates).** High / 0.6, agent scope, Python, `langchain_state_graph` + `langchain_agent`: `checkpointer=` resolves to `MemorySaver` / `InMemorySaver` (call, `MemorySaver(serde=...)`, aliased, or a same-file variable bound to one), a run call of that graph is reachable from a FastAPI / Flask / Celery / Dramatiq entrypoint, and the repo has no `langgraph.json`. New predicates `agent_checkpointer_in_memory`, `agent_server_reachable`; new in-memory fields `AgentDef.CheckpointerClass`, `AgentRunCallDef.AgentFilePath`. Run-call discovery now anchors LangGraph `invoke`/`ainvoke`/`stream`/`astream`/`astream_events`/`batch`/`abatch` on compiled-graph variables, the direct `builder.compile(...).invoke(...)` chain and prebuilt-agent variables only (never on an arbitrary `.invoke`), cross-file for module-level graphs. Test paths (`tests/`, `test_*.py`, `conftest.py`) never satisfy `agent_server_reachable`. **Not covered:** a helper of a helper; a graph built/compiled in a factory function; a saver chosen at runtime; `import graph; graph.app.invoke()`; Django/Starlette/RQ/serverless entrypoints; TypeScript. **Recall on real server repos is unmeasured:** the corpus has 40 run calls, 0 LangGraph, 0 server-reachable, and its full findings output (185) is identical before and after. Rule count **334** (333 + 1; re-derive). Prior review: 2026-10-11 (branch `feat/otel-labels-rules`, rebased onto 9fc2c39) — **OBS-006..008 credit fixes (no schema change, stays 21; no rule change).** Each file's own imports of agent-reliability-otel-labels are followed, so a renamed import (`mark_tool_span as mts`, `markToolSpan as label`) still counts. `trustabl.*` label keys written to a span by hand (`set_attribute` / `setAttribute` first argument, or a key of the literal passed to `set_attributes` / `setAttributes`) are recorded as `attr_literal` signals and credit the ladder rung by rung, so a repo that writes the labels without the library no longer draws OBS-006..008. Every signal carries `TestOrigin`; test-file signals still credit OBS-006..008. Rule counts re-derived: **333** fixture. Prior review 2026-10-07 (HEAD 8d0fb76 + uncommitted working tree, branch `feat/forge-runtime-tracing`) — **agent-reliability-otel-labels rules renumbered to `schema_version` 21.** The OBS-006..008 batch below was built in parallel with the HTTPS-only batch, and both took 19 → 20; after merging main this batch takes 20 → 21 (engine constant and both manifests). Rule counts re-derived: **333** fixture / **333** production (330 + OBS-006..008); `scripts/check-rules-sync.sh` passes. No coverage change. Prior review 2026-10-05 (HEAD d8a4f57 + uncommitted working tree, branch `feat/server-entrypoint-detection`) — **Server-entrypoint fact (Python; no schema change, stays 20; no rule, no predicate):** `RepoInventory.Entrypoints` (also `ScanResult.entrypoints`) records FastAPI / Flask route handlers (`http`) and Celery tasks / Dramatiq actors (`worker`), import-gated, skipping `examples/`, `demo/`, `docs/`, `samples/`, `cookbook/`, `tutorial(s)/`; `AgentRunCallDef.ServerReachable` is stamped in memory when a run call is in the entrypoint, a same-file helper, or an imported helper (one hop each; relative imports resolved within the scan set). Nothing consumes it yet; ranks 4 and 6 build on it in follow-up PRs. Deferred: Django, Starlette `Route()`, RQ, Temporal, serverless handlers, `add_api_route` / `add_url_rule`, class-based views, TypeScript. Corpus baseline: 2 entrypoints (Flask), 40 run calls, 0 stamped — all run calls are standalone scripts. Prior review 2026-10-05 (HEAD 05c44c9 + uncommitted working tree, branch `fix/obs-global-instrumentor-credit`) — **Observability false-positive fix: process-wide instrumentor calls are credited (no schema change; `schema_version` stays 20, no new rules, no fixture change).** PYD-107's fix text recommends `logfire.instrument_pydantic_ai()`, but its match (`repo_has_observability` + `agent_kwarg_missing: [instrument]`) never read that call, so following the fix still produced one finding per agent; LC-112's "configure a global handler" advice had the same gap. Crediting the call as repo-level init evidence alone cannot fix this (the kwarg check does not read init evidence) and would make it worse for `Agent.instrument_all()`, so the credit is agent-scoped: new `AgentDef.InstrumentedBy` (`instrumented_by` in the JSON report, omitted when unset), stamped by `analysis.ApplyGlobalInstrumentation` from the init signals, and `PredAgentKwargMissing` treats the exact kwarg it names as set. Covered: `logfire.instrument_pydantic_ai` and `Agent.instrument_all()` (→ `instrument`, Pydantic AI, Python), `LangChainInstrumentor().instrument()` (→ `callbacks`, LangChain, Python). Also recognized as init evidence but crediting no agent: `logfire.instrument_openai_agents`, `logfire.instrument_mcp`, any other `Instrumentor(...).instrument(...)`. `.instrument_all()` counts only in a file importing `pydantic_ai` or `logfire`; the instrumentor shape only in a file importing `openinference`, `opentelemetry` or `traceloop` (the two-step `i = XInstrumentor(); i.instrument()` is not matched). Behavior changes beyond PYD-107/LC-112: a repo whose only signal is one of these calls now counts as observed (the repo absence rules go quiet, OBS-001 stays silent). VAI-101 is unchanged because the Vercel AI SDK has no process-wide telemetry switch — `experimental_telemetry` is enabled per call. **Opik** added as a vendor (Python `opik` import, `opik.configure` / `OpikTracer` / `track_openai`; TS `opik` import, `new Opik()` / `OpikExporter` / `OpikCallbackHandler`; recon dep needle `opik`). Known risk: a repo that uses Opik only through the `@track` decorator plus env configuration imports `opik` but makes no init call, so it reads as imported-never-initialized and OBS-001 fires; Langfuse `@observe` already behaves the same way, and decorator detection (`@observe`, `@track`, `@traceable`) is deliberately deferred until a rule consumes it. **Observability backlog items investigated and deliberately not shipped.** **Tool result not recorded on a span (P0 rank 3): no rule.** Frameworks span every tool call once tracing is enabled (OpenAI Agents by default, Google ADK, LangChain `BaseTool`, Pydantic AI `instrument`, Vercel `experimental_telemetry`, CrewAI instrumentors), and enablement is often env- or CLI-only (LangSmith, Claude Code, ADK), so a repo-level "tracing present" gate cannot say whether a given tool is spanned. Unlike the HTTPS-only rules it is not same-body evidence, and a rule would fire on correctly traced tools. **Required span attributes (P0 rank 2): no rule.** In every audited SDK the attributes are written by the SDK or by the OTel / OpenInference instrumentor, so a well-instrumented repo contains no attribute literals and a static rule cannot tell "the instrumentor sets them" from "nobody sets them". The OTel GenAI semantic conventions are still Development stability and have renamed keys (`gen_ai.usage.prompt_tokens` → `gen_ai.usage.input_tokens`, `gen_ai.system` → `gen_ai.provider.name`). The backlog's `thread.id` is the OS-thread attribute, not a conversation id; GenAI's conversation attribute is `gen_ai.conversation.id`, but the registry page now marks it deprecated / moved to the GenAI conventions repo — verify the current name before citing it. The session / `group_id` correlation idea (OpenAI `Runner.run` without `RunConfig(group_id=)`, Langfuse / LangChain without `session_id`) belongs to rank 4 (run-level correlation id), which needs a run-call kwarg predicate and is tracked with the HTTP-entrypoint design ticket. Prior review 2026-10-04 (HEAD 4f8f6bd + uncommitted working tree, branch `feat/forge-runtime-tracing`) — **agent-reliability-otel-labels process-label rules (`schema_version` 20 → 21; recorded as 19 → 20 before main's HTTPS-only batch was merged).** New inventory pass `analysis.DiscoverOtelLabels` emits typed `OtelLabelsSignal` records (Kind `import` / `run_start` / `tool_span` / `run_end`) for Python and TypeScript/JavaScript; the Go binding is not inspected. Recon gains `RepoProfile.OtelLabelsDeclaredIn` (`agent-reliability-otel-labels` / `agent_reliability_otel_labels` in the Python root manifests, `@trustabl/agent-reliability-otel-labels` in `package.json`). Both are kept out of `ObservabilitySignals` / `ObsDeps` because the library never creates a span and must not satisfy its own prerequisite. Three new repo-scope predicates (`repo_otel_labels_installed`, `repo_otel_labels_initialized`, `repo_otel_labels_implemented`) back three new rules in `observability/quality.yaml`, all gated on `repo_observability_initialized: true` and mutually exclusive: **OBS-006** (instrumented, library neither declared nor imported; low), **OBS-007** (installed, no `mark_start`; medium), **OBS-008** (run started, no `mark_run_end`, or tools exist and no `mark_tool_span`; medium). They check that the calls exist, not that labels land or are correct: a missing current span makes every call a silent no-op, and a shared `Run`, a constant `attempt`, labelling outside `finally`, a `side_effect` that contradicts the tool body, and a missing `mark_handoff` are deferred. Rule counts at the time: 319 fixture / 319 production, before main's 14 HTTPS-only rules were merged (333 after). Prior review 2026-10-04 (HEAD 5ba476d, branch `feat/forge-runtime-tracing`) — **Recon now detects Google ADK for Go (`go.mod`: `google.golang.org/adk` → recon-only dep name `google-adk-go`, not mapped to an SDK for `scan`; used by `trustabl forge` to select the Go runtime-tracing recipe).** Prior review 2026-10-02 (HEAD 2acaf88 + uncommitted working tree, branch `feat/p0-gap-10-fetch-allowlist`) — **P0 rank #10, fetch tool HTTPS-only (`schema_version` 19 → 20): 14 new rules, one per SSRF rule.** The host allow-list half of the gap already shipped as the allow-list credit on the 14 SSRF rules; what was missing was the scheme. New tool-scope predicate **`has_unpinned_scheme_url_call`** (a recognized HTTP call whose URL is dynamic and does not begin with a literal `https://`; Python via `analysis.URLArgSchemePinned`, TS via the new `url_scheme_unpinned` handler fact next to `dynamic_url`; f-string / template / `+` / `%` / `.format` forms with a leading `https://` are pinned). New rules, medium / 0.6: **CSDK-024/025, OAI-032/033, MCP-031/032, ADK-022/023, LC-026/LC-027, CREW-015, PYD-015, AG2-022, VAI-021** — each fires only when its SSRF sibling has been silenced by an allow-list and no scheme check (`.scheme ==`, `.protocol ===`, `startswith("https://")`) is present, so SSRF and HTTPS never fire on the same tool. **PYD-103** fix text now notes `WebFetchTool(allowed_domains=)` is Anthropic-only and not a substitute for egress controls. Not built: LangGraph nodes (state is not a model-chosen argument, no SSRF base rule), `raw_llm_sdk` (repo scope), a Pydantic `WebFetchTool` `allowed_domains` rule (provider-dependent), scheme validation in another function/module, fully literal `http://` URLs (not dynamic). Rule count **330** (316 + 14; re-derive). Prior review: 2026-10-01 (HEAD 1d24b98 + uncommitted working tree, branch `feat/p0-partial-gaps`) — **P0 partial-gaps batch (no new schema field; `schema_version` stays 19): wall-clock timeout extended to Google ADK and AutoGen; cost cap and general tool-output trust investigated and deliberately not shipped.** New rules **ADK-113** and **AG2-021** (run call with no wall-clock timeout) ride on the existing `agent_run_call_wall_clock_timeout_missing` predicate, widened to ADK and AutoGen: `DiscoverAgentRunCalls` now also discovers ADK `Runner.run` / `run_async` (runner variable resolved to its agent through a same-file `Runner(agent=)` / `InMemoryRunner(agent)` pre-pass; `run_live` and `app=`-built runners are not run calls here) and AutoGen `initiate_chat` / `a_initiate_chat` (receiver **and** recipient) / `run` / `a_run` / `run_stream`; the predicate also credits ADK `abort_signal` and AutoGen `cancellation_token` (non-None). **VAI-020** now also requires `timeout` to be absent (the Vercel AI SDK's native `timeout` option, number or `{ totalMs, stepMs, ... }`, was a false positive); Vercel needed no new TS discovery. **Per-run token/cost cap (P0 rank 8): no further rule.** Checked against upstream: OpenAI Agents (`Runner.run*`/`RunConfig`), LangChain/LangGraph (`max_iterations`, `ModelCallLimitMiddleware` counts calls not tokens), Google ADK (`RunConfig.max_llm_calls` counts calls) and Vercel AI (`maxOutputTokens` is per step) have no run-level token/cost parameter; CrewAI's `Agent.max_tokens` is declared but never read by the agent or executor (usage reaches only `TokenCalcHandler`, which tracks and does not enforce), so an absence rule would point at a no-op. AutoGen v0.4's `TokenUsageTermination` is a real primitive but is a composable termination object on team constructors, which are not discovered — not built. **Untrusted tool output (P0 rank 11): no general rule.** Framework loops return tool output through the structured tool-result channel (no user-code concatenation site), user-code `f"...{result}"` needs inter-procedural taint to call `result` a tool result, and a "delimiter present" check would reward cosmetic tags; RAW-001/002 stay the only statically defensible slice. Rule count **316** (314 + 2; re-derive). Prior review: **P0 remaining-gaps, engine batch (`schema_version` 18 → 19): six rules that needed new predicates or discovery.** **LC-114** (raw `StateGraph` compiled with no `checkpointer`; new `agent_kwargs_observed` + dual-scope `repo_langgraph_platform_config_present`, so it stays silent on unlinked/chained/opaque/subgraph compiles and on repos with a `langgraph.json`), **LC-115** (a function registered via `add_node` that shells out / executes code / writes / fetches a dynamic URL with no `interrupt(` — new `langgraph_node` ToolKind via `DiscoverLangGraphNodes`; same-file undecorated top-level functions only, confidence 0.45), **OAI-120** / **PYD-108** (run call with no wall-clock timeout — new `AgentRunCallDef.WallClockTimeoutWrapped` from a structural ancestor walk of `asyncio.wait_for` / `asyncio.timeout` / `anyio.move_on_after` / `anyio.fail_after`, new predicate `agent_run_call_wall_clock_timeout_missing`), and **RAW-001** / **RAW-002** (new `raw_llm_sdk` category, loaded unconditionally: the bare `anthropic` / `openai` client in a manual tool loop that builds a dynamic system prompt/message from tool-call-derived values — a deliberately narrow same-function heuristic via `DetectRawLLMToolOutputInSystem`). Rule count **314** (308 + 6; re-derive). Prior review 2026-09-30 (branch `feat/p0-remaining-gaps-batch1`, merged with Batch 2 on `main`) — **P0 remaining-gaps batch, rules-layer only (no discovery, predicate or `schema_version` change; stays 18).** New rules **CSDK-114** (TS `query()` main agent with inline `options` and no `options.maxBudgetUsd`), **CREW-111** (`Agent` with no `max_execution_time`, exempting `config=`), **LC-113** (Python `AgentExecutor` with no `max_execution_time`) and **VAI-020** (`generateText`/`streamText` with a `stopWhen`/`maxSteps` loop and no `abortSignal`; `ToolLoopAgent` excluded). The 14 SSRF rules (CSDK-009/013, OAI-018/024, LC-005/013, CREW-005, PYD-005, ADK-012/016, MCP-008/013, AG2-011, VAI-003) now **credit a host allow-list** in the tool body (`not: has_body_text`) instead of firing on any non-literal URL; IDs/severity/confidence unchanged. Deferred because they need new predicates or discovery, which this batch was barred from adding: OAI/PYD whole-run wall-clock deadline (needs a run-call deadline-wrapper fact), Python `ClaudeAgentOptions(max_budget_usd)`, LangGraph no-checkpointer and mutating-node-without-HITL (need a Platform/`langgraph.json` exemption, opaque-`compile()` handling and an `interrupt()` fact), LangChain.js `maxExecutionTime` (option not verified to exist), and untrusted-tool-output concat (no raw-API-loop discovery). Rule count **308** (302 + Batch 2's 6); re-derive with `grep -rhoE '^\s*-\s*id:\s*\S+' testdata/rules-fixture/*/*.yaml | wc -l`. Prior review 2026-09-29 (branch `feat/p0-side-effect-bounds-batch2`, off `main` e5cef47) — Batch 2 of the side-effect-bounds rule: **CREW-014** (CrewAI), **ADK-020** (Python) / **ADK-021** (TypeScript), **AG2-020** (AutoGen / AG2), **VAI-019** (Vercel AI SDK), and **CSDK-023** (Claude Agent SDK TypeScript) — same detection as Batch 1, adapted to each SDK's tool shape (one new `side_effect_bounds.yaml` per SDK; severity high, confidence 0.5). Approval gates differ per SDK and are honest about it: Vercel keys on `needsApproval` (as VAI-018); ADK Python on `require_confirmation` or a `request_confirmation(` call; ADK TypeScript has **no** declarative confirmation option on `FunctionTool`, so ADK-021 treats a `requestConfirmation(` call in the handler as the gate; CrewAI, AutoGen and Claude TS have no tool-level gate (`Task(human_input=True)`, `human_input_mode`, and `canUseTool`/`permissionMode` live on the task/agent/`query()` options, which a tool-scope rule cannot see), so those rules can fire on a tool gated elsewhere. One small discovery change: `DiscoverADKTools` now copies `FunctionTool(...)` keyword arguments into `ToolDef.Config` (previously dropped), which is what lets ADK-020 see `require_confirmation`. Known discovery limits carried into the rules: Vercel tools are matched only through a `const x = tool(...)` binding (inline `tools: { x: tool(...) }` has no name); Claude TS reads parameter names only from an inline object-literal schema; CrewAI `BaseTool` subclasses, autogen_core `FunctionTool`, and ADK bare functions in `tools=[...]` are not discovered. No schema/predicate/`schema_version` change. Rule count **304** (298 + 6; re-derive rather than trust this figure). Prior review 2026-09-29 (pending merge commit; `origin/main` merged into `feat/p0-side-effect-bounds-batch1`) — Batch 1 of the +_Last reviewed: 2026-10-14 (branch `feat/otel-labels-correctness`, rebased onto `main` 32d845e; first reviewed 2026-10-08) — **agent-reliability-otel-labels correctness, Batch 1 (`schema_version` 22 → 23; main took 22 for LC-116).** `OtelLabelsSignal` gains the `run_construct` kind and per-call facts (`ModuleScope`, `RunIDConst`, `FailureSafe`, `HookUnknown`, `ErrorPath`, `AttemptConst`, `SideEffect`, `ExitReason`). Four repo-scope predicates back **OBS-009** (shared `Run`, medium), **OBS-010** (tool span labelled only on success, low), **OBS-011** (constant `attempt`, low) and **OBS-012** (run end only on success, low); test-file signals never count, one correct call credits the repo, and findings carry the offending call's file and line. The scan report gains `otel_labels`. Not built: the handoff rule (deferred, no recipe), the `side_effect` rules (Batch 2), the Go binding. Rule counts re-derived: **338** fixture / **338** production (334 + 4); `scripts/check-rules-sync.sh` passes. Prior review: 2026-10-12 (branch `feat/langgraph-inmemory-checkpointer-server`, merged with `main` 44b2062) — **LC-116: in-memory LangGraph checkpointer run from a server entrypoint (`schema_version` 21 → 22; 1 rule, 2 predicates).** High / 0.6, agent scope, Python, `langchain_state_graph` + `langchain_agent`: `checkpointer=` resolves to `MemorySaver` / `InMemorySaver` (call, `MemorySaver(serde=...)`, aliased, or a same-file variable bound to one), a run call of that graph is reachable from a FastAPI / Flask / Celery / Dramatiq entrypoint, and the repo has no `langgraph.json`. New predicates `agent_checkpointer_in_memory`, `agent_server_reachable`; new in-memory fields `AgentDef.CheckpointerClass`, `AgentRunCallDef.AgentFilePath`. Run-call discovery now anchors LangGraph `invoke`/`ainvoke`/`stream`/`astream`/`astream_events`/`batch`/`abatch` on compiled-graph variables, the direct `builder.compile(...).invoke(...)` chain and prebuilt-agent variables only (never on an arbitrary `.invoke`), cross-file for module-level graphs. Test paths (`tests/`, `test_*.py`, `conftest.py`) never satisfy `agent_server_reachable`. **Not covered:** a helper of a helper; a graph built/compiled in a factory function; a saver chosen at runtime; `import graph; graph.app.invoke()`; Django/Starlette/RQ/serverless entrypoints; TypeScript. **Recall on real server repos is unmeasured:** the corpus has 40 run calls, 0 LangGraph, 0 server-reachable, and its full findings output (185) is identical before and after. Rule count **334** (333 + 1; re-derive). Prior review: 2026-10-11 (branch `feat/otel-labels-rules`, rebased onto 9fc2c39) — **OBS-006..008 credit fixes (no schema change, stays 21; no rule change).** Each file's own imports of agent-reliability-otel-labels are followed, so a renamed import (`mark_tool_span as mts`, `markToolSpan as label`) still counts. `trustabl.*` label keys written to a span by hand (`set_attribute` / `setAttribute` first argument, or a key of the literal passed to `set_attributes` / `setAttributes`) are recorded as `attr_literal` signals and credit the ladder rung by rung, so a repo that writes the labels without the library no longer draws OBS-006..008. Every signal carries `TestOrigin`; test-file signals still credit OBS-006..008. Rule counts re-derived: **333** fixture. Prior review 2026-10-07 (HEAD 8d0fb76 + uncommitted working tree, branch `feat/forge-runtime-tracing`) — **agent-reliability-otel-labels rules renumbered to `schema_version` 21.** The OBS-006..008 batch below was built in parallel with the HTTPS-only batch, and both took 19 → 20; after merging main this batch takes 20 → 21 (engine constant and both manifests). Rule counts re-derived: **333** fixture / **333** production (330 + OBS-006..008); `scripts/check-rules-sync.sh` passes. No coverage change. Prior review 2026-10-05 (HEAD d8a4f57 + uncommitted working tree, branch `feat/server-entrypoint-detection`) — **Server-entrypoint fact (Python; no schema change, stays 20; no rule, no predicate):** `RepoInventory.Entrypoints` (also `ScanResult.entrypoints`) records FastAPI / Flask route handlers (`http`) and Celery tasks / Dramatiq actors (`worker`), import-gated, skipping `examples/`, `demo/`, `docs/`, `samples/`, `cookbook/`, `tutorial(s)/`; `AgentRunCallDef.ServerReachable` is stamped in memory when a run call is in the entrypoint, a same-file helper, or an imported helper (one hop each; relative imports resolved within the scan set). Nothing consumes it yet; ranks 4 and 6 build on it in follow-up PRs. Deferred: Django, Starlette `Route()`, RQ, Temporal, serverless handlers, `add_api_route` / `add_url_rule`, class-based views, TypeScript. Corpus baseline: 2 entrypoints (Flask), 40 run calls, 0 stamped — all run calls are standalone scripts. Prior review 2026-10-05 (HEAD 05c44c9 + uncommitted working tree, branch `fix/obs-global-instrumentor-credit`) — **Observability false-positive fix: process-wide instrumentor calls are credited (no schema change; `schema_version` stays 20, no new rules, no fixture change).** PYD-107's fix text recommends `logfire.instrument_pydantic_ai()`, but its match (`repo_has_observability` + `agent_kwarg_missing: [instrument]`) never read that call, so following the fix still produced one finding per agent; LC-112's "configure a global handler" advice had the same gap. Crediting the call as repo-level init evidence alone cannot fix this (the kwarg check does not read init evidence) and would make it worse for `Agent.instrument_all()`, so the credit is agent-scoped: new `AgentDef.InstrumentedBy` (`instrumented_by` in the JSON report, omitted when unset), stamped by `analysis.ApplyGlobalInstrumentation` from the init signals, and `PredAgentKwargMissing` treats the exact kwarg it names as set. Covered: `logfire.instrument_pydantic_ai` and `Agent.instrument_all()` (→ `instrument`, Pydantic AI, Python), `LangChainInstrumentor().instrument()` (→ `callbacks`, LangChain, Python). Also recognized as init evidence but crediting no agent: `logfire.instrument_openai_agents`, `logfire.instrument_mcp`, any other `Instrumentor(...).instrument(...)`. `.instrument_all()` counts only in a file importing `pydantic_ai` or `logfire`; the instrumentor shape only in a file importing `openinference`, `opentelemetry` or `traceloop` (the two-step `i = XInstrumentor(); i.instrument()` is not matched). Behavior changes beyond PYD-107/LC-112: a repo whose only signal is one of these calls now counts as observed (the repo absence rules go quiet, OBS-001 stays silent). VAI-101 is unchanged because the Vercel AI SDK has no process-wide telemetry switch — `experimental_telemetry` is enabled per call. **Opik** added as a vendor (Python `opik` import, `opik.configure` / `OpikTracer` / `track_openai`; TS `opik` import, `new Opik()` / `OpikExporter` / `OpikCallbackHandler`; recon dep needle `opik`). Known risk: a repo that uses Opik only through the `@track` decorator plus env configuration imports `opik` but makes no init call, so it reads as imported-never-initialized and OBS-001 fires; Langfuse `@observe` already behaves the same way, and decorator detection (`@observe`, `@track`, `@traceable`) is deliberately deferred until a rule consumes it. **Observability backlog items investigated and deliberately not shipped.** **Tool result not recorded on a span (P0 rank 3): no rule.** Frameworks span every tool call once tracing is enabled (OpenAI Agents by default, Google ADK, LangChain `BaseTool`, Pydantic AI `instrument`, Vercel `experimental_telemetry`, CrewAI instrumentors), and enablement is often env- or CLI-only (LangSmith, Claude Code, ADK), so a repo-level "tracing present" gate cannot say whether a given tool is spanned. Unlike the HTTPS-only rules it is not same-body evidence, and a rule would fire on correctly traced tools. **Required span attributes (P0 rank 2): no rule.** In every audited SDK the attributes are written by the SDK or by the OTel / OpenInference instrumentor, so a well-instrumented repo contains no attribute literals and a static rule cannot tell "the instrumentor sets them" from "nobody sets them". The OTel GenAI semantic conventions are still Development stability and have renamed keys (`gen_ai.usage.prompt_tokens` → `gen_ai.usage.input_tokens`, `gen_ai.system` → `gen_ai.provider.name`). The backlog's `thread.id` is the OS-thread attribute, not a conversation id; GenAI's conversation attribute is `gen_ai.conversation.id`, but the registry page now marks it deprecated / moved to the GenAI conventions repo — verify the current name before citing it. The session / `group_id` correlation idea (OpenAI `Runner.run` without `RunConfig(group_id=)`, Langfuse / LangChain without `session_id`) belongs to rank 4 (run-level correlation id), which needs a run-call kwarg predicate and is tracked with the HTTP-entrypoint design ticket. Prior review 2026-10-04 (HEAD 4f8f6bd + uncommitted working tree, branch `feat/forge-runtime-tracing`) — **agent-reliability-otel-labels process-label rules (`schema_version` 20 → 21; recorded as 19 → 20 before main's HTTPS-only batch was merged).** New inventory pass `analysis.DiscoverOtelLabels` emits typed `OtelLabelsSignal` records (Kind `import` / `run_start` / `tool_span` / `run_end`) for Python and TypeScript/JavaScript; the Go binding is not inspected. Recon gains `RepoProfile.OtelLabelsDeclaredIn` (`agent-reliability-otel-labels` / `agent_reliability_otel_labels` in the Python root manifests, `@trustabl/agent-reliability-otel-labels` in `package.json`). Both are kept out of `ObservabilitySignals` / `ObsDeps` because the library never creates a span and must not satisfy its own prerequisite. Three new repo-scope predicates (`repo_otel_labels_installed`, `repo_otel_labels_initialized`, `repo_otel_labels_implemented`) back three new rules in `observability/quality.yaml`, all gated on `repo_observability_initialized: true` and mutually exclusive: **OBS-006** (instrumented, library neither declared nor imported; low), **OBS-007** (installed, no `mark_start`; medium), **OBS-008** (run started, no `mark_run_end`, or tools exist and no `mark_tool_span`; medium). They check that the calls exist, not that labels land or are correct: a missing current span makes every call a silent no-op, and a shared `Run`, a constant `attempt`, labelling outside `finally`, a `side_effect` that contradicts the tool body, and a missing `mark_handoff` are deferred. Rule counts at the time: 319 fixture / 319 production, before main's 14 HTTPS-only rules were merged (333 after). Prior review 2026-10-04 (HEAD 5ba476d, branch `feat/forge-runtime-tracing`) — **Recon now detects Google ADK for Go (`go.mod`: `google.golang.org/adk` → recon-only dep name `google-adk-go`, not mapped to an SDK for `scan`; used by `trustabl forge` to select the Go runtime-tracing recipe).** Prior review 2026-10-02 (HEAD 2acaf88 + uncommitted working tree, branch `feat/p0-gap-10-fetch-allowlist`) — **P0 rank #10, fetch tool HTTPS-only (`schema_version` 19 → 20): 14 new rules, one per SSRF rule.** The host allow-list half of the gap already shipped as the allow-list credit on the 14 SSRF rules; what was missing was the scheme. New tool-scope predicate **`has_unpinned_scheme_url_call`** (a recognized HTTP call whose URL is dynamic and does not begin with a literal `https://`; Python via `analysis.URLArgSchemePinned`, TS via the new `url_scheme_unpinned` handler fact next to `dynamic_url`; f-string / template / `+` / `%` / `.format` forms with a leading `https://` are pinned). New rules, medium / 0.6: **CSDK-024/025, OAI-032/033, MCP-031/032, ADK-022/023, LC-026/LC-027, CREW-015, PYD-015, AG2-022, VAI-021** — each fires only when its SSRF sibling has been silenced by an allow-list and no scheme check (`.scheme ==`, `.protocol ===`, `startswith("https://")`) is present, so SSRF and HTTPS never fire on the same tool. **PYD-103** fix text now notes `WebFetchTool(allowed_domains=)` is Anthropic-only and not a substitute for egress controls. Not built: LangGraph nodes (state is not a model-chosen argument, no SSRF base rule), `raw_llm_sdk` (repo scope), a Pydantic `WebFetchTool` `allowed_domains` rule (provider-dependent), scheme validation in another function/module, fully literal `http://` URLs (not dynamic). Rule count **330** (316 + 14; re-derive). Prior review: 2026-10-01 (HEAD 1d24b98 + uncommitted working tree, branch `feat/p0-partial-gaps`) — **P0 partial-gaps batch (no new schema field; `schema_version` stays 19): wall-clock timeout extended to Google ADK and AutoGen; cost cap and general tool-output trust investigated and deliberately not shipped.** New rules **ADK-113** and **AG2-021** (run call with no wall-clock timeout) ride on the existing `agent_run_call_wall_clock_timeout_missing` predicate, widened to ADK and AutoGen: `DiscoverAgentRunCalls` now also discovers ADK `Runner.run` / `run_async` (runner variable resolved to its agent through a same-file `Runner(agent=)` / `InMemoryRunner(agent)` pre-pass; `run_live` and `app=`-built runners are not run calls here) and AutoGen `initiate_chat` / `a_initiate_chat` (receiver **and** recipient) / `run` / `a_run` / `run_stream`; the predicate also credits ADK `abort_signal` and AutoGen `cancellation_token` (non-None). **VAI-020** now also requires `timeout` to be absent (the Vercel AI SDK's native `timeout` option, number or `{ totalMs, stepMs, ... }`, was a false positive); Vercel needed no new TS discovery. **Per-run token/cost cap (P0 rank 8): no further rule.** Checked against upstream: OpenAI Agents (`Runner.run*`/`RunConfig`), LangChain/LangGraph (`max_iterations`, `ModelCallLimitMiddleware` counts calls not tokens), Google ADK (`RunConfig.max_llm_calls` counts calls) and Vercel AI (`maxOutputTokens` is per step) have no run-level token/cost parameter; CrewAI's `Agent.max_tokens` is declared but never read by the agent or executor (usage reaches only `TokenCalcHandler`, which tracks and does not enforce), so an absence rule would point at a no-op. AutoGen v0.4's `TokenUsageTermination` is a real primitive but is a composable termination object on team constructors, which are not discovered — not built. **Untrusted tool output (P0 rank 11): no general rule.** Framework loops return tool output through the structured tool-result channel (no user-code concatenation site), user-code `f"...{result}"` needs inter-procedural taint to call `result` a tool result, and a "delimiter present" check would reward cosmetic tags; RAW-001/002 stay the only statically defensible slice. Rule count **316** (314 + 2; re-derive). Prior review: **P0 remaining-gaps, engine batch (`schema_version` 18 → 19): six rules that needed new predicates or discovery.** **LC-114** (raw `StateGraph` compiled with no `checkpointer`; new `agent_kwargs_observed` + dual-scope `repo_langgraph_platform_config_present`, so it stays silent on unlinked/chained/opaque/subgraph compiles and on repos with a `langgraph.json`), **LC-115** (a function registered via `add_node` that shells out / executes code / writes / fetches a dynamic URL with no `interrupt(` — new `langgraph_node` ToolKind via `DiscoverLangGraphNodes`; same-file undecorated top-level functions only, confidence 0.45), **OAI-120** / **PYD-108** (run call with no wall-clock timeout — new `AgentRunCallDef.WallClockTimeoutWrapped` from a structural ancestor walk of `asyncio.wait_for` / `asyncio.timeout` / `anyio.move_on_after` / `anyio.fail_after`, new predicate `agent_run_call_wall_clock_timeout_missing`), and **RAW-001** / **RAW-002** (new `raw_llm_sdk` category, loaded unconditionally: the bare `anthropic` / `openai` client in a manual tool loop that builds a dynamic system prompt/message from tool-call-derived values — a deliberately narrow same-function heuristic via `DetectRawLLMToolOutputInSystem`). Rule count **314** (308 + 6; re-derive). Prior review 2026-09-30 (branch `feat/p0-remaining-gaps-batch1`, merged with Batch 2 on `main`) — **P0 remaining-gaps batch, rules-layer only (no discovery, predicate or `schema_version` change; stays 18).** New rules **CSDK-114** (TS `query()` main agent with inline `options` and no `options.maxBudgetUsd`), **CREW-111** (`Agent` with no `max_execution_time`, exempting `config=`), **LC-113** (Python `AgentExecutor` with no `max_execution_time`) and **VAI-020** (`generateText`/`streamText` with a `stopWhen`/`maxSteps` loop and no `abortSignal`; `ToolLoopAgent` excluded). The 14 SSRF rules (CSDK-009/013, OAI-018/024, LC-005/013, CREW-005, PYD-005, ADK-012/016, MCP-008/013, AG2-011, VAI-003) now **credit a host allow-list** in the tool body (`not: has_body_text`) instead of firing on any non-literal URL; IDs/severity/confidence unchanged. Deferred because they need new predicates or discovery, which this batch was barred from adding: OAI/PYD whole-run wall-clock deadline (needs a run-call deadline-wrapper fact), Python `ClaudeAgentOptions(max_budget_usd)`, LangGraph no-checkpointer and mutating-node-without-HITL (need a Platform/`langgraph.json` exemption, opaque-`compile()` handling and an `interrupt()` fact), LangChain.js `maxExecutionTime` (option not verified to exist), and untrusted-tool-output concat (no raw-API-loop discovery). Rule count **308** (302 + Batch 2's 6); re-derive with `grep -rhoE '^\s*-\s*id:\s*\S+' testdata/rules-fixture/*/*.yaml | wc -l`. Prior review 2026-09-29 (branch `feat/p0-side-effect-bounds-batch2`, off `main` e5cef47) — Batch 2 of the side-effect-bounds rule: **CREW-014** (CrewAI), **ADK-020** (Python) / **ADK-021** (TypeScript), **AG2-020** (AutoGen / AG2), **VAI-019** (Vercel AI SDK), and **CSDK-023** (Claude Agent SDK TypeScript) — same detection as Batch 1, adapted to each SDK's tool shape (one new `side_effect_bounds.yaml` per SDK; severity high, confidence 0.5). Approval gates differ per SDK and are honest about it: Vercel keys on `needsApproval` (as VAI-018); ADK Python on `require_confirmation` or a `request_confirmation(` call; ADK TypeScript has **no** declarative confirmation option on `FunctionTool`, so ADK-021 treats a `requestConfirmation(` call in the handler as the gate; CrewAI, AutoGen and Claude TS have no tool-level gate (`Task(human_input=True)`, `human_input_mode`, and `canUseTool`/`permissionMode` live on the task/agent/`query()` options, which a tool-scope rule cannot see), so those rules can fire on a tool gated elsewhere. One small discovery change: `DiscoverADKTools` now copies `FunctionTool(...)` keyword arguments into `ToolDef.Config` (previously dropped), which is what lets ADK-020 see `require_confirmation`. Known discovery limits carried into the rules: Vercel tools are matched only through a `const x = tool(...)` binding (inline `tools: { x: tool(...) }` has no name); Claude TS reads parameter names only from an inline object-literal schema; CrewAI `BaseTool` subclasses, autogen_core `FunctionTool`, and ADK bare functions in `tools=[...]` are not discovered. No schema/predicate/`schema_version` change. Rule count **304** (298 + 6; re-derive rather than trust this figure). Prior review 2026-09-29 (pending merge commit; `origin/main` merged into `feat/p0-side-effect-bounds-batch1`) — Batch 1 of the side-effect-bounds rule (competitive-analysis backlog rank 12, P0): new rules **OAI-030** (Python), **OAI-031** (TypeScript), **PYD-014**, **MCP-030**, and **LC-025** — each flags a send/notify or refund/charge/pay/payout/transfer/ diff --git a/README.md b/README.md index 3e2cc1e9..d208840a 100644 --- a/README.md +++ b/README.md @@ -468,6 +468,13 @@ files in the scanned repo. Each run produces a `ScanResult` containing: instrumented in its manifest and produces no traces at all. Detected statically in Python and TypeScript/JavaScript only — Trustabl does not ingest runtime traces. +- **Process labels** (`otel_labels`) — the `agent-reliability-otel-labels` + calls found in code (import, `Run` construction, run start, tool-span label, + run end) and any `trustabl.*` label keys written by hand, with the file and + line and the facts the OBS-006..012 rules read: whether a `Run` is shared, + whether a label runs when the tool fails, whether `attempt` is a fixed + number, and the literal `side_effect` / `exit_reason` values. Python and + TypeScript/JavaScript only. Not folded into the scan ID. - **Server entrypoints** (`entrypoints`, omitted when none) — Python functions a server framework invokes for an external request or job: FastAPI and Flask diff --git a/internal/analysis/otel_labels.go b/internal/analysis/otel_labels.go index decb162a..cc27b492 100644 --- a/internal/analysis/otel_labels.go +++ b/internal/analysis/otel_labels.go @@ -220,6 +220,9 @@ func otelLabelsCallSignals(pf ParsedFile, lang models.Language, test bool, alias } var out []models.OtelLabelsSignal astutil.Walk(pf.Tree.RootNode(), func(n *sitter.Node) bool { + if sig, ok := otelLabelsRunSignal(n, pf, lang, test, aliases); ok { + out = append(out, sig) + } if n.Type() != callType { return true } @@ -232,14 +235,16 @@ func otelLabelsCallSignals(pf ParsedFile, lang models.Language, test bool, alias if name != c.Name || (c.NeedsReceiver && !hasReceiver) { continue } - out = append(out, models.OtelLabelsSignal{ + sig := models.OtelLabelsSignal{ Kind: c.Kind, File: pf.RelPath, StartLine: astutil.NodeLine(n), EndLine: astutil.NodeEndLine(n), Language: lang, TestOrigin: test, - }) + } + otelLabelsCallFacts(&sig, n, pf.Source, lang) + out = append(out, sig) } return true }) diff --git a/internal/analysis/otel_labels_facts.go b/internal/analysis/otel_labels_facts.go new file mode 100644 index 00000000..3985f36c --- /dev/null +++ b/internal/analysis/otel_labels_facts.go @@ -0,0 +1,320 @@ +package analysis + +import ( + "slices" + "strings" + + sitter "github.com/smacker/go-tree-sitter" + + "github.com/trustabl/trustabl/internal/analysis/astutil" + "github.com/trustabl/trustabl/internal/models" +) + +// otelLabelsPos is where a library call sits relative to its enclosing +// function. The walk up stops at the first function, lambda or method, so a +// finally block in an outer function does not count for a call inside a +// nested one. +type otelLabelsPos struct { + fn *sitter.Node // nearest enclosing function, lambda or method; nil at module scope + inFinally bool // a finally clause lies between the call and fn + inExcept bool // an except / catch clause lies between the call and fn + defaultParam bool // the call is a default parameter value of fn + mainGuard bool // module scope, under `if __name__ == "__main__":` + instanceInit bool // TS/JS: initializer of a non-static class field, run once per instance +} + +var otelLabelsFuncTypes = map[models.Language]map[string]bool{ + models.LanguagePython: {"function_definition": true, "lambda": true}, + models.LanguageTypeScript: { + "function_declaration": true, + "function_expression": true, + "function": true, + "arrow_function": true, + "method_definition": true, + "generator_function_declaration": true, + "generator_function": true, + }, +} + +func otelLabelsPosition(n *sitter.Node, src []byte, lang models.Language) otelLabelsPos { + var pos otelLabelsPos + funcs := otelLabelsFuncTypes[lang] + for p := n.Parent(); p != nil; p = p.Parent() { + t := p.Type() + if funcs[t] { + pos.fn = p + return pos + } + switch t { + case "finally_clause": + pos.inFinally = true + case "except_clause", "except_group_clause", "catch_clause": + pos.inExcept = true + case "default_parameter", "typed_default_parameter": + pos.defaultParam = true + case "public_field_definition", "field_definition": + // tree-sitter-typescript / tree-sitter-javascript. A non-static + // field initializer runs per instance; a static one runs once. + if lang == models.LanguageTypeScript && !hasStaticModifier(p) { + pos.instanceInit = true + } + case "if_statement": + if lang == models.LanguagePython && isPyMainGuard(p, src) { + pos.mainGuard = true + } + } + } + return pos +} + +// hasStaticModifier reports whether a class field node carries the `static` +// keyword (an anonymous child of the field definition). +func hasStaticModifier(field *sitter.Node) bool { + for i := 0; i < int(field.ChildCount()); i++ { + if field.Child(i).Type() == "static" { + return true + } + } + return false +} + +// isPyMainGuard reports whether an if statement is `if __name__ == "__main__":` +// in either operand order and quote style, with or without one pair of +// surrounding parentheses. +func isPyMainGuard(ifStmt *sitter.Node, src []byte) bool { + cond := strings.Join(strings.Fields(astutil.NodeText(ifStmt.ChildByFieldName("condition"), src)), "") + cond = strings.ReplaceAll(cond, "'", `"`) + if len(cond) > 1 && cond[0] == '(' && cond[len(cond)-1] == ')' { + cond = cond[1 : len(cond)-1] + } + return cond == `__name__=="__main__"` || cond == `"__main__"==__name__` +} + +// resolvesToOtelLabelsRun reports whether a callee names the library's Run in +// this file: a bare name bound to Run by an import, or a member of a module or +// namespace alias. Never repo-wide (spec D5). +func resolvesToOtelLabelsRun(callee string, aliases map[string]string) bool { + callee = strings.TrimSpace(callee) + if i := strings.LastIndex(callee, "."); i >= 0 { + recv, name := callee[:i], callee[i+1:] + return name == "Run" && (aliases[recv] == "*" || isOtelLabelsPyModule(recv)) + } + return aliases[callee] == "Run" +} + +// otelLabelsRunSignal returns a run_construct signal when n builds the +// library's Run: Python `Run(...)`, TS `new Run(...)`. +// +// ModuleScope is true when the Run is built once rather than per run: no +// enclosing function (a Python class body or a TS/JS static class field +// included; a non-static TS/JS class field initializer runs per instance, like +// Python's `self.run = Run()` in __init__, and is not module scope), or — in Python only — a default +// parameter value, which Python evaluates once at definition time. A Python +// construct under `if __name__ == "__main__":` is a one-run script and is not +// module scope. RunIDConst is true when the run id is a string literal. +func otelLabelsRunSignal(n *sitter.Node, pf ParsedFile, lang models.Language, test bool, aliases map[string]string) (models.OtelLabelsSignal, bool) { + var callee *sitter.Node + switch { + case lang == models.LanguagePython && n.Type() == "call": + callee = n.ChildByFieldName("function") + case lang == models.LanguageTypeScript && n.Type() == "new_expression": + callee = n.ChildByFieldName("constructor") + default: + return models.OtelLabelsSignal{}, false + } + if callee == nil || !resolvesToOtelLabelsRun(astutil.NodeText(callee, pf.Source), aliases) { + return models.OtelLabelsSignal{}, false + } + pos := otelLabelsPosition(n, pf.Source, lang) + moduleScope := pos.fn == nil && !pos.mainGuard && !pos.instanceInit + if lang == models.LanguagePython && pos.defaultParam { + moduleScope = true + } + return models.OtelLabelsSignal{ + Kind: models.OtelLabelsRunConstruct, + File: pf.RelPath, + StartLine: astutil.NodeLine(n), + EndLine: astutil.NodeEndLine(n), + Language: lang, + TestOrigin: test, + ModuleScope: moduleScope, + RunIDConst: runIDConst(n, pf.Source, lang), + }, true +} + +// runIDConst reports whether a Run is given a literal run id: Python +// Run("x") / Run(run_id="x"), TS new Run("x"). Every run then shares that id. +func runIDConst(n *sitter.Node, src []byte, lang models.Language) bool { + args := n.ChildByFieldName("arguments") + if args == nil { + return false + } + if lang == models.LanguagePython { + if v, ok := astutil.KwargValue(n, src, "run_id"); ok { + _, lit := pyStringLiteral(v) + return lit + } + first := firstPositionalArg(args) + if first == nil { + return false + } + _, lit := pyStringLiteral(astutil.NodeText(first, src)) + return lit + } + first := firstPositionalArg(args) + return first != nil && first.Type() == "string" +} + +// otelLabelsErrorCallbacks are function names callback-based frameworks call +// on the failure path (LangChain's callback handlers, and the TypeScript +// spelling). +var otelLabelsErrorCallbacks = map[string]bool{ + "on_tool_error": true, + "on_chain_error": true, + "handleToolError": true, +} + +// otelLabelsCallFacts stamps the position and literal facts a tool_span or +// run_end signal carries (spec §4.2). +func otelLabelsCallFacts(sig *models.OtelLabelsSignal, call *sitter.Node, src []byte, lang models.Language) { + pos := otelLabelsPosition(call, src, lang) + errorCallback := otelLabelsErrorCallbacks[otelLabelsFuncName(pos.fn, src)] + switch sig.Kind { + case models.OtelLabelsToolSpan: + sig.FailureSafe = pos.inFinally || pos.inExcept || errorCallback || + isPromiseHandler(pos.fn, src, "finally", "catch") + sig.HookUnknown = lang == models.LanguagePython && isSuccessHook(pos.fn, src) + sig.AttemptConst = attemptConst(call, src, lang) + sig.SideEffect = sideEffectLiteral(call, src, lang) + case models.OtelLabelsRunEnd: + // finally does not count: a fixed reason there is written on success + // and failure alike (spec D9). + sig.ErrorPath = pos.inExcept || errorCallback || isPromiseHandler(pos.fn, src, "catch") + sig.ExitReason = exitReasonLiteral(call, src, lang) + } +} + +func otelLabelsFuncName(fn *sitter.Node, src []byte) string { + if fn == nil { + return "" + } + return astutil.NodeText(fn.ChildByFieldName("name"), src) +} + +// isPromiseHandler reports whether fn is passed directly to a promise method +// such as `.finally(...)` or `.catch(...)`. +func isPromiseHandler(fn *sitter.Node, src []byte, methods ...string) bool { + if fn == nil { + return false + } + args := fn.Parent() + if args == nil || args.Type() != "arguments" { + return false + } + call := args.Parent() + if call == nil || call.Type() != "call_expression" { + return false + } + member := call.ChildByFieldName("function") + if member == nil || member.Type() != "member_expression" { + return false + } + return slices.Contains(methods, astutil.NodeText(member.ChildByFieldName("property"), src)) +} + +// isSuccessHook reports whether fn is an `on_tool_end` method of a class that +// subclasses OpenAI Agents' RunHooks or AgentHooks. Whether those hooks run +// after a tool raises is unverified (spec O8), so such a label is neither safe +// nor unsafe (spec D10). +func isSuccessHook(fn *sitter.Node, src []byte) bool { + if fn == nil || fn.Type() != "function_definition" || otelLabelsFuncName(fn, src) != "on_tool_end" { + return false + } + for p := fn.Parent(); p != nil; p = p.Parent() { + switch p.Type() { + case "class_definition": + bases := astutil.NodeText(p.ChildByFieldName("superclasses"), src) + return strings.Contains(bases, "RunHooks") || strings.Contains(bases, "AgentHooks") + case "function_definition", "lambda": + return false + } + } + return false +} + +func attemptConst(call *sitter.Node, src []byte, lang models.Language) bool { + if lang == models.LanguagePython { + v, ok := astutil.KwargValue(call, src, "attempt") + return ok && isIntLiteral(v) + } + v := tsOptionValue(call, src, "attempt") + return v != nil && v.Type() == "number" +} + +func isIntLiteral(s string) bool { + s = strings.TrimSpace(s) + if s == "" { + return false + } + for _, r := range s { + if r < '0' || r > '9' { + return false + } + } + return true +} + +// tsOptionValue returns the value node of a property in the options object +// passed as the call's second argument. Nil when the property is absent, +// written as shorthand (`{ attempt }` is a variable), or the argument is not an +// object literal. +func tsOptionValue(call *sitter.Node, src []byte, key string) *sitter.Node { + args := call.ChildByFieldName("arguments") + if args == nil || args.NamedChildCount() < 2 { + return nil + } + obj := args.NamedChild(1) + if obj.Type() != "object" { + return nil + } + for i := 0; i < int(obj.NamedChildCount()); i++ { + p := obj.NamedChild(i) + if p.Type() != "pair" { + continue + } + if strings.Trim(astutil.NodeText(p.ChildByFieldName("key"), src), `"'`) == key { + return p.ChildByFieldName("value") + } + } + return nil +} + +func sideEffectLiteral(call *sitter.Node, src []byte, lang models.Language) string { + if lang == models.LanguagePython { + v, ok := astutil.KwargValue(call, src, "side_effect") + if !ok { + return "" + } + s, _ := pyStringLiteral(v) + return s + } + return tsStringValue(tsOptionValue(call, src, "sideEffect"), src) +} + +// exitReasonLiteral returns the literal exit reason: Python's exit_reason= +// keyword (required by the library), TS markRunEnd's second argument. +func exitReasonLiteral(call *sitter.Node, src []byte, lang models.Language) string { + if lang == models.LanguagePython { + v, ok := astutil.KwargValue(call, src, "exit_reason") + if !ok { + return "" + } + s, _ := pyStringLiteral(v) + return s + } + args := call.ChildByFieldName("arguments") + if args == nil || args.NamedChildCount() < 2 { + return "" + } + return tsStringValue(args.NamedChild(1), src) +} diff --git a/internal/analysis/otel_labels_test.go b/internal/analysis/otel_labels_test.go index 96b76329..24c70ecb 100644 --- a/internal/analysis/otel_labels_test.go +++ b/internal/analysis/otel_labels_test.go @@ -66,6 +66,7 @@ def run_agent(prompt): got := analysis.DiscoverOtelLabels([]analysis.ParsedFile{pf}) want := []models.OtelLabelsSignalKind{ models.OtelLabelsImport, + models.OtelLabelsRunConstruct, models.OtelLabelsRunStart, models.OtelLabelsToolSpan, models.OtelLabelsRunEnd, @@ -79,8 +80,8 @@ def run_agent(prompt): t.Errorf("signal %d: got %q, want %q", i, kinds[i], want[i]) } } - if got[1].StartLine != 7 { - t.Errorf("run_start line: got %d, want 7", got[1].StartLine) + if got[2].StartLine != 7 { + t.Errorf("run_start line: got %d, want 7", got[2].StartLine) } } @@ -321,3 +322,343 @@ func TestOtelLabelsKeys_MatchVendoredManifest(t *testing.T) { t.Errorf("engine keys %v, manifest attributes %v", got, want) } } + +func runConstructs(sigs []models.OtelLabelsSignal) []models.OtelLabelsSignal { + var out []models.OtelLabelsSignal + for _, s := range sigs { + if s.Kind == models.OtelLabelsRunConstruct { + out = append(out, s) + } + } + return out +} + +func TestDiscoverOtelLabels_RunConstructScope(t *testing.T) { + cases := []struct { + name, src string + wantModuleScope bool + wantRunIDConst bool + }{ + {"per run, inside a function", `from agent_reliability_otel_labels import Run + +def run_agent(): + run = Run() +`, false, false}, + {"module scope", `from agent_reliability_otel_labels import Run + +RUN = Run() +`, true, false}, + {"class body", `from agent_reliability_otel_labels import Run + +class Service: + run = Run() +`, true, false}, + {"default parameter, evaluated once", `from agent_reliability_otel_labels import Run + +def handle(req, run=Run()): + return req +`, true, false}, + {"script main guard", `from agent_reliability_otel_labels import Run + +if __name__ == "__main__": + run = Run() +`, false, false}, + {"script main guard, parenthesized", `from agent_reliability_otel_labels import Run + +if (__name__ == "__main__"): + run = Run() +`, false, false}, + {"literal run id, positional", `from agent_reliability_otel_labels import Run + +def run_agent(): + run = Run("fixed") +`, false, true}, + {"literal run id, keyword", `from agent_reliability_otel_labels import Run + +def run_agent(): + run = Run(run_id="fixed") +`, false, true}, + {"per-request run id", `from agent_reliability_otel_labels import Run + +def run_agent(request_id): + run = Run(run_id=request_id) +`, false, false}, + {"module alias", `import agent_reliability_otel_labels as otl + +def run_agent(): + run = otl.Run() +`, false, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := runConstructs(analysis.DiscoverOtelLabels([]analysis.ParsedFile{parsePyFile(t, "agent.py", tc.src)})) + if len(got) != 1 { + t.Fatalf("got %d run_construct signals, want 1: %+v", len(got), got) + } + if got[0].ModuleScope != tc.wantModuleScope { + t.Errorf("ModuleScope = %v, want %v", got[0].ModuleScope, tc.wantModuleScope) + } + if got[0].RunIDConst != tc.wantRunIDConst { + t.Errorf("RunIDConst = %v, want %v", got[0].RunIDConst, tc.wantRunIDConst) + } + }) + } +} + +// Run is too generic a name to trust repo-wide (spec D5): a Run imported from +// somewhere else is not the library's, even when the library is imported. +func TestDiscoverOtelLabels_RunFromAnotherModuleIsIgnored(t *testing.T) { + src := `from agent_reliability_otel_labels import mark_run_end +from myapp.jobs import Run + +RUN = Run() +` + got := runConstructs(analysis.DiscoverOtelLabels([]analysis.ParsedFile{parsePyFile(t, "agent.py", src)})) + if len(got) != 0 { + t.Errorf("myapp.jobs.Run recorded as the library's Run: %+v", got) + } +} + +func TestDiscoverOtelLabels_TSRunConstructScope(t *testing.T) { + cases := []struct { + name, src string + wantModuleScope bool + wantRunIDConst bool + }{ + {"per run", `import { Run } from "@trustabl/agent-reliability-otel-labels"; +export function runAgent() { const run = new Run(); } +`, false, false}, + {"module scope", `import { Run } from "@trustabl/agent-reliability-otel-labels"; +const RUN = new Run(); +`, true, false}, + // TypeScript evaluates a default parameter per call, unlike Python. + {"default parameter", `import { Run } from "@trustabl/agent-reliability-otel-labels"; +export function handle(req: unknown, run = new Run()) { return req; } +`, false, false}, + {"literal run id", `import { Run } from "@trustabl/agent-reliability-otel-labels"; +export function runAgent() { const run = new Run("fixed"); } +`, false, true}, + {"namespace import", `import * as otl from "@trustabl/agent-reliability-otel-labels"; +const RUN = new otl.Run(); +`, true, false}, + // A non-static class field initializer runs once per instance, like + // Python's `self.run = Run()` in __init__, which is not flagged. + {"instance class field", `import { Run } from "@trustabl/agent-reliability-otel-labels"; +class Svc { run = new Run(); } +`, false, false}, + {"static class field is shared", `import { Run } from "@trustabl/agent-reliability-otel-labels"; +class Svc { static run = new Run(); } +`, true, false}, + {"instance field of an exported class", `import { Run } from "@trustabl/agent-reliability-otel-labels"; +export class Svc { private run = new Run(); } +`, false, false}, + {"arrow function in a static field", `import { Run } from "@trustabl/agent-reliability-otel-labels"; +class Svc { static make = () => new Run(); } +`, false, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := runConstructs(analysis.DiscoverOtelLabels([]analysis.ParsedFile{parseTSForTest(t, "agent.ts", tc.src)})) + if len(got) != 1 { + t.Fatalf("got %d run_construct signals, want 1: %+v", len(got), got) + } + if got[0].ModuleScope != tc.wantModuleScope { + t.Errorf("ModuleScope = %v, want %v", got[0].ModuleScope, tc.wantModuleScope) + } + if got[0].RunIDConst != tc.wantRunIDConst { + t.Errorf("RunIDConst = %v, want %v", got[0].RunIDConst, tc.wantRunIDConst) + } + }) + } +} + +func firstOfKind(t *testing.T, sigs []models.OtelLabelsSignal, kind models.OtelLabelsSignalKind) models.OtelLabelsSignal { + t.Helper() + for _, s := range sigs { + if s.Kind == kind { + return s + } + } + t.Fatalf("no %q signal in %+v", kind, sigs) + return models.OtelLabelsSignal{} +} + +func TestDiscoverOtelLabels_PythonToolSpanFacts(t *testing.T) { + const imp = "from agent_reliability_otel_labels import mark_tool_span\n\n" + cases := []struct { + name, body string + failureSafe bool + hookUnknown bool + attempt bool + sideEffect string + }{ + {"after the call", `def tool(span, args): + result = find(args) + mark_tool_span(span, args=args, result=result, attempt=n, side_effect="read") +`, false, false, false, "read"}, + {"in finally", `def tool(span, args): + result = None + try: + result = find(args) + finally: + mark_tool_span(span, args=args, result=result, attempt=1, side_effect="write") +`, true, false, true, "write"}, + {"in a try nested in finally (forge's pattern)", `def tool(span, args): + try: + return find(args) + finally: + try: + mark_tool_span(span, args=args, result=None, attempt=attempts[key]) + except ValueError: + pass +`, true, false, false, ""}, + {"in except", `def tool(span, args): + try: + return find(args) + except Exception: + mark_tool_span(span, args=args, result=None, attempt=n) + raise +`, true, false, false, ""}, + {"error callback", `class Handler: + def on_tool_error(self, error, **kw): + mark_tool_span(self.span, args={}, result=None, attempt=self.n) +`, true, false, false, ""}, + {"OpenAI Agents success hook", `class Hooks(RunHooks): + async def on_tool_end(self, context, agent, tool, result): + mark_tool_span(self.span, args={}, result=result, attempt=self.n) +`, false, true, false, ""}, + {"on_tool_end of an unrelated class", `class Hooks: + def on_tool_end(self, result): + mark_tool_span(self.span, args={}, result=result, attempt=self.n) +`, false, false, false, ""}, + {"finally in an outer function does not reach a nested one", `def outer(span): + try: + pass + finally: + def inner(): + mark_tool_span(span, args={}, result=None, attempt=n) +`, false, false, false, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := analysis.DiscoverOtelLabels([]analysis.ParsedFile{parsePyFile(t, "agent.py", imp+tc.body)}) + s := firstOfKind(t, got, models.OtelLabelsToolSpan) + if s.FailureSafe != tc.failureSafe { + t.Errorf("FailureSafe = %v, want %v", s.FailureSafe, tc.failureSafe) + } + if s.HookUnknown != tc.hookUnknown { + t.Errorf("HookUnknown = %v, want %v", s.HookUnknown, tc.hookUnknown) + } + if s.AttemptConst != tc.attempt { + t.Errorf("AttemptConst = %v, want %v", s.AttemptConst, tc.attempt) + } + if s.SideEffect != tc.sideEffect { + t.Errorf("SideEffect = %q, want %q", s.SideEffect, tc.sideEffect) + } + }) + } +} + +func TestDiscoverOtelLabels_PythonRunEndFacts(t *testing.T) { + const imp = "from agent_reliability_otel_labels import mark_run_end\n\n" + cases := []struct { + name, body string + errorPath bool + reason string + }{ + {"success path", `def run(root): + mark_run_end(root, exit_reason="final_answer") +`, false, "final_answer"}, + {"except", `def run(root): + try: + work() + except Exception: + mark_run_end(root, exit_reason="error") + raise +`, true, "error"}, + // A fixed success reason in finally mislabels every failed run (D9). + {"finally is not the error path", `def run(root): + try: + work() + finally: + mark_run_end(root, exit_reason="final_answer") +`, false, "final_answer"}, + {"variable reason", `def run(root, reason): + mark_run_end(root, exit_reason=reason) +`, false, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := analysis.DiscoverOtelLabels([]analysis.ParsedFile{parsePyFile(t, "agent.py", imp+tc.body)}) + s := firstOfKind(t, got, models.OtelLabelsRunEnd) + if s.ErrorPath != tc.errorPath { + t.Errorf("ErrorPath = %v, want %v", s.ErrorPath, tc.errorPath) + } + if s.ExitReason != tc.reason { + t.Errorf("ExitReason = %q, want %q", s.ExitReason, tc.reason) + } + }) + } +} + +func TestDiscoverOtelLabels_TSFacts(t *testing.T) { + src := `import { markRunEnd, markToolSpan } from "@trustabl/agent-reliability-otel-labels"; + +export async function safe(span: any, args: any, attempt: number) { + let result: unknown; + try { + result = await find(args); + } finally { + markToolSpan(span, { args, result, attempt, sideEffect: "read" }); + } +} + +export function chained(span: any, args: any) { + return find(args).finally(() => markToolSpan(span, { args, result: null, attempt: 1 })); +} + +export async function run(root: any) { + try { + await work(); + markRunEnd(root, "final_answer"); + } catch (err) { + markRunEnd(root, "error"); + throw err; + } +} + +export function viaCatch(root: any) { + return work().catch(() => markRunEnd(root, "error")); +} +` + got := analysis.DiscoverOtelLabels([]analysis.ParsedFile{parseTSForTest(t, "agent.ts", src)}) + var spans, ends []models.OtelLabelsSignal + for _, s := range got { + switch s.Kind { + case models.OtelLabelsToolSpan: + spans = append(spans, s) + case models.OtelLabelsRunEnd: + ends = append(ends, s) + } + } + if len(spans) != 2 || len(ends) != 3 { + t.Fatalf("got %d tool spans and %d run ends, want 2 and 3: %+v", len(spans), len(ends), got) + } + // try/finally; the shorthand { attempt } is a variable, not a literal. + if !spans[0].FailureSafe || spans[0].AttemptConst || spans[0].SideEffect != "read" { + t.Errorf("finally span: %+v", spans[0]) + } + // .finally(() => ...) runs on rejection too. + if !spans[1].FailureSafe || !spans[1].AttemptConst { + t.Errorf(".finally span: %+v", spans[1]) + } + if ends[0].ErrorPath || ends[0].ExitReason != "final_answer" { + t.Errorf("success run end: %+v", ends[0]) + } + if !ends[1].ErrorPath || ends[1].ExitReason != "error" { + t.Errorf("catch run end: %+v", ends[1]) + } + if !ends[2].ErrorPath || ends[2].ExitReason != "error" { + t.Errorf(".catch run end: %+v", ends[2]) + } +} diff --git a/internal/models/models.go b/internal/models/models.go index f7c86fed..b2f60f63 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -442,6 +442,9 @@ const ( OtelLabelsRunStart OtelLabelsSignalKind = "run_start" OtelLabelsToolSpan OtelLabelsSignalKind = "tool_span" OtelLabelsRunEnd OtelLabelsSignalKind = "run_end" + // OtelLabelsRunConstruct is a Run() / new Run() resolved through the + // file's import of the library. + OtelLabelsRunConstruct OtelLabelsSignalKind = "run_construct" // OtelLabelsAttrLiteral is a trustabl.* label key written to a span by // hand (set_attribute / setAttribute), without the library. OtelLabelsAttrLiteral OtelLabelsSignalKind = "attr_literal" @@ -455,7 +458,8 @@ const ( // repo_observability_initialized — the library would clear its own // prerequisite and OBS-001 would misfire on it. // -// The fact fields are zero when they do not apply to the Kind. +// The fact fields are zero when they do not apply to the Kind, or when the +// value is not a literal. type OtelLabelsSignal struct { Kind OtelLabelsSignalKind `json:"kind"` File string `json:"file"` @@ -463,8 +467,16 @@ type OtelLabelsSignal struct { EndLine int `json:"end_line"` Language Language `json:"language"` - TestOrigin bool `json:"test_origin,omitempty"` // all kinds: File classifies as a test - Key string `json:"key,omitempty"` // attr_literal: the trustabl.* key + TestOrigin bool `json:"test_origin,omitempty"` // all kinds: File classifies as a test + ModuleScope bool `json:"module_scope,omitempty"` // run_construct: built once, not per run + RunIDConst bool `json:"run_id_const,omitempty"` // run_construct: the run id is a string literal + FailureSafe bool `json:"failure_safe,omitempty"` // tool_span: runs when the tool raises + HookUnknown bool `json:"hook_unknown,omitempty"` // tool_span: inside a framework success hook + ErrorPath bool `json:"error_path,omitempty"` // run_end: on the error path (not finally) + AttemptConst bool `json:"attempt_const,omitempty"` // tool_span: attempt is an integer literal + SideEffect string `json:"side_effect,omitempty"` // tool_span: literal side_effect value + ExitReason string `json:"exit_reason,omitempty"` // run_end: literal exit_reason value + Key string `json:"key,omitempty"` // attr_literal: the trustabl.* key } // DepRef is one dependency declared in a repo manifest — the agent-path @@ -676,26 +688,31 @@ type ScanResult struct { // Observability reports the instrumentation found in code — the positive // signal, not only the findings. Additive inventory, NOT folded into ScanID // (same treatment as Dependencies). - Observability []ObservabilitySignal `json:"observability,omitempty"` - Entrypoints []EntrypointDef `json:"entrypoints,omitempty"` // server entrypoints (Python): a fact only, no rule consumes it yet - Vulnerabilities []DepVuln `json:"vulnerabilities,omitempty"` // --vuln-scan OSV matches (TR-271); absent on the default path - Secrets []SecretMatch `json:"secrets,omitempty"` // --secret-scan hardcoded-credential matches; absent on the default path - SlashCommands []SlashCommandDef `json:"slash_commands"` - PluginManifests []PluginManifest `json:"plugin_manifests"` - ClaudeSettings []ClaudeSettings `json:"claude_settings"` - Findings []Finding `json:"findings"` - Surfaces []SurfaceReadiness `json:"surfaces"` - OverallScore float64 `json:"overall_score"` - ProjectedScores ProjectedScores `json:"projected_scores"` - RulesSource string `json:"rules_source"` // repo the rule pack came from - RulesVersion string `json:"rules_version"` // resolved rules commit SHA - RulesFromCache bool `json:"rules_from_cache"` // true if rules came from cache (network skipped/unreachable) - RulesStale bool `json:"rules_stale,omitempty"` // true if the cached signed bundle's channel statement has expired - RulesSchemaVersion int `json:"rules_schema_version,omitempty"` // pack manifest's declared schema_version - RulesSchemaNewer bool `json:"rules_schema_newer,omitempty"` // pack targets a schema newer than this build supports - RulesSkipped []string `json:"rules_skipped,omitempty"` // rule IDs dropped as forward-incompatible (sorted, deduped) - RulesOrigin RulesOrigin `json:"rules_origin"` // provenance of the rules (signed channel / unsigned / custom) - Coverage Coverage `json:"coverage"` // how many source files parsed vs. were skipped + Observability []ObservabilitySignal `json:"observability,omitempty"` + // OtelLabels reports the agent-reliability-otel-labels calls and + // hand-written label keys found in code, with the facts the OBS-006..012 + // rules read. Additive inventory, NOT folded into ScanID (same treatment + // as Observability). + OtelLabels []OtelLabelsSignal `json:"otel_labels,omitempty"` + Entrypoints []EntrypointDef `json:"entrypoints,omitempty"` // server entrypoints (Python): a fact only, no rule consumes it yet + Vulnerabilities []DepVuln `json:"vulnerabilities,omitempty"` // --vuln-scan OSV matches (TR-271); absent on the default path + Secrets []SecretMatch `json:"secrets,omitempty"` // --secret-scan hardcoded-credential matches; absent on the default path + SlashCommands []SlashCommandDef `json:"slash_commands"` + PluginManifests []PluginManifest `json:"plugin_manifests"` + ClaudeSettings []ClaudeSettings `json:"claude_settings"` + Findings []Finding `json:"findings"` + Surfaces []SurfaceReadiness `json:"surfaces"` + OverallScore float64 `json:"overall_score"` + ProjectedScores ProjectedScores `json:"projected_scores"` + RulesSource string `json:"rules_source"` // repo the rule pack came from + RulesVersion string `json:"rules_version"` // resolved rules commit SHA + RulesFromCache bool `json:"rules_from_cache"` // true if rules came from cache (network skipped/unreachable) + RulesStale bool `json:"rules_stale,omitempty"` // true if the cached signed bundle's channel statement has expired + RulesSchemaVersion int `json:"rules_schema_version,omitempty"` // pack manifest's declared schema_version + RulesSchemaNewer bool `json:"rules_schema_newer,omitempty"` // pack targets a schema newer than this build supports + RulesSkipped []string `json:"rules_skipped,omitempty"` // rule IDs dropped as forward-incompatible (sorted, deduped) + RulesOrigin RulesOrigin `json:"rules_origin"` // provenance of the rules (signed channel / unsigned / custom) + Coverage Coverage `json:"coverage"` // how many source files parsed vs. were skipped // NoAgentSurfaces marks a scan that found nothing to evaluate: no tools, // agents, subagents or skills. Scoring an empty set yields 1.0, which reads // as a perfect result when in fact nothing was checked, so consumers must be diff --git a/internal/rules/evaluator.go b/internal/rules/evaluator.go index 809436f4..7086c4dc 100644 --- a/internal/rules/evaluator.go +++ b/internal/rules/evaluator.go @@ -178,6 +178,18 @@ func (e MatchExpr) EvaluateRepo(p models.RepoProfile, inv models.RepoInventory) if e.RepoOtelLabelsImplemented != nil && !PredRepoOtelLabelsImplemented(*e.RepoOtelLabelsImplemented, inv) { return false } + if e.RepoOtelLabelsRunShared != nil && !PredRepoOtelLabelsRunShared(*e.RepoOtelLabelsRunShared, inv) { + return false + } + if e.RepoOtelLabelsToolSpanFailureUnsafe != nil && !PredRepoOtelLabelsToolSpanFailureUnsafe(*e.RepoOtelLabelsToolSpanFailureUnsafe, inv) { + return false + } + if e.RepoOtelLabelsAttemptConstant != nil && !PredRepoOtelLabelsAttemptConstant(*e.RepoOtelLabelsAttemptConstant, inv) { + return false + } + if e.RepoOtelLabelsRunEndSuccessOnly != nil && !PredRepoOtelLabelsRunEndSuccessOnly(*e.RepoOtelLabelsRunEndSuccessOnly, inv) { + return false + } if len(e.RepoClaudeDefaultModeIs) > 0 && !PredRepoClaudeDefaultModeIs(e.RepoClaudeDefaultModeIs, inv) { return false } @@ -516,6 +528,10 @@ var predicatesByScope = map[models.Scope]map[string]bool{ "repo_otel_labels_installed": true, "repo_otel_labels_initialized": true, "repo_otel_labels_implemented": true, + "repo_otel_labels_run_shared": true, + "repo_otel_labels_tool_span_failure_unsafe": true, + "repo_otel_labels_attempt_constant": true, + "repo_otel_labels_run_end_success_only": true, "repo_langgraph_platform_config_present": true, "repo_raw_anthropic_tool_output_in_system": true, "repo_raw_openai_tool_output_in_system": true, @@ -620,6 +636,10 @@ func (e MatchExpr) setPredicateNames() []string { add(e.RepoOtelLabelsInstalled != nil, "repo_otel_labels_installed") add(e.RepoOtelLabelsInitialized != nil, "repo_otel_labels_initialized") add(e.RepoOtelLabelsImplemented != nil, "repo_otel_labels_implemented") + add(e.RepoOtelLabelsRunShared != nil, "repo_otel_labels_run_shared") + add(e.RepoOtelLabelsToolSpanFailureUnsafe != nil, "repo_otel_labels_tool_span_failure_unsafe") + add(e.RepoOtelLabelsAttemptConstant != nil, "repo_otel_labels_attempt_constant") + add(e.RepoOtelLabelsRunEndSuccessOnly != nil, "repo_otel_labels_run_end_success_only") return n } diff --git a/internal/rules/otel_labels_evidence_test.go b/internal/rules/otel_labels_evidence_test.go new file mode 100644 index 00000000..d5004cab --- /dev/null +++ b/internal/rules/otel_labels_evidence_test.go @@ -0,0 +1,43 @@ +package rules + +import ( + "testing" + + "github.com/trustabl/trustabl/internal/models" +) + +func TestRepoRuleDetector_OtelLabelsFindingLocation(t *testing.T) { + yes := true + rule := RuleDef{ + ID: "OBS-TEST", Title: "t", Scope: models.ScopeRepo, Severity: "low", Confidence: 0.5, + Match: MatchExpr{All: []MatchExpr{{RepoOtelLabelsAttemptConstant: &yes}}}, + } + inv := models.RepoInventory{OtelLabelsSignals: []models.OtelLabelsSignal{ + {Kind: models.OtelLabelsToolSpan, File: "app/agent.py", StartLine: 12, EndLine: 14, AttemptConst: true}, + {Kind: models.OtelLabelsToolSpan, File: "tests/test_agent.py", StartLine: 3, EndLine: 3, AttemptConst: true, TestOrigin: true}, + }} + got := repoRuleDetector{rule: rule}.Detect(models.RepoProfile{}, inv) + if len(got) != 1 { + t.Fatalf("got %d findings, want 1", len(got)) + } + if got[0].FilePath != "app/agent.py" || got[0].StartLine != 12 || got[0].EndLine != 14 { + t.Errorf("location = %s:%d-%d, want app/agent.py:12-14", got[0].FilePath, got[0].StartLine, got[0].EndLine) + } +} + +// A repo rule backed by none of the correctness predicates keeps its empty +// location, so existing reports do not change. +func TestRepoRuleDetector_OtherRepoRulesKeepEmptyLocation(t *testing.T) { + yes := true + rule := RuleDef{ + ID: "OBS-TEST", Title: "t", Scope: models.ScopeRepo, Severity: "low", Confidence: 0.5, + Match: MatchExpr{RepoOtelLabelsInitialized: &yes}, + } + inv := models.RepoInventory{OtelLabelsSignals: []models.OtelLabelsSignal{ + {Kind: models.OtelLabelsRunStart, File: "app/agent.py", StartLine: 7, EndLine: 7}, + }} + got := repoRuleDetector{rule: rule}.Detect(models.RepoProfile{}, inv) + if len(got) != 1 || got[0].FilePath != "" || got[0].StartLine != 0 { + t.Errorf("got %+v, want one finding with an empty location", got) + } +} diff --git a/internal/rules/policies_test.go b/internal/rules/policies_test.go index ca8a5229..3c540c9b 100644 --- a/internal/rules/policies_test.go +++ b/internal/rules/policies_test.go @@ -3565,6 +3565,49 @@ def summarize(state): `, wantFires: false}, } +// otelLabelsCorrectnessInv is an instrumented repo that is fully labelled +// except for the signals passed in, so OBS-006..008 are silent and only the +// correctness rule under test can fire. +func otelLabelsCorrectnessInv(extra ...models.OtelLabelsSignal) models.RepoInventory { + base := []models.OtelLabelsSignal{ + {Kind: models.OtelLabelsImport, File: "agent.py", StartLine: 1, EndLine: 1}, + {Kind: models.OtelLabelsRunStart, File: "agent.py", StartLine: 5, EndLine: 5}, + } + return models.RepoInventory{ + SDKsDetected: []models.SDK{models.SDKOpenAIAgents}, + Manifest: models.ScanManifest{PythonFiles: []string{"agent.py"}}, + ObservabilitySignals: []models.ObservabilitySignal{ + {Vendor: models.VendorOTel, Kind: models.ObsSignalInit}, + }, + Tools: []models.ToolDef{{Name: "search", Kind: models.KindOpenAITool}}, + OtelLabelsSignals: append(base, extra...), + } +} + +var otelLabelsCorrectnessProfile = models.RepoProfile{ + Languages: []models.Language{models.LanguagePython}, + OtelLabelsDeclaredIn: []string{"pyproject.toml"}, +} + +// Correct labelling: a per-run Run, a counted attempt, a failure-safe tool +// span, and a run end on the error path. +var otelLabelsGood = []models.OtelLabelsSignal{ + {Kind: models.OtelLabelsRunConstruct, File: "agent.py", StartLine: 4, EndLine: 4}, + {Kind: models.OtelLabelsToolSpan, File: "agent.py", StartLine: 9, EndLine: 9, FailureSafe: true}, + {Kind: models.OtelLabelsRunEnd, File: "agent.py", StartLine: 11, EndLine: 11, ExitReason: "final_answer"}, + {Kind: models.OtelLabelsRunEnd, File: "agent.py", StartLine: 13, EndLine: 13, ExitReason: "error", ErrorPath: true}, +} + +// withOtel returns a copy of the good signals with one replaced or added. +func withOtel(replace int, s models.OtelLabelsSignal) []models.OtelLabelsSignal { + out := append([]models.OtelLabelsSignal(nil), otelLabelsGood...) + if replace >= 0 { + out[replace] = s + return out + } + return append(out, s) +} + // policyRepoRuleCases covers repo-scoped rules. var policyRepoRuleCases = []policyRepoCase{ // ─── LangChain repo rule (LC-201) ─────────────────────────────────────── @@ -4387,6 +4430,44 @@ var policyRepoRuleCases = []policyRepoCase{ }, }, false}, + // agent-reliability-otel-labels correctness (OBS-009..012). Each starts + // from a fully labelled repo and breaks one thing. + {"OBS-009 fires on a module-scope Run", "OBS-009", otelLabelsCorrectnessProfile, + otelLabelsCorrectnessInv(withOtel(0, models.OtelLabelsSignal{Kind: models.OtelLabelsRunConstruct, File: "agent.py", StartLine: 2, ModuleScope: true})...), + true}, + {"OBS-009 fires on a literal run id", "OBS-009", otelLabelsCorrectnessProfile, + otelLabelsCorrectnessInv(withOtel(0, models.OtelLabelsSignal{Kind: models.OtelLabelsRunConstruct, File: "agent.py", StartLine: 4, RunIDConst: true})...), + true}, + {"OBS-009 silent on a per-run Run", "OBS-009", otelLabelsCorrectnessProfile, + otelLabelsCorrectnessInv(otelLabelsGood...), false}, + {"OBS-009 silent when the shared Run is in a test file", "OBS-009", otelLabelsCorrectnessProfile, + otelLabelsCorrectnessInv(withOtel(-1, models.OtelLabelsSignal{Kind: models.OtelLabelsRunConstruct, File: "tests/test_agent.py", StartLine: 3, ModuleScope: true, TestOrigin: true})...), + false}, + {"OBS-010 fires when no tool-span label runs on failure", "OBS-010", otelLabelsCorrectnessProfile, + otelLabelsCorrectnessInv(withOtel(1, models.OtelLabelsSignal{Kind: models.OtelLabelsToolSpan, File: "agent.py", StartLine: 9})...), + true}, + {"OBS-010 silent when a tool-span label is failure-safe", "OBS-010", otelLabelsCorrectnessProfile, + otelLabelsCorrectnessInv(otelLabelsGood...), false}, + {"OBS-010 silent when labels are only in a success hook", "OBS-010", otelLabelsCorrectnessProfile, + otelLabelsCorrectnessInv(withOtel(1, models.OtelLabelsSignal{Kind: models.OtelLabelsToolSpan, File: "agent.py", StartLine: 9, HookUnknown: true})...), + false}, + {"OBS-011 fires when every attempt is a constant", "OBS-011", otelLabelsCorrectnessProfile, + otelLabelsCorrectnessInv(withOtel(1, models.OtelLabelsSignal{Kind: models.OtelLabelsToolSpan, File: "agent.py", StartLine: 9, FailureSafe: true, AttemptConst: true})...), + true}, + {"OBS-011 silent when the attempt is counted", "OBS-011", otelLabelsCorrectnessProfile, + otelLabelsCorrectnessInv(otelLabelsGood...), false}, + {"OBS-012 fires when the run end is recorded only on success", "OBS-012", otelLabelsCorrectnessProfile, + otelLabelsCorrectnessInv(otelLabelsGood[:3]...), true}, + {"OBS-012 silent when the error path records the run end", "OBS-012", otelLabelsCorrectnessProfile, + otelLabelsCorrectnessInv(otelLabelsGood...), false}, + {"OBS-012 silent when the repo is not instrumented", "OBS-012", + otelLabelsCorrectnessProfile, + func() models.RepoInventory { + inv := otelLabelsCorrectnessInv(otelLabelsGood[:3]...) + inv.ObservabilitySignals = nil + return inv + }(), + false}, // Mutual exclusion with the absence rules: a declared-but-unwired dep is // OBS-005 at medium, NOT "this project wires no observability" at low. {"OAI-203 silent when the dep is declared (OBS-005 owns that case)", "OAI-203", diff --git a/internal/rules/predicates.go b/internal/rules/predicates.go index 40d9dd57..c485a430 100644 --- a/internal/rules/predicates.go +++ b/internal/rules/predicates.go @@ -1795,6 +1795,109 @@ func PredRepoOtelLabelsImplemented(want bool, inv models.RepoInventory) bool { return implemented == want } +// countedOtelLabels returns the signals of one kind that a correctness check +// may count: test files never decide one (spec D8). +func countedOtelLabels(inv models.RepoInventory, kind models.OtelLabelsSignalKind) []models.OtelLabelsSignal { + var out []models.OtelLabelsSignal + for _, s := range inv.OtelLabelsSignals { + if s.Kind == kind && !s.TestOrigin { + out = append(out, s) + } + } + return out +} + +// otelRunSharedOffender returns the first Run built once rather than per run, +// or given a literal run id; nil when there is none. +func otelRunSharedOffender(inv models.RepoInventory) *models.OtelLabelsSignal { + for _, s := range countedOtelLabels(inv, models.OtelLabelsRunConstruct) { + if s.ModuleScope || s.RunIDConst { + return &s + } + } + return nil +} + +// otelToolSpanFailureUnsafeOffender returns the first tool-span label when no +// tool-span label runs on failure. Labels inside a framework success hook are +// left out: unknown, not unsafe (spec D10). One failure-safe label credits the +// repo (spec D6). +func otelToolSpanFailureUnsafeOffender(inv models.RepoInventory) *models.OtelLabelsSignal { + var spans []models.OtelLabelsSignal + for _, s := range countedOtelLabels(inv, models.OtelLabelsToolSpan) { + if !s.HookUnknown { + spans = append(spans, s) + } + } + if len(spans) == 0 { + return nil + } + for _, s := range spans { + if s.FailureSafe { + return nil + } + } + return &spans[0] +} + +// otelAttemptConstantOffender returns the first tool-span label when every one +// passes attempt as an integer literal. +func otelAttemptConstantOffender(inv models.RepoInventory) *models.OtelLabelsSignal { + spans := countedOtelLabels(inv, models.OtelLabelsToolSpan) + if len(spans) == 0 { + return nil + } + for _, s := range spans { + if !s.AttemptConst { + return nil + } + } + return &spans[0] +} + +// otelRunEndSuccessOnlyOffender returns the first run-end label when none can +// record a failure. A run end credits the repo when it sits on the error path, +// passes the literal "error", or passes a variable reason that may carry +// "error". A fixed reason in finally does not (spec D9). +func otelRunEndSuccessOnlyOffender(inv models.RepoInventory) *models.OtelLabelsSignal { + ends := countedOtelLabels(inv, models.OtelLabelsRunEnd) + if len(ends) == 0 { + return nil + } + for _, s := range ends { + if s.ErrorPath || s.ExitReason == "error" || s.ExitReason == "" { + return nil + } + } + return &ends[0] +} + +// PredRepoOtelLabelsRunShared reports whether a Run is shared between runs: +// built at module scope, in a class body or as a Python default argument, or +// given a literal run id. Every run then writes the same trustabl.run_id. +func PredRepoOtelLabelsRunShared(want bool, inv models.RepoInventory) bool { + return (otelRunSharedOffender(inv) != nil) == want +} + +// PredRepoOtelLabelsToolSpanFailureUnsafe reports whether tool spans are +// labelled only on success: tool-span labels exist and none sits in a +// finally / except / catch, an error callback, or a .finally / .catch handler. +func PredRepoOtelLabelsToolSpanFailureUnsafe(want bool, inv models.RepoInventory) bool { + return (otelToolSpanFailureUnsafeOffender(inv) != nil) == want +} + +// PredRepoOtelLabelsAttemptConstant reports whether every tool-span label +// passes attempt as an integer literal, so no retry can ever show. +func PredRepoOtelLabelsAttemptConstant(want bool, inv models.RepoInventory) bool { + return (otelAttemptConstantOffender(inv) != nil) == want +} + +// PredRepoOtelLabelsRunEndSuccessOnly reports whether no run-end label can +// record a failure. +func PredRepoOtelLabelsRunEndSuccessOnly(want bool, inv models.RepoInventory) bool { + return (otelRunEndSuccessOnlyOffender(inv) != nil) == want +} + // PredRepoClaudeOptionsModeWithoutKwarg fires when a SINGLE discovered // ClaudeAgentOptions(...) construction both sets permission_mode to one of // e.Modes and does not set e.Kwarg. This correlates the two facts at the diff --git a/internal/rules/predicates_test.go b/internal/rules/predicates_test.go index 671f69a6..2041fa8f 100644 --- a/internal/rules/predicates_test.go +++ b/internal/rules/predicates_test.go @@ -2043,3 +2043,101 @@ func TestPredRepoOtelLabels_HandWrittenKeysCredit(t *testing.T) { t.Error("trustabl.tool.attempt counts as a tool-span key") } } + +func otelInv(sigs ...models.OtelLabelsSignal) models.RepoInventory { + return models.RepoInventory{OtelLabelsSignals: sigs} +} + +func TestPredRepoOtelLabelsRunShared(t *testing.T) { + run := func(s models.OtelLabelsSignal) models.OtelLabelsSignal { + s.Kind = models.OtelLabelsRunConstruct + return s + } + cases := []struct { + name string + inv models.RepoInventory + want bool + }{ + {"no Run at all", otelInv(), false}, + {"per-run Run", otelInv(run(models.OtelLabelsSignal{})), false}, + {"module-scope Run", otelInv(run(models.OtelLabelsSignal{ModuleScope: true})), true}, + {"literal run id", otelInv(run(models.OtelLabelsSignal{RunIDConst: true})), true}, + {"module-scope Run only in a test file", otelInv(run(models.OtelLabelsSignal{ModuleScope: true, TestOrigin: true})), false}, + } + for _, tc := range cases { + if got := rules.PredRepoOtelLabelsRunShared(true, tc.inv); got != tc.want { + t.Errorf("%s: got %v, want %v", tc.name, got, tc.want) + } + } +} + +func TestPredRepoOtelLabelsToolSpanFailureUnsafe(t *testing.T) { + span := func(s models.OtelLabelsSignal) models.OtelLabelsSignal { + s.Kind = models.OtelLabelsToolSpan + return s + } + cases := []struct { + name string + inv models.RepoInventory + want bool + }{ + {"no tool spans", otelInv(), false}, + {"every span unsafe", otelInv(span(models.OtelLabelsSignal{}), span(models.OtelLabelsSignal{})), true}, + {"one safe span credits the repo", otelInv(span(models.OtelLabelsSignal{}), span(models.OtelLabelsSignal{FailureSafe: true})), false}, + {"only success-hook spans: unknown, not unsafe", otelInv(span(models.OtelLabelsSignal{HookUnknown: true})), false}, + {"unsafe span only in a test file", otelInv(span(models.OtelLabelsSignal{TestOrigin: true})), false}, + {"a safe test span does not credit unsafe production spans", otelInv(span(models.OtelLabelsSignal{}), span(models.OtelLabelsSignal{FailureSafe: true, TestOrigin: true})), true}, + } + for _, tc := range cases { + if got := rules.PredRepoOtelLabelsToolSpanFailureUnsafe(true, tc.inv); got != tc.want { + t.Errorf("%s: got %v, want %v", tc.name, got, tc.want) + } + } +} + +func TestPredRepoOtelLabelsAttemptConstant(t *testing.T) { + span := func(s models.OtelLabelsSignal) models.OtelLabelsSignal { + s.Kind = models.OtelLabelsToolSpan + return s + } + cases := []struct { + name string + inv models.RepoInventory + want bool + }{ + {"no tool spans", otelInv(), false}, + {"every attempt constant", otelInv(span(models.OtelLabelsSignal{AttemptConst: true})), true}, + {"one counted attempt credits the repo", otelInv(span(models.OtelLabelsSignal{AttemptConst: true}), span(models.OtelLabelsSignal{})), false}, + {"constant attempt only in a test file", otelInv(span(models.OtelLabelsSignal{AttemptConst: true, TestOrigin: true})), false}, + } + for _, tc := range cases { + if got := rules.PredRepoOtelLabelsAttemptConstant(true, tc.inv); got != tc.want { + t.Errorf("%s: got %v, want %v", tc.name, got, tc.want) + } + } +} + +func TestPredRepoOtelLabelsRunEndSuccessOnly(t *testing.T) { + end := func(s models.OtelLabelsSignal) models.OtelLabelsSignal { + s.Kind = models.OtelLabelsRunEnd + return s + } + cases := []struct { + name string + inv models.RepoInventory + want bool + }{ + {"no run ends", otelInv(), false}, + {"final_answer only", otelInv(end(models.OtelLabelsSignal{ExitReason: "final_answer"})), true}, + {"final_answer and max_steps, still no error", otelInv(end(models.OtelLabelsSignal{ExitReason: "final_answer"}), end(models.OtelLabelsSignal{ExitReason: "max_steps"})), true}, + {"on the error path", otelInv(end(models.OtelLabelsSignal{ExitReason: "final_answer"}), end(models.OtelLabelsSignal{ExitReason: "error", ErrorPath: true})), false}, + {"literal error anywhere", otelInv(end(models.OtelLabelsSignal{ExitReason: "error"})), false}, + {"variable reason", otelInv(end(models.OtelLabelsSignal{})), false}, + {"error path only in a test file", otelInv(end(models.OtelLabelsSignal{ExitReason: "final_answer"}), end(models.OtelLabelsSignal{ExitReason: "error", ErrorPath: true, TestOrigin: true})), true}, + } + for _, tc := range cases { + if got := rules.PredRepoOtelLabelsRunEndSuccessOnly(true, tc.inv); got != tc.want { + t.Errorf("%s: got %v, want %v", tc.name, got, tc.want) + } + } +} diff --git a/internal/rules/rule_detector.go b/internal/rules/rule_detector.go index b22dc021..bef9208a 100644 --- a/internal/rules/rule_detector.go +++ b/internal/rules/rule_detector.go @@ -150,9 +150,47 @@ func (d repoRuleDetector) Detect(p models.RepoProfile, inv models.RepoInventory) if !d.rule.Match.EvaluateRepo(p, inv) { return nil } + if s := otelLabelsEvidence(d.rule.Match, inv); s != nil { + return []models.Finding{findingFromRule(d.rule, models.ScopeRepo, s.File, s.StartLine, s.EndLine, "")} + } return []models.Finding{findingFromRule(d.rule, models.ScopeRepo, "", 0, 0, "")} } +// otelLabelsEvidence returns the offending call for a rule that asserts one of +// the agent-reliability-otel-labels correctness predicates (set to true), so +// the finding names the file and line instead of "somewhere in the repo" (spec +// D3). Test-file signals are never returned. A predicate under `not` is not +// evidence and is not searched. +func otelLabelsEvidence(e MatchExpr, inv models.RepoInventory) *models.OtelLabelsSignal { + lookups := []struct { + set *bool + offender func(models.RepoInventory) *models.OtelLabelsSignal + }{ + {e.RepoOtelLabelsRunShared, otelRunSharedOffender}, + {e.RepoOtelLabelsToolSpanFailureUnsafe, otelToolSpanFailureUnsafeOffender}, + {e.RepoOtelLabelsAttemptConstant, otelAttemptConstantOffender}, + {e.RepoOtelLabelsRunEndSuccessOnly, otelRunEndSuccessOnlyOffender}, + } + for _, l := range lookups { + if l.set != nil && *l.set { + if s := l.offender(inv); s != nil { + return s + } + } + } + for _, sub := range e.All { + if s := otelLabelsEvidence(sub, inv); s != nil { + return s + } + } + for _, sub := range e.Any { + if s := otelLabelsEvidence(sub, inv); s != nil { + return s + } + } + return nil +} + func agentKindMatches(kind string, a models.AgentDef) bool { switch kind { case "openai_agent": diff --git a/internal/rules/schema.go b/internal/rules/schema.go index c41702b0..9832a3f0 100644 --- a/internal/rules/schema.go +++ b/internal/rules/schema.go @@ -158,6 +158,13 @@ type MatchExpr struct { RepoOtelLabelsInitialized *bool `yaml:"repo_otel_labels_initialized,omitempty"` RepoOtelLabelsImplemented *bool `yaml:"repo_otel_labels_implemented,omitempty"` + // agent-reliability-otel-labels correctness predicates (OBS-009..012). + // Test-file signals never count (spec D8). + RepoOtelLabelsRunShared *bool `yaml:"repo_otel_labels_run_shared,omitempty"` + RepoOtelLabelsToolSpanFailureUnsafe *bool `yaml:"repo_otel_labels_tool_span_failure_unsafe,omitempty"` + RepoOtelLabelsAttemptConstant *bool `yaml:"repo_otel_labels_attempt_constant,omitempty"` + RepoOtelLabelsRunEndSuccessOnly *bool `yaml:"repo_otel_labels_run_end_success_only,omitempty"` + // RepoLangGraphPlatformConfigPresent is ALSO valid at agent scope (dual-scope, // like repo_has_observability): a langgraph.json means LangGraph Platform // injects its own checkpointer. diff --git a/internal/rules/schema.yaml b/internal/rules/schema.yaml index b773e32e..24091d8a 100644 --- a/internal/rules/schema.yaml +++ b/internal/rules/schema.yaml @@ -667,6 +667,28 @@ rules: # repo needs only the run end. Proves the calls exist, not that the # labels land: with no current span each call is a silent no-op. # trustabl.* keys written by hand with set_attribute(s) count too. +# repo_otel_labels_run_shared: true | false +# Fires when a Run of agent-reliability-otel-labels is (or is not) shared +# between runs: built at module scope, in a class body, or as a Python +# default argument, or given a literal run id. A Python construct under +# `if __name__ == "__main__":` is a one-run script and does not count. +# Run is matched through each file's import. Test files never count. +# repo_otel_labels_tool_span_failure_unsafe: true | false +# Fires when tool spans are (or are not) labelled only on success: +# mark_tool_span / markToolSpan calls exist and none sits in a finally, +# except or catch block, a named error callback, or a function passed to +# .finally( / .catch(. Calls inside an OpenAI Agents RunHooks / AgentHooks +# on_tool_end method are left out (unknown). One failure-safe call +# credits the repo. Test files never count. +# repo_otel_labels_attempt_constant: true | false +# Fires when every tool-span label passes `attempt` as an integer +# literal. One counted attempt credits the repo. Test files never count. +# repo_otel_labels_run_end_success_only: true | false +# Fires when run-end labels exist and none can record a failure. A call +# credits the repo when it sits in an except / catch block, an error +# callback or a .catch( handler, passes the literal "error", or passes a +# variable reason. A fixed reason in finally does not credit it. Test +# files never count. # repo_raw_anthropic_tool_output_in_system: true | false # Fires when a function drives the bare `anthropic` client in a manual # tool-use loop (.messages.create(..., tools=...)) and passes a runtime- diff --git a/internal/rules/schema_version.go b/internal/rules/schema_version.go index f1c51484..c11d359c 100644 --- a/internal/rules/schema_version.go +++ b/internal/rules/schema_version.go @@ -27,4 +27,4 @@ package rules // that rule (safe) instead of mis-evaluating a predicate whose meaning changed. // That rename discipline is what keeps lenient loading safe without a hard // version gate. -const SupportedSchemaVersion = 22 +const SupportedSchemaVersion = 23 diff --git a/internal/sarif/render.go b/internal/sarif/render.go index 706fa0f4..4d709ee3 100644 --- a/internal/sarif/render.go +++ b/internal/sarif/render.go @@ -156,9 +156,11 @@ func resultFromFinding(f models.Finding, ruleIndex *int, useBase bool) Result { r.Locations = []Location{loc} } - // Kind: "informational" for META results and repo-scoped rule findings - // (rule findings without a file location). Default (fail) for regular - // located rule findings — emit empty so SARIF's default applies. + // Kind: "informational" for META results and rule findings without a file + // location (most repo-scoped findings; the otel-labels correctness rules + // OBS-009..012 carry the offending call's file and line, so they render as + // ordinary located results). Default (fail) for regular located rule + // findings — emit empty so SARIF's default applies. if isInformational(f) { r.Kind = "informational" } @@ -185,8 +187,10 @@ func resultFromFinding(f models.Finding, ruleIndex *int, useBase bool) Result { // isInformational returns true for findings that should carry SARIF // kind="informational". Covers all META results that reach this builder -// (META-001/004 don't reach it — they're notifications) plus repo-scoped rule -// findings (which findingFromRule emits with empty FilePath/Line). +// (META-001/004 don't reach it — they're notifications) plus any rule finding +// with no file location. Repo-scoped findings carry a location only when the +// rule's evidence lookup supplies one (the otel-labels correctness rules +// OBS-009..012); otherwise findingFromRule emits an empty FilePath/Line. func isInformational(f models.Finding) bool { if len(f.RuleID) >= 4 && f.RuleID[:4] == "META" { return true diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index 6b32daf7..49e33b6e 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -654,6 +654,7 @@ func Run(cfg Config) (models.ScanResult, error) { Dependencies: inventory.Dependencies, Observability: inventory.ObservabilitySignals, Entrypoints: inventory.Entrypoints, + OtelLabels: inventory.OtelLabelsSignals, Vulnerabilities: vulns, Secrets: secrets, SlashCommands: inventory.SlashCommands, diff --git a/internal/scanner/scanner_test.go b/internal/scanner/scanner_test.go index b87dd413..f7d75aa3 100644 --- a/internal/scanner/scanner_test.go +++ b/internal/scanner/scanner_test.go @@ -4,9 +4,11 @@ import ( "archive/zip" "bytes" "encoding/json" + "fmt" "io/fs" "os" "path/filepath" + "regexp" "runtime" "strings" "testing" @@ -1372,3 +1374,252 @@ agent = Agent(name="travel-booker") }) } } + +// The facts the OBS-006..012 rules read are published in the report, so a +// finding can be checked against them. Not folded into ScanID. +func TestScan_ReportsOtelLabels(t *testing.T) { + dir := t.TempDir() + src := `from agent_reliability_otel_labels import Run, mark_run_end + + +def run_agent(root): + run = Run() + run.mark_start(root) + mark_run_end(root, exit_reason="final_answer") +` + if err := os.WriteFile(filepath.Join(dir, "agent.py"), []byte(src), 0o644); err != nil { + t.Fatal(err) + } + res, err := scanner.Run(scanner.Config{Target: dir, RulesFS: rulesFixture(t)}) + if err != nil { + t.Fatalf("scanner.Run: %v", err) + } + kinds := map[models.OtelLabelsSignalKind]bool{} + for _, s := range res.OtelLabels { + kinds[s.Kind] = true + } + for _, k := range []models.OtelLabelsSignalKind{ + models.OtelLabelsImport, models.OtelLabelsRunConstruct, + models.OtelLabelsRunStart, models.OtelLabelsRunEnd, + } { + if !kinds[k] { + t.Errorf("report has no %q signal: %+v", k, res.OtelLabels) + } + } +} + +// otelCorrectnessSrc builds an instrumented, fully labelled agent and lets a +// case swap one part for a broken version. +func otelCorrectnessSrc(moduleRun, runDecl, attempt, tool, runEnd string) string { + return `from agents import Agent, function_tool +from opentelemetry import trace +from opentelemetry.sdk.trace import TracerProvider +from agent_reliability_otel_labels import Run, canonical_fp, mark_run_end, mark_tool_span + +trace.set_tracer_provider(TracerProvider()) +tracer = trace.get_tracer(__name__) +` + moduleRun + ` + +@function_tool +def search_flights(origin: str) -> str: + """Search flights from an airport.""" + return origin + + +agent = Agent(name="travel-booker", tools=[search_flights]) + + +def run_agent(args): +` + runDecl + ` + attempts = {} + with tracer.start_as_current_span("invoke_agent") as root: + run.mark_start(root) + try: + with tracer.start_as_current_span("execute_tool") as span: + key = canonical_fp(args) + attempts[key] = attempts.get(key, 0) + 1 +` + strings.ReplaceAll(tool, "ATTEMPT", attempt) + ` +` + runEnd +} + +const ( + otelGoodRun = " run = Run()" + otelGoodAttempt = "attempts[key]" + otelGoodTool = ` result = None + try: + result = search_flights(args) + finally: + mark_tool_span(span, args=args, result=result, attempt=ATTEMPT, + step_index=run.next_step(), run_id=run.run_id)` + otelUnsafeTool = ` result = search_flights(args) + mark_tool_span(span, args=args, result=result, attempt=ATTEMPT, + step_index=run.next_step(), run_id=run.run_id)` + otelGoodRunEnd = ` mark_run_end(root, exit_reason="final_answer") + except Exception: + mark_run_end(root, exit_reason="error") + raise +` + otelSuccessOnlyRunEnd = ` mark_run_end(root, exit_reason="final_answer") + finally: + pass +` +) + +func TestScan_OtelLabelsCorrectness(t *testing.T) { + cases := []struct { + name string + src string + testFile string + want []string + wantLine string // a line of src the single finding must point at + }{ + {"correct labelling", otelCorrectnessSrc("", otelGoodRun, otelGoodAttempt, otelGoodTool, otelGoodRunEnd), "", nil, ""}, + {"shared module-scope Run", otelCorrectnessSrc("RUN = Run()\n", " run = RUN", otelGoodAttempt, otelGoodTool, otelGoodRunEnd), "", []string{"OBS-009"}, "RUN = Run()"}, + {"label skipped on failure", otelCorrectnessSrc("", otelGoodRun, otelGoodAttempt, otelUnsafeTool, otelGoodRunEnd), "", []string{"OBS-010"}, "mark_tool_span(span"}, + {"constant attempt", otelCorrectnessSrc("", otelGoodRun, "1", otelGoodTool, otelGoodRunEnd), "", []string{"OBS-011"}, "mark_tool_span(span"}, + {"run end on success only", otelCorrectnessSrc("", otelGoodRun, otelGoodAttempt, otelGoodTool, otelSuccessOnlyRunEnd), "", []string{"OBS-012"}, `mark_run_end(root, exit_reason="final_answer")`}, + {"misuse only in a test file", otelCorrectnessSrc("", otelGoodRun, otelGoodAttempt, otelGoodTool, otelGoodRunEnd), + `from agent_reliability_otel_labels import Run, mark_tool_span + +RUN = Run() + + +def test_label(span): + mark_tool_span(span, args={}, result=None, attempt=1) +`, nil, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + manifest := "[project]\nname = \"f\"\ndependencies = [\"openai-agents\", \"opentelemetry-sdk\", \"agent-reliability-otel-labels\"]\n" + files := map[string]string{"pyproject.toml": manifest, "agent.py": tc.src} + if tc.testFile != "" { + files["tests/test_agent.py"] = tc.testFile + } + for name, body := range files { + p := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + res, err := scanner.Run(scanner.Config{Target: dir, RulesFS: rulesFixture(t)}) + if err != nil { + t.Fatalf("scanner.Run: %v", err) + } + var got []string + var hit models.Finding + for _, f := range res.Findings { + switch f.RuleID { + case "OBS-006", "OBS-007", "OBS-008", "OBS-009", "OBS-010", "OBS-011", "OBS-012": + got = append(got, f.RuleID) + hit = f + } + } + if strings.Join(got, ",") != strings.Join(tc.want, ",") { + t.Fatalf("otel-labels findings: got %v, want %v", got, tc.want) + } + if tc.wantLine == "" { + return + } + wantLine := 0 + for i, line := range strings.Split(tc.src, "\n") { + if strings.Contains(line, tc.wantLine) { + wantLine = i + 1 + break + } + } + if hit.FilePath != "agent.py" || hit.StartLine != wantLine { + t.Errorf("finding at %s:%d, want agent.py:%d", hit.FilePath, hit.StartLine, wantLine) + } + }) + } +} + +// Forge tells a coding model how to write the labels and scan judges the +// result. The code in forge's label blocks must not trip the rules that judge +// it. Go blocks are out of scope (the Go binding is not inspected). +func TestScan_ForgeLabelBlocksPassOtelLabelsRules(t *testing.T) { + fence := regexp.MustCompile("(?s)```(python|typescript)\n(.*?)```") + paths, _ := filepath.Glob("../forge/tracingtext/*_labels.md") + recipes, _ := filepath.Glob("../forge/tracingtext/recipes/*_labels.md") + blocks := map[string][]string{} + for _, p := range append(paths, recipes...) { + raw, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + for _, m := range fence.FindAllStringSubmatch(string(raw), -1) { + blocks[m[1]] = append(blocks[m[1]], m[2]) + } + } + setups := []struct { + lang, ext, manifestName, manifest, setup string + }{ + {"python", ".py", "pyproject.toml", + "[project]\nname = \"f\"\ndependencies = [\"openai-agents\", \"opentelemetry-sdk\", \"agent-reliability-otel-labels\"]\n", + `from agents import Agent +from opentelemetry import trace +from opentelemetry.sdk.trace import TracerProvider + +trace.set_tracer_provider(TracerProvider()) +tracer = trace.get_tracer(__name__) +agent = Agent(name="travel-booker") +`}, + {"typescript", ".ts", "package.json", + `{"name": "f", "dependencies": {"@openai/agents": "*", "@opentelemetry/sdk-node": "*", "@trustabl/agent-reliability-otel-labels": "*"}}`, + `import { NodeSDK } from "@opentelemetry/sdk-node"; +import { Agent } from "@openai/agents"; + +const sdk = new NodeSDK({}); +sdk.start(); +export const agent = new Agent({ name: "travel-booker" }); +`}, + } + for _, s := range setups { + if len(blocks[s.lang]) == 0 { + t.Fatalf("no %s label blocks found under internal/forge/tracingtext", s.lang) + } + // One repo per block: scanning them together would let one correct + // block mask a bad one (a run_end here crediting a missing one there). + for i, b := range blocks[s.lang] { + t.Run(fmt.Sprintf("%s/%d", s.lang, i), func(t *testing.T) { + dir := t.TempDir() + write := func(name, body string) { + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + write(s.manifestName, s.manifest) + write("tracing_setup"+s.ext, s.setup) + write("labels"+s.ext, b) + res, err := scanner.Run(scanner.Config{Target: dir, RulesFS: rulesFixture(t)}) + if err != nil { + t.Fatalf("scanner.Run: %v", err) + } + // Anchors, so the check cannot pass vacuously: tracing is + // initialized and the block was read as labelling code. + var initialized bool + for _, o := range res.Observability { + if o.Kind == models.ObsSignalInit { + initialized = true + } + } + kinds := map[models.OtelLabelsSignalKind]bool{} + for _, l := range res.OtelLabels { + kinds[l.Kind] = true + } + if !initialized || !kinds[models.OtelLabelsToolSpan] || !kinds[models.OtelLabelsRunEnd] { + t.Fatalf("anchors missing: initialized=%v kinds=%v\n%s", initialized, kinds, b) + } + for _, f := range res.Findings { + if strings.HasPrefix(f.RuleID, "OBS-0") && f.RuleID >= "OBS-006" && f.RuleID <= "OBS-012" { + t.Errorf("forge's own %s label block %d trips %s at %s:%d\n%s", s.lang, i, f.RuleID, f.FilePath, f.StartLine, b) + } + } + }) + } + } +} diff --git a/testdata/rules-fixture/manifest.yaml b/testdata/rules-fixture/manifest.yaml index 36205528..f203f22f 100644 --- a/testdata/rules-fixture/manifest.yaml +++ b/testdata/rules-fixture/manifest.yaml @@ -11,4 +11,4 @@ # # This file is metadata, not a rule: the engine's loader skips manifest.yaml # when walking the pack for policy files. -schema_version: 22 +schema_version: 23 diff --git a/testdata/rules-fixture/observability/quality.yaml b/testdata/rules-fixture/observability/quality.yaml index fad5d7d1..3c5fe6a9 100644 --- a/testdata/rules-fixture/observability/quality.yaml +++ b/testdata/rules-fixture/observability/quality.yaml @@ -269,3 +269,147 @@ rules: "error" on the exception path. Verify by exporting one run and checking for trustabl.tool.input_fp on each tool span and trustabl.exit_reason on the root span. + + # ─── agent-reliability-otel-labels label correctness (OBS-009..012) ──────── + # OBS-006..008 prove the labels are written; these prove they are right. + # Each is gated where its labels must exist, and a single correct call + # credits the repo (spec D6), so partial misuse is accepted as a miss. Test + # files never decide one (D8). Findings carry the offending call's file and + # line. + - id: OBS-009 + title: One agent-reliability-otel-labels Run is shared by every agent run + severity: medium + confidence: 0.6 + applies_to: + - claude_sdk + - openai_agents + - langchain + - pydantic_ai + - vercel_ai + - crewai + - google_adk + - autogen + - mcp + scope: repo + match: + all: + - repo_observability_initialized: true + - repo_otel_labels_initialized: true + - repo_otel_labels_run_shared: true + explanation: > + A Run is built once instead of once per agent run: at module scope, in a + class body, or as a default argument, or it is given a fixed run id. + Every run then writes the same trustabl.run_id, and the step counter keeps + counting across runs. Two users' separate requests read as one run, and + two identical questions read as a loop, so any check that groups tool + calls by run draws the wrong conclusion, and nothing in the trace shows + the labels are wrong. + fix: > + Create the Run inside the function that handles one agent run, with run = + Run() (const run = new Run() in TypeScript) as its first line, and pass it + to the code that labels spans. Do not keep a Run at module scope, as a + class attribute, or as a default argument, and leave run_id unset or pass + a per-request id. Verify by exporting two runs and checking that their + root spans carry different trustabl.run_id values. + + - id: OBS-010 + title: Tool spans are labelled only when the tool succeeds + severity: low + confidence: 0.55 + applies_to: + - claude_sdk + - openai_agents + - langchain + - pydantic_ai + - vercel_ai + - crewai + - google_adk + - autogen + - mcp + scope: repo + match: + all: + - repo_observability_initialized: true + - repo_otel_labels_implemented: true + - repo_otel_labels_tool_span_failure_unsafe: true + explanation: > + Every mark_tool_span call runs after the tool returns, outside any + finally or except block, so a tool call that raises is never labelled. + Failed calls are the ones a retry storm is made of: a tool that fails and + is retried ten times leaves one labelled span, without the fingerprints + and attempt counts that would show the repetition. + fix: > + Set result = None before calling the tool, call it inside try, and call + mark_tool_span in the finally block (in TypeScript, let result; try { ... } + finally { markToolSpan(...) }, or .finally(() => markToolSpan(...)) on the + promise). Pass error_class when the failure type is known. Verify by + forcing one tool call to fail and checking that its span carries + trustabl.tool.input_fp. + + - id: OBS-011 + title: The tool-call attempt count is a constant + severity: low + confidence: 0.7 + applies_to: + - claude_sdk + - openai_agents + - langchain + - pydantic_ai + - vercel_ai + - crewai + - google_adk + - autogen + - mcp + scope: repo + match: + all: + - repo_observability_initialized: true + - repo_otel_labels_implemented: true + - repo_otel_labels_attempt_constant: true + explanation: > + Every mark_tool_span call passes attempt as a fixed number, usually 1, so + every call reports as a first attempt. trustabl.tool.attempt exists to + show retries; with a constant it never can, and an agent that calls the + same tool with the same arguments again and again looks like a series of + fresh calls. + fix: > + Count attempts per run: keep a dict (a Map in TypeScript) created next to + the Run, keyed by canonical_fp(args) (canonicalFp in TypeScript), + increment it before each call, and pass the count as attempt. Verify by + calling one tool twice with the same arguments in one run and checking + that the second span carries trustabl.tool.attempt = 2. + + - id: OBS-012 + title: The run's end is recorded only when the run succeeds + severity: low + confidence: 0.55 + applies_to: + - claude_sdk + - openai_agents + - langchain + - pydantic_ai + - vercel_ai + - crewai + - google_adk + - autogen + - mcp + scope: repo + match: + all: + - repo_observability_initialized: true + - repo_otel_labels_implemented: true + - repo_otel_labels_run_end_success_only: true + explanation: > + Every mark_run_end call records a fixed success reason such as + "final_answer", on the success path or in a finally block, and none + records "error". A run that raises then ends with no trustabl.exit_reason, + or is recorded as finished. Separating failed runs from completed ones is + what the exit reason is for, so every check on how runs end reports + success. + fix: > + Call mark_run_end(root, exit_reason="final_answer") at the end of the try + block and mark_run_end(root, exit_reason="error") in the except block + before re-raising (markRunEnd(root, "error") in catch in TypeScript). Use + "max_steps" or "policy_stop" when those stopped the run, and never record a + fixed success reason in finally. Verify by forcing one run to fail and + checking that its root span carries trustabl.exit_reason = error.