Repository navigation
Conversation
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
/lc-review |
There was a problem hiding this comment.
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_templatecreates new records with the sentinel etagcreate-only: the record must not exist. No other code in this repo uses a sentinel etag to create a record. The unit tests mockHive, 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, everytemplates saveof a new name fails, and so does everyinstall-defaultsentry. Please check this against a real hive before the PR leaves draft. - No retry guard on hive writes.
hive.setinsave_template/promote_templateuses the client's defaultretry_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-budgetcheck run was found for this commit. The base branchmasterdoes 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_codenow setsresponse_bodyon the 401/403/404/422/429 typed errors, andClient.requestsets it onRateLimitError. The attribute is new on those classes. They do not becomeApiErrorsubclasses, soisinstance(..., ApiError)checks,_lc_error_codeandis_record_not_foundbehave as before. Code that calledhasattr(e, 'response_body')to detect anApiErrorwould now also match these errors. No such caller exists in this repo.- New top-level
investigateCLI group, lazy-loaded via_COMMAND_MODULE_MAP. There is a newinvestigation_enginediscovery profile and a new help topic. These add to the command surface without changing existing commands. - The new SDK class
limacharlie.sdk.investigation.Investigationwrites to theinvestigation_planhive through the existingHiveclass.
This review was generated automatically by Claude Code.
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>
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
Draft-by-default
Errors, secrets, paging, termination
Known limits (not bugs)
|
|
/lc-review |
There was a problem hiding this comment.
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,1e3and.inf. iter_actionshas the same cursor-advance guard asiter_runs, andactions list --allnow uses it.runs start --followin a structured format now writes JSON lines.- A landed-write guard is added in
save_template.
- The YAML 1.2 int/float resolvers and the int constructor are correct for
- 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
Hivemocked. That is mocking, not simulation. - test-time-budget: no run was found for this commit.
masterrequires no status checks, either in branch protection or in rulesets.
Blast Radius
errors.error_from_status_codeandClient.requestnow setresponse_bodyon the typed 401/403/404/422/429 errors. These errors do not change class, so existingisinstancechecks and the hive helpers (is_record_not_found,is_permission_denied) behave as before.- The rest of the PR is additive:
- the
investigateCLI group, which is lazy-loaded; - the
limacharlie.sdk.investigationmodule; - a discovery profile and a help topic;
- writes to the
investigation_planhive through the existingHiveclass.
- the
This review was generated automatically by Claude Code.
…d tags Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
/lc-review |
There was a problem hiding this comment.
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, thecommentand thetags. So whenhive.setfails on an overwrite, a lost--commentor--tagis now reported as an error. The new test,test_a_failed_overwrite_does_not_pass_as_saved_when_the_comment_was_lost, calls the realsave_templatewithHivemocked. 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.setinpromote_templatestill uses the client's default gateway-timeout retry.
- test-time-budget: I found no run for this commit.
masterrequires no status checks, in either branch protection or rulesets.
Blast Radius
- This commit only changes
limacharlie/sdk/investigation.py(save_templateerror path) and its unit test. It does not touch existing code. - The PR as a whole is unchanged from the last review:
response_bodyis now set on the typed 401/403/404/422/429 errors. Their classes do not change.- The other changes are additive: the lazy-loaded
investigateCLI group, thelimacharlie.sdk.investigationmodule, a discovery profile, a help topic, and writes to theinvestigation_planhive.
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>
|
/lc-review |
There was a problem hiding this comment.
AI Code Review Summary
Re-review of 4c42051. This commit changes four things:
- Wave and
--samples-fileinputs 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_errorsnow wraps everyApiErrorand keepsresponse_body, soerror_codestill carries the hive'slc_error_code. Non-ApiErrorerrors that are not permission refusals (ValidationError,NotFoundError, ...) still pass through unchanged.install_default_templatescatchesLimaCharlieError, so wrapping does not change what it does.save_templatehandles 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_errorsnow catches everyLimaCharlieError. Each error keeps its ownexit_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_documentturns a value JSON cannot carry into aValidationError. A recursive alias falls in that case, becausejson.dumpsraisesValueError.- 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_templateand CLI commands, withHiveand the SDK mocked. That is mocking, not simulation.test_investigation_examples.pyreads 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 firsthive.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.setinpromote_templatestill 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.
masterrequires no status checks in either branch protection or rulesets. This PR does not touch any workflow.
Blast Radius
- This commit changes only the new
investigatecommand 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_templateanddelete_templatenow raiseInvestigationErrorfor hiveApiErrorfailures.InvestigationErroris still anApiErrorsubclass, so existingexcept ApiErrorhandlers still catch it. - In the CLI, input errors on
investigatecommands now exit with 4 instead of click's usage-error code 2. - The rest of the PR is unchanged from the last review:
response_bodyis set on the typed 401/403/404/422/429 errors.- The
investigategroup is lazy-loaded. - Templates are written to the
investigation_planhive.
This review was generated automatically by Claude Code.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Self-review of the fixes (4c42051 + test nit): (1) wave/sample files now go through the same YAML 1.2 loader as templates, so |
|
/lc-review |
There was a problem hiding this comment.
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_templateandinstall_default_templatesagain againstHive.set. When no tags or comment are given,Hive.setsends nousr_mtdblock. So a plain overwrite or promote does not resetenabledor 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.setinpromote_templatestill 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.
masterrequires 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
investigateCLI group and thelimacharlie.sdk.investigationmodule are new, along withparse_document. - Template-record failures are wrapped as
InvestigationError, which is still a subclass ofApiError. - Writes go to the
investigation_planhive through the existingHiveclass.
- The typed 401/403/404/422/429 errors now carry
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>
|
/lc-review |
There was a problem hiding this comment.
AI Code Review Summary
Re-review of 5e20b76. This commit changes the investigate flags and help text to match the engine:
runs start --autonow sends noplan.runs starthas new--category,--severity,--priorityand--tagflags that go intotrigger.--urgencyis nowlow|medium|high|critical.--stricthas a new meaning.runs listhas new help text.policy get --templateis 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
--helpoutput, 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.autofor a person's run, a start with no plan picks the active template byapplies_to, andtrigger.category/severity/priority/tagsare 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.setinpromote_templatestill uses the client's default retry on a gateway timeout.
- test-time-budget: no run was found for this commit.
masterrequires 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
investigatemodule,limacharlie/sdk/investigation.py, the docs, the CHANGELOG and tests. - SDK:
Investigation.get_effective_policygains an optionaltemplatekeyword. The change is additive. - CLI behaviour changes, all limited to the unreleased
investigategroup:runs start --urgency normalis no longer accepted.--autonow sends a request with noplan.
- The rest of the PR is unchanged from the last review.
This review was generated automatically by Claude Code.
LimaCharlie Cloud Security — code scanNo new code findings were introduced by this pull request. This check reports and never fails: no Scanned 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>
|
/lc-review |
There was a problem hiding this comment.
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_templateandinstall_default_templates, withHiveand 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_dumpquotes 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.setinpromote_templatestill uses the client's default retry on a gateway timeout.- The engine-side claims about
triggerfields and--autostill need the live check planned before the PR leaves draft.
- test-time-budget: no run was found for this commit.
masterrequires 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
investigateCLI group and thelimacharlie.sdk.investigationmodule. - Templates are written to the
investigation_planhive through the existingHiveclass.
- The typed 401/403/404/422/429 errors now carry
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>
|
/lc-review |
There was a problem hiding this comment.
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_templateand 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_000and1e3are now sent as numbers. Before this commit they were sent as text. promotenow raisesValidationErrorfor any stored record with a typed value outside the known fields, for exampledescription: 5. That includes records written by earlier builds of this PR.
- In wave and sample files,
- 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.setinpromote_templatestill uses the client's default retry on a gateway timeout.- The engine-side
triggerand--autoclaims still need the planned live check.
- test-time-budget: no run was found for this commit.
masterrequires 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, theinvestigatecommand module, the docs and tests. Investigation._callnow refuses non-finite floats withValidationError. This affects every engine write that has a body.dump_templateoutput now quotes more strings.templates get --yamloutput 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
investigategroup is lazy-loaded. - Templates are written to the
investigation_planhive.
- The typed 401/403/404/422/429 errors now carry
This review was generated automatically by Claude Code.
…s with a generated fixture Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
/lc-review |
There was a problem hiding this comment.
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.setinpromote_templatestill uses the client's default retry on a gateway timeout.- The engine-side
triggerand--autoclaims still need the planned live check before the PR leaves draft.
- test-time-budget: no run was found for this commit.
masterrequires 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
investigateCLI group and thelimacharlie.sdk.investigationmodule. - Templates are written to the
investigation_planhive through the existingHiveclass.
- The typed 401/403/404/422/429 errors now carry
This review was generated automatically by Claude Code.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
/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>
|
/lc-review |
There was a problem hiding this comment.
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.readplusoutput.setis 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. Ifoutput.setis 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.setinpromote_templatestill uses the client's default retry on a gateway timeout.- The engine-side
triggerand--autoclaims still need the live check.
- test-time-budget: I found no run for this commit.
masterrequires 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
investigateCLI group and thelimacharlie.sdk.investigationmodule. - Templates are written to the
investigation_planhive through the existingHiveclass.
- The typed 401/403/404/422/429 errors now carry
This review was generated automatically by Claude Code.
|
/lc-review |
There was a problem hiding this comment.
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.setrequirement appears only in this doc. The permissions table above it, the_PERMISSION_HINTtext inlimacharlie/sdk/investigation.pyand theinvestigateexplain texts do not mention it. A user or agent who gets a 403 on asensor.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.setinpromote_templatestill uses the client's default retry on a gateway timeout.- The engine-side
triggerand--autoclaims 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
masterrequires 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
investigateCLI group and thelimacharlie.sdk.investigationmodule. - Templates are written to the
investigation_planhive through the existingHiveclass.
- The typed 401/403/404/422/429 errors now carry
This review was generated automatically by Claude Code.
There was a problem hiding this comment.
AI Code Review Summary
Re-review of b0b05c8. Since 85824ba, this commit adds run deletion:
Investigation.delete_runsendsDELETE runs/{id}with a validated, escaped id andwrite=True, so a gateway timeout is not retried.- A new
runs delete RUN_ID --confirmCLI 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_runand 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) andretry_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 == 1shows nothing on its own, because a replay would happen inside the mockedclient.request. Theretry_gateway_timeout is Falsecheck 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.setinpromote_templatestill uses the client's default retry on a gateway timeout.- The engine-side
trigger,--autoandsensor.read.*permission claims still need the live check planned before the PR leaves draft.
- test-time-budget: no run was found for this commit.
masterrequires 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, theinvestigatecommand 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. Theinvestigation_enginediscovery 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
investigategroup is lazy-loaded. - Templates are written to the
investigation_planhive.
- The typed 401/403/404/422/429 errors now carry
This review was generated automatically by Claude Code.
… a server setting name from the docs Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Review round: reworded the docs line so it no longer names a server setting. |
|
/lc-review |
There was a problem hiding this comment.
AI Code Review Summary
Re-review of 3cc81af. Since b0b05c8, this commit does three things:
delete_rungives 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_countcheck 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_runwith the HTTP client mocked. That is mocking, not simulation. They only assert.suggestion, so they miss what the user actually sees. Please also assertstr(ei.value)andei.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.setinpromote_templatestill uses the client's default retry on a gateway timeout.- The engine-side
trigger,--autoandsensor.read.*claims still need the live check.
- test-time-budget: no run was found for this commit.
masterrequires 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_runerror reporting, the docs and unit tests, all inside the new, unreleasedinvestigatefeature. - 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
investigateCLI group is lazy-loaded. - Templates are written to the
investigation_planhive.
- The typed 401/403/404/422/429 errors now carry
This review was generated automatically by Claude Code.
…carries one hint Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
/lc-review |
There was a problem hiding this comment.
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_messagestrips the old_DISABLED_HINTsuffix, becausesuggestionwas not changed before it was read. The newInvestigationErrorthen holds only_DELETE_ROUTE_HINT.response_body,issues,warnings, the status andexit_codeall carry over.error_codeis still None, because the body had no engine code. - The test now checks what the user sees:
str(ei.value)has exactly oneSuggestion:line, the old hint is gone, andto_dict()["error"]is the plain message. It calls the realdelete_runwith 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-sidetrigger,--autoandsensor.read.*claims. hive.setinpromote_templatestill uses the client's default retry on a gateway timeout.
- The live checks are still pending: the
- test-time-budget: no run was found for this commit.
masterrequires no status checks in branch protection or in rulesets. This PR does not touch any workflow.
Blast Radius
- This commit only changes
Investigation.delete_runerror reporting and one unit test. Both are inside the new, unreleasedinvestigatefeature. - 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
investigateCLI group is lazy-loaded. - Templates are written to the
investigation_planhive.
- The typed 401/403/404/422/429 errors now carry
This review was generated automatically by Claude Code.
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 investigatecommand group and anInvestigationSDK 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.Agent workflow (documented in
doc/cli/investigation-engine.md,limacharlie help investigation-engineand--ai-help):kit-> draft ->validate->teston past alerts -> refine ->saveas a draft -> a person runspromote.Safety properties
templates savealways writes a draft. A source that saysstatus: activeis refused; only an explicit--status active(a person's call) writes an active template. Nothing is written when validation has errors.--overwrite; a draft never replaces an active template; creates are conditional on the record not existing.promoteanddeleterequire--confirm;promotere-validates and writes conditionally on the etag it read.install-defaultsnever overwrites silently: existing records are skipped with installed and library versions. Library templates are installed as drafts unless--status activeis passed.Output and errors
JSON by default when piped; the usual
--outputformats otherwise, with readable text fortemplates test(per-sample verdicts and the agreement summary) and listings. Engine failures keep the engine'serror_codeand codederrors/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(asApiErroralready did), which the new error mapping needs.Depends on
kit --section library, used bytemplates 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.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) andtests/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