Skip to content

Add OBS-009..012 for agent-reliability-otel-labels label correctness - #261

Merged
ericksondelacruz merged 2 commits into
mainfrom
feat/otel-labels-correctness
Oct 25, 2026
Merged

ericksondelacruz merged 2 commits into
mainfrom
feat/otel-labels-correctness

Conversation

@ericksondelacruz

@ericksondelacruz ericksondelacruz commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

OBS-006..008 check that the agent-reliability-otel-labels labels are written at all. This PR adds OBS-009..012, four repo-scope rules that check the labels are correct:

Rule Fires when Severity Confidence
OBS-009 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 medium 0.6
OBS-010 Tool spans are labelled only on success: no mark_tool_span in a finally / except / catch block, an error callback, or a .finally / .catch handler low 0.55
OBS-011 Every tool-span label passes attempt as a constant low 0.7
OBS-012 No run-end label can record a failure low 0.55

Changes

  • Discovery (internal/analysis/otel_labels_facts.go). OtelLabelsSignal gains the run_construct kind and the facts the rules read: ModuleScope, RunIDConst, FailureSafe, HookUnknown, ErrorPath, AttemptConst, SideEffect and ExitReason. Three cases are deliberately not flagged:
    • A Python if __name__ == "__main__": construct is a one-run script.
    • A non-static TS/JS class field runs once per instance.
    • Labels inside an OpenAI Agents RunHooks / AgentHooks on_tool_end are treated as unknown, not unsafe.
  • Rules. Four new predicates. Test files never count, and one correct call credits the whole repo.
  • Finding location. Findings point at the offending call's file and line, so SARIF renders them as located results.
  • Report. The scan report gains otel_labels. It is not folded into ScanID.
  • Schema. schema_version goes from 26 to 27, because main already took 22 for LC-116 and 23–26 for the agent_config pack.
  • Docs. ARCHITECTURE, COVERAGE, README, CLAUDE.md and schema.yaml are updated. The fixture holds 347 rules.

Testing

  • go vet ./... and go test -race ./... pass.
  • scripts/check-rules-sync.sh against the agent-reliability-rules branch feat/otel-labels-correctness: in sync, 133 files (re-run 2026-10-26 after merging main).
  • The 0.1.1 README examples from agent-reliability-otel-labels were scanned with this build: all facts are correct and no OBS-009..012 fires. The 0.1.0 examples fire all four, each at the offending line.
  • On 10 real agent repos, findings were identical before and after (checked 2026-10-08, before the rebase onto main).

Merge order

  • Do not merge until agent-reliability-otel-labels 0.1.1 is published. The 0.1.0 README example trips OBS-009..012. The release PR is "Release 0.1.1: README examples that label failed tool calls and failed runs" in agent-reliability-otel-labels.
  • Merge together with the feat/otel-labels-correctness PRs in agent-reliability-rules and trustabl-rulebook. The rulebook PR is "Rationale for OBS-009..012 (otel-labels label correctness) and LC-116".
  • CI's rules-sync job uses the rules branch with the same name when it exists.

Known limits

Recorded in the design spec and not addressed in this PR:

  • OBS-009 fires on a module-level Run() in examples/ or scripts/.
  • A few failure-safe shapes are not recognised: Python __exit__, and TS .then(ok, onRejected).
  • self.run = Run() in a long-lived object is not caught.
  • A conditional exit reason whose branches are all literals counts as variable.

… 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.

@sairenchristianbuerano sairenchristianbuerano left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Resolve merge conflict

@sairenchristianbuerano

Copy link
Copy Markdown
Collaborator

@ericksondelacruz heads-up on the rule schema number before this merges.

main is now at schema 25 in both the analyzer and the rules pack: AGCFG-008 (Codex hooks) took 24 and AGCFG-009 (AGENTS.md hidden text) took 25. This PR still sets 23 (it was 22), and the companion rules PR #184 sets 23 too. Merged as is, the engine constant would go backwards while the pack says 25, and an older engine would skip the OTel label rules without telling anyone.

Before merging:

  1. Merge or rebase main. Expect conflicts in internal/rules/schema_version.go, testdata/rules-fixture/manifest.yaml, schema.go, evaluator.go, schema.yaml and policies_test.go (AGCFG-008 and 009 touched the same places).
  2. Set the schema to the next free number, 26 (or 27 if the CI rules pair in feat(analysis): add github_workflow component and CI-001..005 predicates (rule schema 28, provisional) #264 merges first). Check main right before you set it. Use the same number in internal/rules/schema_version.go, the analyzer's testdata/rules-fixture/manifest.yaml, and manifest.yaml in docs(skill): list every --detectors category the CLI accepts #184.
  3. Run go test ./... and check the rule files are still identical in the fixture and the rules pack.
  4. Merge the analyzer first, then docs(skill): list every --detectors category the CLI accepts #184 and the rulebook PR right after.

Resolve conflicts with the agent_config work on main:

- schema_version: main took 23-26 for the agent_config pack, so
  OBS-009..012's four predicates move to 27 (SupportedSchemaVersion and
  the fixture manifest).
- rule_detector.go: repo findings try otel-labels evidence first, then
  fall back to main's configHitLocation for agent_config rules.
- COVERAGE.md / CLAUDE.md: keep main's review entries, restate this
  batch as 26 -> 27 with re-derived counts (347 = 343 + 4).
@ericksondelacruz

Copy link
Copy Markdown
Collaborator Author

@ericksondelacruz heads-up on the rule schema number before this merges.

main is now at schema 25 in both the analyzer and the rules pack: AGCFG-008 (Codex hooks) took 24 and AGCFG-009 (AGENTS.md hidden text) took 25. This PR still sets 23 (it was 22), and the companion rules PR #184 sets 23 too. Merged as is, the engine constant would go backwards while the pack says 25, and an older engine would skip the OTel label rules without telling anyone.

Before merging:

  1. Merge or rebase main. Expect conflicts in internal/rules/schema_version.go, testdata/rules-fixture/manifest.yaml, schema.go, evaluator.go, schema.yaml and policies_test.go (AGCFG-008 and 009 touched the same places).
  2. Set the schema to the next free number, 26 (or 27 if the CI rules pair in feat(analysis): add github_workflow component and CI-001..005 predicates (rule schema 30, provisional) #264 merges first). Check main right before you set it. Use the same number in internal/rules/schema_version.go, the analyzer's testdata/rules-fixture/manifest.yaml, and manifest.yaml in docs(skill): list every --detectors category the CLI accepts #184.
  3. Run go test ./... and check the rule files are still identical in the fixture and the rules pack.
  4. Merge the analyzer first, then docs(skill): list every --detectors category the CLI accepts #184 and the rulebook PR right after.

This is now handled. Thank you!

@ericksondelacruz
ericksondelacruz merged commit 26b6152 into main Oct 25, 2026
5 checks passed
ericksondelacruz added a commit that referenced this pull request Oct 25, 2026
Main took schema 27 for OBS-009..012 (#261). The 26 -> 27 bump here
merged cleanly on top of it, so renumber by hand: SupportedSchemaVersion
and the fixture manifest go to 28. COVERAGE.md keeps both review entries;
CHANGELOG, CLAUDE.md, ARCHITECTURE and COVERAGE restate the batch as
27 -> 28 with re-derived counts (356 = 347 + 9).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants