Skip to content

[GA-held DRAFT, do not merge] Investigation Engine CLI and SDK (limacharlie investigate) - #455

Draft
maximelb wants to merge 14 commits into
masterfrom
feature/ie-cli
Draft

maximelb wants to merge 14 commits into
masterfrom
feature/ie-cli

Conversation

@maximelb

Copy link
Copy Markdown
Contributor

GA-held draft: do not merge, do not release. The Investigation Engine is experimental and off by default in production; this PR is kept as a draft until it ships.

What

A new limacharlie investigate command group and an Investigation SDK class, so a person or an AI agent (Claude Code, Codex, ...) pointed at the CLI can build, test and tune investigation templates and run investigations.

investigate kit [--section ...]            authoring kit in parts (schema, guide, operators, examples, policy, codes, packs, library)
investigate operators | packs              typed catalogues (--brief index)
investigate templates list | get | validate | test | save | promote | delete | install-defaults
investigate runs start | get | list | events [--follow] | steps | case | cancel
investigate wave --file wave.yaml          waves with `use:` packs, stop_when, select, limit, --full, --validate-only
investigate actions list | approve | deny  approvals inbox
investigate policy get | status | pause | resume

Agent workflow (documented in doc/cli/investigation-engine.md, limacharlie help investigation-engine and --ai-help): kit -> draft -> validate -> test on past alerts -> refine -> save as a draft -> a person runs promote.

Safety properties

  • templates save always writes a draft. A source that says status: active is refused; only an explicit --status active (a person's call) writes an active template. Nothing is written when validation has errors.
  • A record is never overwritten without --overwrite; a draft never replaces an active template; creates are conditional on the record not existing.
  • promote and delete require --confirm; promote re-validates and writes conditionally on the etag it read.
  • install-defaults never overwrites silently: existing records are skipped with installed and library versions. Library templates are installed as drafts unless --status active is passed.
  • Writes (start, wave, test, decide, cancel) are sent once and never replayed after a gateway timeout.
  • The JWT is never printed or logged by the new code.

Output and errors

JSON by default when piped; the usual --output formats otherwise, with readable text for templates test (per-sample verdicts and the agreement summary) and listings. Engine failures keep the engine's error_code and coded errors / warnings (code, path, message, suggestion) verbatim, written to stderr as a document in machine formats. A refused wave prints its coded errors on stdout and exits 4. A 404 without an engine error code is reported as "feature not enabled for this deployment", a 403 names the permissions needed.

Small change outside the new files: the typed 401/403/404/422/429 errors raised for an HTTP error answer now also carry the parsed answer as response_body (as ApiError already did), which the new error mapping needs.

Depends on

  • The default template library (kit --section library, used by templates install-defaults) is served by an engine version that is not released yet. Without it the command stops with a clear message; every other command works against the current engine API.
  • The routes only exist where the Investigation Engine is enabled for the API deployment.

Tests

pytest tests/unit/ tests/microbenchmarks/ passes. New: tests/unit/test_sdk_investigation.py (routes and parameters, id and range validation, follow termination, error mapping, YAML 1.2 text handling, save/promote/install guarantees) and tests/unit/test_cli_investigate.py (command wiring, sample building, exit codes, output and error rendering). All HTTP is mocked; no integration test was run against a live or local stack.

🤖 Generated with Claude Code

maximelb and others added 2 commits November 10, 2026 13:11
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

No high or medium issues found. There are 4 low-priority findings, posted inline.

General Feedback

  • Create-only etag is not tested against a real server. save_template creates new records with the sentinel etag create-only: the record must not exist. No other code in this repo uses a sentinel etag to create a record. The unit tests mock Hive, and the PR body says no live test was run. The code relies on the hive accepting a write that carries an etag for a record that does not exist yet. If the hive rejects it, every templates save of a new name fails, and so does every install-defaults entry. Please check this against a real hive before the PR leaves draft.
  • No retry guard on hive writes. hive.set in save_template/promote_template uses the client's default retry_gateway_timeout=True. If a 504 arrives after the write was applied, the retry carries a stale or create-only etag and fails. The record is then saved, but the command reports an error.
  • test-time-budget: No test-time-budget check run was found for this commit. The base branch master does not require the check: branch protection and rulesets list no required checks. This PR does not touch any workflow.

Blast Radius

  • errors.error_from_status_code now sets response_body on the 401/403/404/422/429 typed errors, and Client.request sets it on RateLimitError. The attribute is new on those classes. They do not become ApiError subclasses, so isinstance(..., ApiError) checks, _lc_error_code and is_record_not_found behave as before. Code that called hasattr(e, 'response_body') to detect an ApiError would now also match these errors. No such caller exists in this repo.
  • New top-level investigate CLI group, lazy-loaded via _COMMAND_MODULE_MAP. There is a new investigation_engine discovery profile and a new help topic. These add to the command surface without changing existing commands.
  • The new SDK class limacharlie.sdk.investigation.Investigation writes to the investigation_plan hive through the existing Hive class.

This review was generated automatically by Claude Code.

Comment thread limacharlie/sdk/investigation.py Resolved
Comment thread limacharlie/sdk/investigation.py Resolved
Comment thread limacharlie/commands/investigate.py Resolved
Comment thread limacharlie/commands/investigate.py Resolved
YAML 1.2 number handling for template sources, iter_actions cursor guard,
JSON-lines stream for runs start --follow, landed-write guard for template
saves, hive permission hint for template records.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

Self-review (adversarial pass over the full diff)

Checked each CLI call against the routes the API gateway actually serves (method, path, query and body names, permissions) and against the engine's documented request shapes; only routed endpoints are called.

Route and parameter correctness

  • Every SDK method maps to a served route: authoring-kit?sections=, operators, packs?name=, templates, plans/validate (body yaml), test-plan (POST, body is the request object) and test-plan/{id}?after&wait, runs (POST/GET), runs/{id}, /steps?values, /case, /events?after&wait&limit, /cancel (body revert_leases), waves, actions?state&class&run_id&limit&cursor, runs/{id}/actions/{aid}/{verb} (body note, seconds), policy/effective?category&priority&severity&target_tags&class, autonomy, autonomy/{pause|resume}.
  • Not sent: template on the effective policy (the gateway does not forward it), and no route that does not exist (no stats/compare/trace/labels commands).
  • Ids are validated with the gateway's own rule (letters, digits, . _ : -, 1..128, not ./..) and percent-escaped; long-poll wait 0..10 and limit bounds are checked before the call.

Draft-by-default

  • status defaults to draft; a source with status: active is refused unless a person passes --status active; the stored record is read back and its status compared with what was requested.
  • No silent overwrite: existing record needs --overwrite; an active record is never replaced by a draft; unknown statuses are left alone; creates send an etag that matches no stored record (the hive ignores the etag when no record exists and refuses the write when one does). A replayed write that already landed is detected by reading the record back.
  • promote/delete need --confirm; promote re-validates the exact stored text and writes conditionally on the etag it read.
  • install-defaults writes drafts (even for library entries written as active), skips existing records with installed/library versions, never overwrites without --overwrite.
  • Template text keeps its meaning: YAML 1.1 booleans, timestamps and 012/1_000/10:30 stay text; anchors/aliases are refused; the validated text is the stored text.

Errors, secrets, paging, termination

  • Engine error_code, errors and warnings are kept verbatim; 404 without an engine code is reported as "feature not enabled", 403 names the permissions, hive permission failures name investigation_plan.*. In machine formats the error is a JSON document on stderr; a refused wave prints its coded errors on stdout and exits 4.
  • The JWT is never read or printed by the new code (the engine receives it from the gateway; the CLI only authenticates the HTTP request through the existing client).
  • Writes are never replayed after a gateway timeout; reads may be.
  • Listings page with the engine's opaque cursor; --all walks cursors and stops with an error if a cursor does not advance (runs and actions).
  • events --follow and runs start --follow end when the engine says done (exit 0), at --timeout (exit 1, resume point on stderr) or on Ctrl-C (130); in structured formats the stream is JSON lines.

Known limits (not bugs)

  • install-defaults needs an engine that serves the library kit section; until then it stops with a clear message.
  • Nothing was run against a live or local stack; HTTP is mocked, plus a one-off check of the real client against a stubbed urlopen (query encoding, JSON body and content type, error bodies).

@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of cad3f10. The four earlier findings are fixed. I found no high or medium issues. One new low-priority note is inline.

General Feedback

  • Fixes I checked:
    • The YAML 1.2 int/float resolvers and the int constructor are correct for 012, 1_000, 10:30, 0x1f, 0o17, 1e3 and .inf.
    • iter_actions has the same cursor-advance guard as iter_runs, and actions list --all now uses it.
    • runs start --follow in a structured format now writes JSON lines.
    • A landed-write guard is added in save_template.
  • Create-only etag: the author says the hive accepts any etag when no record exists. A live check is still pending before the PR leaves draft. I am noting it, not blocking on it.
  • I could not run the tests in this environment because there is no Python interpreter. The new tests call the real SDK and CLI code, with the HTTP client and Hive mocked. That is mocking, not simulation.
  • test-time-budget: no run was found for this commit. master requires no status checks, either in branch protection or in rulesets.

Blast Radius

  • errors.error_from_status_code and Client.request now set response_body on the typed 401/403/404/422/429 errors. These errors do not change class, so existing isinstance checks and the hive helpers (is_record_not_found, is_permission_denied) behave as before.
  • The rest of the PR is additive:
    • the investigate CLI group, which is lazy-loaded;
    • the limacharlie.sdk.investigation module;
    • a discovery profile and a help topic;
    • writes to the investigation_plan hive through the existing Hive class.

This review was generated automatically by Claude Code.

Comment thread limacharlie/sdk/investigation.py Resolved
…d tags

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of 8fc2401. This commit changes only the landed-write guard in save_template. I found no high or medium issues and no new findings.

General Feedback

  • The guard now marks a failed write as saved only if three things match what was sent: the stored data, the comment and the tags. So when hive.set fails on an overwrite, a lost --comment or --tag is now reported as an error. The new test, test_a_failed_overwrite_does_not_pass_as_saved_when_the_comment_was_lost, calls the real save_template with Hive mocked. That is mocking, not simulation.
  • Still open from earlier rounds, as notes only (not blocking):
    • The create-only etag has not been tried against a live hive.
    • hive.set in promote_template still uses the client's default gateway-timeout retry.
  • test-time-budget: I found no run for this commit. master requires no status checks, in either branch protection or rulesets.

Blast Radius

  • This commit only changes limacharlie/sdk/investigation.py (save_template error path) and its unit test. It does not touch existing code.
  • The PR as a whole is unchanged from the last review:
    • response_body is now set on the typed 401/403/404/422/429 errors. Their classes do not change.
    • The other changes are additive: the lazy-loaded investigate CLI group, the limacharlie.sdk.investigation module, a discovery profile, a help topic, and writes to the investigation_plan hive.

This review was generated automatically by Claude Code.

… every failure, run mode docs, example check

- wave and --samples-file are read with YAML 1.2 scalar rules (off/yes stay text, dates stay strings)
- unreadable or malformed input files and every Hive/API failure render as structured errors
- help and docs: the run mode default is full; triage is cheaper
- docs wave example selects the real ioc.prevalence field (seen); a test checks every documented wave against a recorded operator catalogue

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of 4c42051. This commit changes four things:

  • Wave and --samples-file inputs are now parsed with YAML 1.2 scalar rules (parse_document).
  • Every SDK failure now prints as a structured error in machine output formats.
  • Template-record hive failures are wrapped as InvestigationError.
  • The run mode text is corrected, and a test checks the documented wave examples against a recorded catalogue.

I found no high or medium issues and no new inline findings.

General Feedback

  • What I checked in this commit:
    • _template_record_errors now wraps every ApiError and keeps response_body, so error_code still carries the hive's lc_error_code. Non-ApiError errors that are not permission refusals (ValidationError, NotFoundError, ...) still pass through unchanged. install_default_templates catches LimaCharlieError, so wrapping does not change what it does.
    • save_template handles a missing record and checks whether a failed write landed. Both happen inside the wrapped function, so they see the raw hive errors as before.
    • _engine_errors now catches every LimaCharlieError. Each error keeps its own exit_code: 4 for refused inputs, 2 for auth, 1 for API errors. In table mode the error is still re-raised for the CLI's normal text handling.
    • parse_document turns a value JSON cannot carry into a ValidationError. A recursive alias falls in that case, because json.dumps raises ValueError.
    • Anchors and aliases are allowed in wave and sample files. A file built to expand hugely through aliases could use a lot of memory in json.dumps. The file is the user's own local input, so this is a note only.
  • New tests call the real parse_document, save_template, promote_template and CLI commands, with Hive and the SDK mocked. That is mocking, not simulation. test_investigation_examples.py reads a checked-in fixture, with no network and no timing, so it should not be flaky.
  • Small test nit: in test_a_coded_hive_refusal_keeps_its_lc_error_code, the first hive.get.side_effect = ... assignment is overwritten on the next line and has no effect.
  • Still open from earlier rounds (notes only, not blocking):
    • The create-only etag has not been tried against a live hive.
    • hive.set in promote_template still uses the client's default gateway-timeout retry. A replayed promote with a stale etag would report a failure even though the write landed.
  • test-time-budget: I found no run for this commit. master requires no status checks in either branch protection or rulesets. This PR does not touch any workflow.

Blast Radius

  • This commit changes only the new investigate command module, limacharlie/sdk/investigation.py, docs and tests. It adds a public helper, limacharlie.sdk.investigation.parse_document.
  • Behaviour change for SDK users: get_template, save_template, promote_template and delete_template now raise InvestigationError for hive ApiError failures. InvestigationError is still an ApiError subclass, so existing except ApiError handlers still catch it.
  • In the CLI, input errors on investigate commands now exit with 4 instead of click's usage-error code 2.
  • The rest of the PR is unchanged from the last review:
    • response_body is set on the typed 401/403/404/422/429 errors.
    • The investigate group is lazy-loaded.
    • Templates are written to the investigation_plan hive.

This review was generated automatically by Claude Code.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

Self-review of the fixes (4c42051 + test nit): (1) wave/sample files now go through the same YAML 1.2 loader as templates, so off/yes/dates reach the engine as written; unreadable or malformed input is a coded invalid_input error (exit 4) and JSON when asked. (2) Help/docs say the run default is full (templates test stays triage, which matches the engine); no behaviour change. (3) The CLI wrapper catches every SDK failure and keeps error_code, status, suggestion and the error's own exit code; the SDK wraps hive API errors as InvestigationError without touching input errors. (4) Doc example uses seen; a test checks every documented wave against a recorded operator/pack catalogue fixture, no network. Full unit + microbenchmark suite passes (5 pre-existing platform skips). PR stays draft; not to be merged until GA.

@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of 5a4302f. Since 4c42051, the only change is in tests/unit/test_sdk_investigation.py: the dead hive.get.side_effect assignment in test_a_coded_hive_refusal_keeps_its_lc_error_code is removed, as noted in the last review. The test behaves the same: hive.get returns a draft record and hive.set raises the coded 400. I found no high or medium issues and no new findings.

General Feedback

  • I checked save_template, promote_template and install_default_templates again against Hive.set. When no tags or comment are given, Hive.set sends no usr_mtd block. So a plain overwrite or promote does not reset enabled or drop the stored metadata.
  • Still open from earlier rounds. These are notes only and do not block:
    • The create-only etag has not been tried against a live hive. The author says this check is on the list before the PR leaves draft.
    • hive.set in promote_template still uses the client's default retry on a gateway timeout. If that retry replays a promote that already landed, the stale etag makes it fail. The command then reports an error even though the template is active.
  • test-time-budget: no run was found for this commit. master requires no status checks in branch protection or in rulesets. This PR does not touch any workflow.

Blast Radius

  • This commit changes one unit test and nothing in the library.
  • The PR as a whole has not changed since the last review:
    • The typed 401/403/404/422/429 errors now carry response_body. Their classes do not change.
    • The lazy-loaded investigate CLI group and the limacharlie.sdk.investigation module are new, along with parse_document.
    • Template-record failures are wrapped as InvestigationError, which is still a subclass of ApiError.
    • Writes go to the investigation_plan hive through the existing Hive class.

This review was generated automatically by Claude Code.

- runs start --urgency is low|medium|high|critical (default derived from the alert, else medium)
- --auto sends no plan (the engine refuses plan.auto for a person's run) so the engine picks the active template by applies_to; new --category/--severity/--priority/--tag carry the alert attributes it matches on
- templates test --strict help: refuses overrides that would loosen the policy
- runs list --trigger takes the trigger kind, --verdict also takes undecided, --started-by also matches a name
- policy get --template; --case-mode and --wait help corrected

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of 5e20b76. This commit changes the investigate flags and help text to match the engine:

  • runs start --auto now sends no plan.
  • runs start has new --category, --severity, --priority and --tag flags that go into trigger.
  • --urgency is now low|medium|high|critical.
  • --strict has a new meaning.
  • runs list has new help text.
  • policy get --template is new.

I found no high or medium issues. There is one low-priority note, inline.

General Feedback

  • The new tests call the real CLI commands and SDK methods, with the SDK or the HTTP client mocked. That is mocking, not simulation. The help-text tests check strings in click's --help output, which is deterministic, so they are not flaky.
  • I could not check the engine-side claims in this commit against code in this repo. These are: the engine refuses plan.auto for a person's run, a start with no plan picks the active template by applies_to, and trigger.category/severity/priority/tags are the field names the engine reads. All tests mock HTTP. Please confirm them with the live check you already plan to run before the PR leaves draft.
  • Still open from earlier rounds. These are notes only and do not block:
    • The create-only etag has not been tried against a live hive.
    • hive.set in promote_template still uses the client's default retry on a gateway timeout.
  • test-time-budget: no run was found for this commit. master requires no status checks, either in branch protection or in rulesets. This PR does not touch any workflow.

Blast Radius

  • This commit changes only the new investigate module, limacharlie/sdk/investigation.py, the docs, the CHANGELOG and tests.
  • SDK: Investigation.get_effective_policy gains an optional template keyword. The change is additive.
  • CLI behaviour changes, all limited to the unreleased investigate group:
    • runs start --urgency normal is no longer accepted.
    • --auto now sends a request with no plan.
  • The rest of the PR is unchanged from the last review.

This review was generated automatically by Claude Code.

Comment thread limacharlie/sdk/investigation.py Resolved
@limacharlie-refractionpoint

limacharlie-refractionpoint Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

LimaCharlie Cloud Security — code scan

No new code findings were introduced by this pull request.

This check reports and never fails: no gating.fail_on is set on the code_scanning policy.

Scanned refractionPOINT/python-limacharlie 8891083…7834408 — only findings new in the head commit are listed; anything already on the base branch is the repository's own finding set, on the Cloud Security Code page.

This comment is updated in place on every push to this pull request.

…h shapes

Engine v0.1.4 lets a template's report.summary sentence be a string or
{text, when}. The SDK and CLI treat a template as an opaque mapping, so no code
change is needed; tests now pin that save, get, install-defaults and the YAML
round trip keep both shapes unchanged.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of 178a9fd. Since 5e20b76, this commit adds one line to doc/cli/investigation-engine.md and new unit tests in tests/unit/test_sdk_investigation.py. The tests check that a report.summary sentence written as a plain string or as {text, when} survives each path unchanged. No library code changed. I found no high or medium issues and no new findings.

General Feedback

  • The new tests call the real parse_template_source, dump_template, get_template, save_template and install_default_templates, with Hive and the HTTP client mocked. That is mocking, not simulation. The expected values are fixed literals (SUMMARY, SUMMARY_SRC), not copies of the code's logic. The tests use no timing or shared state, so they should not be flaky.
  • The round-trip test includes the string "yes" as a sentence. This is a good check: safe_dump quotes it, and the YAML 1.2 loader would also keep it as text without quotes.
  • Still open from earlier rounds. These are notes only and do not block:
    • The create-only etag has not been tried against a live hive.
    • hive.set in promote_template still uses the client's default retry on a gateway timeout.
    • The engine-side claims about trigger fields and --auto still need the live check planned before the PR leaves draft.
  • test-time-budget: no run was found for this commit. master requires no status checks in branch protection or in rulesets. This PR does not touch any workflow.

Blast Radius

  • This commit changes only a doc line and unit tests. It has no runtime effect.
  • The PR as a whole is the same as at the last review:
    • The typed 401/403/404/422/429 errors now carry response_body. Their classes do not change.
    • New: the lazy-loaded investigate CLI group and the limacharlie.sdk.investigation module.
    • Templates are written to the investigation_plan hive through the existing Hive class.

This review was generated automatically by Claude Code.

…L parser does

Unquoted scalars under a fact's args follow gopkg.in/yaml.v3 (012, 1_000, 0o17,
1e3, timestamps), other template fields keep the author's text except the
numeric and boolean ones, mapping keys are text. A fixture generated from the
engine's own parser pins every edge spelling. The template dumper quotes every
string the engine would read as something else, and save/promote validate text
proven to read back as the stored record. NaN/Infinity are refused as
invalid_input. The docs state who a run executes as when the policy names a
secret identity.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of aef84c4. This commit replaces the YAML 1.2 loader with a port of the engine's yaml.v3 scalar rules. It also sets typing by position in the template: args are resolved like yaml.v3, a short list of fields are numbers or booleans, and everything else is text. A new dumper quotes any string the engine would read as something else, save/promote now refuse a record that does not read back the same, and NaN/Infinity are refused before anything is sent. I found no high or medium issues. There is one low-priority note, inline.

General Feedback

  • Tests: the new tests call the real parse_template_source, dump_template, parse_document, save_template, promote_template and CLI commands. Expected values come from a checked-in fixture that the engine's parser generated, not from this code's own logic. That is real testing, not simulation. The tests use no timing, network or shared state.
  • Behaviour changes in this commit:
    • In wave and sample files, 012, 1_000 and 1e3 are now sent as numbers. Before this commit they were sent as text.
    • promote now raises ValidationError for any stored record with a typed value outside the known fields, for example description: 5. That includes records written by earlier builds of this PR.
  • Not checked: I could not confirm the yaml.v3 claims against the engine. The fixture is stated to come from the engine's parser, so I relied on it.
  • Still open from earlier rounds (notes only, not blocking):
    • The create-only etag has not been tried against a live hive.
    • hive.set in promote_template still uses the client's default retry on a gateway timeout.
    • The engine-side trigger and --auto claims still need the planned live check.
  • test-time-budget: no run was found for this commit. master requires no status checks in branch protection or rulesets, and this PR does not touch any workflow.

Blast Radius

  • This commit changes only the new limacharlie/sdk/investigation.py, the investigate command module, the docs and tests.
  • Investigation._call now refuses non-finite floats with ValidationError. This affects every engine write that has a body.
  • dump_template output now quotes more strings. templates get --yaml output therefore differs from earlier builds of this PR.
  • The rest of the PR is the same as at the last review:
    • The typed 401/403/404/422/429 errors now carry response_body. Their classes do not change.
    • The investigate group is lazy-loaded.
    • Templates are written to the investigation_plan hive.

This review was generated automatically by Claude Code.

Comment thread limacharlie/sdk/investigation.py Resolved
…s with a generated fixture

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of 5f77b1f. Since aef84c4, this commit adds one fixture (tests/unit/fixtures/investigation_typed_fields.json) and one unit test. The test checks that _NUMBER_PATHS and _BOOL_PATHS match the fixture exactly. No library code changed. I found no high or medium issues and no new findings.

General Feedback

  • The new test reads the real module constants and compares them with a fixed fixture. That is real testing, not simulation. It uses no timing, network or shared state, so it will not be flaky.
  • Limit: the fixture is a checked-in copy. The test only catches a new typed engine field after someone regenerates the fixture from the engine. Nothing in this repo regenerates it or checks that it is current. Please add the regeneration step to the release checklist for engine changes.
  • Still open from earlier rounds (notes only, not blocking):
    • The create-only etag has not been tried against a live hive.
    • hive.set in promote_template still uses the client's default retry on a gateway timeout.
    • The engine-side trigger and --auto claims still need the planned live check before the PR leaves draft.
  • test-time-budget: no run was found for this commit. master requires no status checks in branch protection or rulesets. This PR does not touch any workflow.

Blast Radius

  • This commit changes only a test fixture and a unit test. It has no runtime effect.
  • The PR as a whole is the same as at the last review:
    • The typed 401/403/404/422/429 errors now carry response_body. Their classes do not change.
    • New: the lazy-loaded investigate CLI group and the limacharlie.sdk.investigation module.
    • Templates are written to the investigation_plan hive through the existing Hive class.

This review was generated automatically by Claude Code.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

Expose DELETE for runs with validated IDs, explicit CLI confirmation, discovery and documentation. Preserve API errors and disable gateway timeout retries for deletion writes.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of 85824ba. Since 5f77b1f, this commit only adds one paragraph to doc/cli/investigation-engine.md. The paragraph explains which permissions the live sensor metadata operators (sensor.read.*) need on the execution identity. No code or tests changed. I found no high or medium issues and no new findings.

General Feedback

  • Not checked: the new paragraph makes several claims about server-side behaviour. It says sensor.task.read plus output.set is enough, that only allowlisted metadata commands are allowed, that file contents, mutations and unknown commands are refused, and that the permission grants no tasking when the gate is off. None of this code is in this repo, so I could not confirm it. Please include it in the live check planned before the PR leaves draft. If output.set is required, say why in the doc. An "observe-only" identity holding a write permission may surprise readers.
  • Still open from earlier rounds. These are notes only and do not block:
    • The create-only etag has not been tried against a live hive.
    • hive.set in promote_template still uses the client's default retry on a gateway timeout.
    • The engine-side trigger and --auto claims still need the live check.
  • test-time-budget: I found no run for this commit. master requires no status checks, in either branch protection or rulesets. This PR does not touch any workflow.

Blast Radius

  • This commit changes documentation only and has no runtime effect.
  • The PR as a whole is the same as at the last review:
    • The typed 401/403/404/422/429 errors now carry response_body. Their classes do not change.
    • New: the lazy-loaded investigate CLI group and the limacharlie.sdk.investigation module.
    • Templates are written to the investigation_plan hive through the existing Hive class.

This review was generated automatically by Claude Code.

@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of 85824ba. Since 5f77b1f, this commit changes only doc/cli/investigation-engine.md. It adds a paragraph on the permissions that the live sensor metadata operators (sensor.read.*) need. No code or tests changed. I found no high or medium issues. One low-priority doc note is inline.

General Feedback

  • The new paragraph has no fenced YAML block, so test_investigation_examples.py (which parses the doc's YAML blocks) is not affected.
  • The new sensor.task.read + output.set requirement appears only in this doc. The permissions table above it, the _PERMISSION_HINT text in limacharlie/sdk/investigation.py and the investigate explain texts do not mention it. A user or agent who gets a 403 on a sensor.read.* step will not see these permissions in the CLI's hint. Consider adding a row to the table and a short line to the hint.
  • Still open from earlier rounds (notes only, not blocking):
    • The create-only etag has not been tried against a live hive.
    • hive.set in promote_template still uses the client's default retry on a gateway timeout.
    • The engine-side trigger and --auto claims still need the planned live check before the PR leaves draft.
  • test-time-budget: no check run was found for this commit. Earlier rounds found that master requires no status checks in branch protection or rulesets. This PR does not touch any workflow.

Blast Radius

  • This commit changes documentation only. It has no runtime effect.
  • The PR as a whole is the same as at the last review:
    • The typed 401/403/404/422/429 errors now carry response_body. Their classes do not change.
    • New: the lazy-loaded investigate CLI group and the limacharlie.sdk.investigation module.
    • Templates are written to the investigation_plan hive through the existing Hive class.

This review was generated automatically by Claude Code.

Comment thread doc/cli/investigation-engine.md Resolved

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of b0b05c8. Since 85824ba, this commit adds run deletion:

  • Investigation.delete_run sends DELETE runs/{id} with a validated, escaped id and write=True, so a gateway timeout is not retried.
  • A new runs delete RUN_ID --confirm CLI command.
  • Discovery, explain text and doc updates.

I found no high or medium issues. There is one low-priority note, inline.

General Feedback

  • Tests: the new tests call the real delete_run and the real CLI command, with the HTTP client or the SDK mocked. That is mocking, not simulation. The SDK test checks the exact method, the escaped path (run%3A1.a-b) and retry_gateway_timeout=False. The CLI test checks that the SDK is never built without --confirm. The tests use no timing, network or shared state, so they should not be flaky.
  • Small test nit: in test_delete_run_preserves_errors_without_replay, call_count == 1 shows nothing on its own, because a replay would happen inside the mocked client.request. The retry_gateway_timeout is False check is the assertion that matters.
  • Still open from earlier rounds (notes only, not blocking):
    • The create-only etag has not been tried against a live hive.
    • hive.set in promote_template still uses the client's default retry on a gateway timeout.
    • The engine-side trigger, --auto and sensor.read.* permission claims still need the live check planned before the PR leaves draft.
  • test-time-budget: no run was found for this commit. master requires no status checks in branch protection or in rulesets, and this PR does not touch any workflow.

Blast Radius

  • This commit changes only the new limacharlie/sdk/investigation.py, the investigate command module, the discovery profile list, the docs and tests.
  • SDK: a new public method, Investigation.delete_run. The change is additive.
  • CLI: a new destructive command, investigate runs delete, gated by --confirm. The investigation_engine discovery profile now lists it, so agents that load that profile will see it.
  • The rest of the PR is the same as at the last review:
    • The typed 401/403/404/422/429 errors now carry response_body. Their classes do not change.
    • The investigate group is lazy-loaded.
    • Templates are written to the investigation_plan hive.

This review was generated automatically by Claude Code.

Comment thread limacharlie/sdk/investigation.py Resolved
… a server setting name from the docs

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

Review round: reworded the docs line so it no longer names a server setting. runs delete needs an API deployment that serves run deletion (newer than the other run routes); a 404 with no engine error code now says exactly that instead of pointing at the feature flag, and the docs note it. The live check of the delete route, the create-only etag and the permission claims stays on the list to do before this leaves draft.

@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of 3cc81af. Since b0b05c8, this commit does three things:

  • delete_run gives a run-deletion hint for a 404 that has no engine error code.
  • The docs no longer name the server setting.
  • Two tests are added, and the call_count check that proved nothing is removed.

I found one medium issue, inline. The new hint is set on the exception after the exception text was built. So table output still shows the old "feature not enabled" hint, and the JSON error field shows both hints.

General Feedback

  • The doc change resolves the earlier note about naming the server setting.
  • The run-deletion 404 hint was meant to resolve the earlier DELETE runs/{id} thread. It only works once the inline issue is fixed. Please also confirm the route in the live check you plan to run.
  • Tests: the new tests call the real delete_run with the HTTP client mocked. That is mocking, not simulation. They only assert .suggestion, so they miss what the user actually sees. Please also assert str(ei.value) and ei.value.to_dict()["error"].
  • Still open from earlier rounds (notes only, not blocking):
    • The create-only etag has not been tried against a live hive.
    • hive.set in promote_template still uses the client's default retry on a gateway timeout.
    • The engine-side trigger, --auto and sensor.read.* claims still need the live check.
  • test-time-budget: no run was found for this commit. master requires no status checks in branch protection or in rulesets, and this PR does not touch any workflow.

Blast Radius

  • This commit only changes Investigation.delete_run error reporting, the docs and unit tests, all inside the new, unreleased investigate feature.
  • The rest of the PR is the same as at the last review:
    • The typed 401/403/404/422/429 errors now carry response_body. Their classes do not change.
    • The investigate CLI group is lazy-loaded.
    • Templates are written to the investigation_plan hive.

This review was generated automatically by Claude Code.

Comment thread limacharlie/sdk/investigation.py Resolved
…carries one hint

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@maximelb

Copy link
Copy Markdown
Contributor Author

/lc-review

@limacharlie-event-notifications limacharlie-event-notifications Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review Summary

Re-review of 7834408. Since 3cc81af, this commit rebuilds the run-deletion 404 error in delete_run instead of setting exc.suggestion on the old error. It also extends the test. I found no high or medium issues and no new findings.

General Feedback

  • The fix is correct. exc.raw_message strips the old _DISABLED_HINT suffix, because suggestion was not changed before it was read. The new InvestigationError then holds only _DELETE_ROUTE_HINT. response_body, issues, warnings, the status and exit_code all carry over. error_code is still None, because the body had no engine code.
  • The test now checks what the user sees: str(ei.value) has exactly one Suggestion: line, the old hint is gone, and to_dict()["error"] is the plain message. It calls the real delete_run with the HTTP client mocked. That is mocking, not simulation.
  • Still open from earlier rounds (notes only, not blocking):
    • The live checks are still pending: the DELETE runs/{id} route, the create-only etag, and the engine-side trigger, --auto and sensor.read.* claims.
    • hive.set in promote_template still uses the client's default retry on a gateway timeout.
  • test-time-budget: no run was found for this commit. master requires no status checks in branch protection or in rulesets. This PR does not touch any workflow.

Blast Radius

  • This commit only changes Investigation.delete_run error reporting and one unit test. Both are inside the new, unreleased investigate feature.
  • The rest of the PR is the same as at the last review:
    • The typed 401/403/404/422/429 errors now carry response_body. Their classes do not change.
    • The investigate CLI group is lazy-loaded.
    • Templates are written to the investigation_plan hive.

This review was generated automatically by Claude Code.

This branch has not been deployed

No deployments
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.

1 participant