feat(cli): registry-driven completion + canonical flag/arg rename (CLI ergonomics Plan 2) - #80
Merged
Merged
Conversation
…n 2 T1) New action_flags module (FlagSpec/ValueKind/COMMON_LOG_FLAGS); ActionSpec gains flags + examples fields with a full-form macro arm; flags_for/examples_for accessors. search/filter/tail/errors carry COMMON_LOG_FLAGS; all CLI query actions ship at least one example. (dead_code allows are scoped; removed when the completion engine + help consume them in later tasks.)
…2, stage 1/n) search/filter/tail/errors/sessions/incident/correlate/timeline/patterns: --hostname->--host, --source-ip->--source, --app-name->--app, --from->--since, --to->--until, --received-from->--received-since, --received-to->--received-until. Collapsed incident's now-duplicate --hostname|--host arm. Tests updated; service-logs/AI flags renamed in later stages. (help.rs usage text + MCP args follow.)
…surface (Plan 2 T2, stage 2/2) Renames --hostname->--host, --source-ip->--source, --app-name->--app, --from->--since, --to->--until, --received-from->--received-since, --received-to->--received-until across parse_ai/parse_ai_more/parse_admin/ commands + help usage text + all CLI tests. CLI flag vocabulary is now uniform. MCP arg rename deferred (collisions + serde, see next).
…ived_since/received_until (Plan 2 T3, stage 1) Renames the request-arg field across app Request structs, db SearchParams, CLI arg structs, into_request, MCP schema property keys, arg docs, and tests. Field name = MCP wire key = CLI flag (no serde aliases). These fields are request-only (no response-data equivalent), so a global identifier rename was safe.
…n 2 T3) Renames request/arg struct fields source_ip->source, app_name->app, from->since, to->until, hostname->host across *Request (app/models), *Params (db), CLI *Args, REST *Query structs, file_tail request structs, ~510 readers, MCP schema property keys, arg docs, and tests. Field name = MCP wire key = CLI flag (no serde aliases). Response/data structs (LogEntry, *Entry, FileTailSource, etc.) keep their names -- output is a separate contract. hostname/from folded into the pre-existing host/since schema properties.
Registry-driven completion: cortex __complete actions|flags|value emits candidates from ACTION_SPECS (action names+descriptions, per-command flags+help, fixed enums + relative-time hints) plus LIVE values (hostnames/apps/source IDs) via a bounded read-only DB query cached ~60s with a 150ms timeout that degrades silently to empty. cortex completions zsh installs a delegating _cortex function. Exposed FlagSpec/ValueKind + flags_for/examples_for/description_for from mcp; added a CLI registry facade (hyphen<->underscore mapping).
cortex <command> --help now appends an Examples block built from the ACTION_SPECS examples field, keeping examples in lockstep with canonical flags.
…an 2 T8)
CONFIG.md, CLI.md, contracts/forwarder-dropins.md, smoke-test-http.sh, smoke-ai.sh,
and docs/mcp/{TESTS,PATTERNS}.md: --hostname->--host, --source-ip->--source,
--app-name->--app, --from->--since, --to->--until (+ received-* and MCP arg keys).
Completes the canonical-vocabulary migration across the docs surface.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR adds shell completion and registry-backed help examples, standardizes canonical CLI/MCP/API request names such as ChangesCLI surface and request contract update
Sequence Diagram(s)sequenceDiagram
participant Shell
participant CortexCLI
participant Registry
participant SQLite
Shell->>CortexCLI: cortex __complete flags search
CortexCLI->>Registry: lookup flags/examples/descriptions
Registry-->>CortexCLI: action metadata
Shell->>CortexCLI: cortex __complete value --host
CortexCLI->>SQLite: query distinct host values (cached)
SQLite-->>CortexCLI: host candidates
CortexCLI-->>Shell: completion candidates
Estimated code review effort🎯 5 (Critical) | ⏱️ ~100 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Plan 2 (1.26.0) renamed CLI flags / MCP args to canonical names but left several user-facing strings on the old vocabulary, where following them now errors against the migrated parser/schema: - file-tail add usage + required-flag error: --hostname -> --host - ai incident-context required-flag errors: --from/--to -> --since/--until - search/abuse/ask-history "scan capped" hints: --from/--to -> --since/--until - file_tails op=add admin help param: hostname -> host - similar_incidents MCP input example: app_name -> app Also reverted the similar_incidents doc 'Response fields' line to the unchanged output names (hostname/app_name) — the output struct IncidentCluster was correctly never renamed, so the doc had drifted off the real contract. Tests/clippy/fmt green; patch bump to 1.26.1.
5 tasks
…letion-rename # Conflicts: # CHANGELOG.md # Cargo.lock # Cargo.toml # docker-compose.prod.yml # mcpb/manifest.json # server.json # src/cli/parse_logs.rs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
src/cli/parse_ai_more_tests.rs (1)
21-63: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winStrengthen assertions for renamed canonical filters.
Several updated tests pass renamed flags but do not assert the corresponding parsed fields (
since/until, and someapp/toolvalues). That weakens regression protection for the rename contract.Consider explicitly asserting these in:
parse_ai_similar_and_ask_history_accept_all_filtersparse_ai_incidents_accepts_terms_and_window_filtersparse_ai_investigate_accepts_incident_filters_and_limitsparse_ai_assess_accepts_incident_and_investigation_filtersAlso applies to: 101-211
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/parse_ai_more_tests.rs` around lines 21 - 63, The test function parse_ai_similar_and_ask_history_accept_all_filters is missing assertions for the since and until fields that are being passed to the parse functions. In the match arm for SimilarIncidents, add assertions to verify args.since and args.until match the expected values (t0 and t1 respectively). Similarly, in the match arm for AskHistory, add assertions to verify args.since and args.until are correctly parsed. This strengthens the regression protection for the renamed filter contract.src/cli/parse_ai_tests.rs (1)
57-127: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winValidate all newly renamed fields in parser expectations.
These tests now pass canonical flags (
--since/--until,--host/--app/--source) but only assert a subset of mapped fields, leaving rename regressions under-tested.Also applies to: 143-164
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/parse_ai_tests.rs` around lines 57 - 127, The test function parse_ai_search_abuse_and_correlate_accept_equals_forms() validates only a subset of the fields set by the parser functions. Add assertions for all the fields being passed in the test inputs to ensure renamed fields are fully tested. For parse_ai_search, add assertions for the since and until date fields; for parse_ai_abuse, add assertions for project, tool, since, until, and limit fields; for parse_ai_correlate, add assertions for all remaining fields including project, tool, ai_query, log_query, host, app, since, until, and severity_min that are currently passed but not validated.src/app/services/logs.rs (1)
41-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHost/time parse errors still reference legacy parameter names.
Line 41 reports
hostname, and Lines 317-318 label timestamp errors asfrom/toeven though the handler consumeshost,since, anduntil.Proposed fix
- "host_state requires host_id or hostname".into(), + "host_state requires host_id or host".into(), @@ - let from = parse_optional_timestamp(req.since.as_deref(), "from")?; - let to = parse_optional_timestamp(req.until.as_deref(), "to")?; + let from = parse_optional_timestamp(req.since.as_deref(), "since")?; + let to = parse_optional_timestamp(req.until.as_deref(), "until")?;Also applies to: 317-318
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/services/logs.rs` around lines 41 - 42, The error messages in the logs.rs file are referencing outdated parameter names that no longer match the current handler API. Update the error message around line 41 that currently references "hostname" to reference "host" instead, and update the timestamp-related error messages around lines 317-318 that currently reference "from" and "to" parameters to reference "since" and "until" respectively to align with the actual parameter names consumed by the handler.src/cli/parse.rs (1)
12-47:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep top-level command suggestions in sync with parsed commands.
Line 93 adds
completionsas a valid command, butTOP_LEVEL_COMMANDS(used at Line 96 for unknown-command suggestions) does not include it. That makes typo guidance inconsistent with actual parser behavior.Proposed fix
pub(crate) const TOP_LEVEL_COMMANDS: &[&str] = &[ @@ "correlate-state", "file-tail", + "completions", ];Also applies to: 92-93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/parse.rs` around lines 12 - 47, The TOP_LEVEL_COMMANDS constant array is missing the "completions" command that was added to the parser at line 93, causing inconsistency between what the parser accepts and what suggestions are offered for unknown commands at line 96. Add "completions" as a string entry to the TOP_LEVEL_COMMANDS array to keep it synchronized with all valid parsed commands.src/cli/dispatch_ai_tests.rs (1)
14-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert
since/untilin this filter-preservation test.Line 14 onward verifies other filters but not the renamed time filters, so a mapping regression for
since/untilwould go undetected.Suggested patch
assert_eq!(req.query, "error"); assert_eq!(req.project.as_deref(), Some("/repo")); assert_eq!(req.tool.as_deref(), Some("codex")); + assert_eq!(req.since.as_deref(), Some("2026-01-01T00:00:00Z")); + assert_eq!(req.until, None); assert_eq!(req.limit, Some(25));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/dispatch_ai_tests.rs` around lines 14 - 18, The test assertion block for the filter-preservation test is missing assertions for the renamed time filter fields `since` and `until`. Add assertions after the existing assert_eq calls to verify that both `req.since` and `req.until` are correctly mapped and preserved, using the same pattern as the other assertions like those for `query`, `project`, `tool`, and `limit` to ensure regression detection for these time-based filters.src/cli/dispatch_surface.rs (1)
31-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale timeline comment terminology to canonical flags.
The comment still says
from/to, but this path now forwardssince/until. Keeping comments aligned avoids drift confusion during future refactors.Suggested patch
- // it applies a bucket-sized window only when neither `from` nor `to` is set. + // it applies a bucket-sized window only when neither `since` nor `until` is set. // Both CLI modes reach that service (local directly, HTTP via the server), - // so we pass `from`/`to` through verbatim — no per-binary duplication. + // so we pass `since`/`until` through verbatim — no per-binary duplication.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/dispatch_surface.rs` around lines 31 - 33, The comment in dispatch_surface.rs contains outdated terminology referring to `from` and `to` parameters, but the current implementation uses `since` and `until` flags instead. Update the comment text to replace the references to `from/to` with `since/until` to align the documentation with the actual flag names being used in the code.src/cli/dispatch_tests.rs (1)
1485-1487:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign timeline test wording with
since/untilnomenclature.The test comments and assertion messages still reference
from/to, which no longer matches the request contract under test.Suggested patch
- // must therefore pass `from`/`to` through verbatim and NOT inject a default + // must therefore pass `since`/`until` through verbatim and NOT inject a default @@ - "into_request must not inject a default `from`; the service applies it" + "into_request must not inject a default `since`; the service applies it" @@ - "into_request must not inject a default `to`" + "into_request must not inject a default `until`" @@ -fn timeline_args_into_request_explicit_from_preserved() { - // Explicit from must override the default. +fn timeline_args_into_request_explicit_from_preserved() { + // Explicit since must override the default. @@ - "explicit from must not be overridden by the default" + "explicit since must not be overridden by the default"Also applies to: 1502-1507, 1512-1513, 1527-1528
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/dispatch_tests.rs` around lines 1485 - 1487, The test comments and assertion messages in the dispatch_tests.rs file still reference the old `from/to` parameter nomenclature, but the underlying request contract has been updated to use `since/until` instead. Update all comments and assertion messages across the specified ranges (including the test `timeline_applies_default_lookback_only_when_from_and_to_both_absent` and other related assertions) to replace references to `from/to` with the new `since/until` terminology to maintain alignment with the actual request contract being tested.src/api.rs (1)
560-569:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject stale timeline/patterns query keys instead of silently ignoring them.
Line 560 and Line 594 currently accept unknown query params, so renamed-out keys like
from,to,hostname, andapp_nameare silently dropped for these endpoints. That can produce broader/incorrect results and is inconsistent with the stricter renamed handlers in this file.Proposed fix
#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] struct TimelineQuery { @@ #[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] struct PatternsQuery {Also applies to: 594-603
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api.rs` around lines 560 - 569, The TimelineQuery struct and the similar query struct around line 594-603 silently ignore unknown query parameters instead of rejecting them, which allows stale/renamed keys like from, to, hostname, and app_name to be dropped without error and produce incorrect results. Add the #[serde(deny_unknown_fields)] attribute to both the TimelineQuery struct and the other query struct to make deserialization fail when unknown fields are provided, ensuring consistency with stricter handlers in the file and preventing silent data loss.README.md (1)
189-220:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftCritical inconsistency: parameter tables use old flag names; narrative uses new names.
The
cortex searchparameters table (lines 189-198) and response examples still show old parameter names (hostname,source_ip,app_name,from,to) without the tilde marks indicating changes in this PR. However, the narrative text at line 238 and examples throughout the file use the new canonical names (host,source,app,since,until). Per the PR objectives, request struct fields should be renamed to canonical names with no serde aliases.This creates a documentation contract break: users reading the parameter reference will see incorrect field/flag names that do not match the actual CLI or MPC wire protocol.
Scope of issue: This same inconsistency likely affects all parameter tables in README.md:
cortex search(lines 189-198) — shows old namescortex tail(lines 250-256) — check if updatedcortex errors(lines 268-273) — check if updatedcortex sessions(lines 319-327) — check if updatedcortex correlate(lines 354-364) — check if updatedVerify and update all parameter tables to use canonical field names (
host,source,app,since,until,received_since,received_until) to match the wire protocol and examples in the same document.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 189 - 220, The parameter tables in README.md for cortex commands (search, tail, errors, sessions, correlate) are using outdated parameter names that do not match the canonical names used in the narrative text and API examples. Update all parameter tables to replace the old names with the new canonical names: change `hostname` to `host`, `source_ip` to `source`, `app_name` to `app`, `from` to `since`, and `to` to `until`. Verify this change is applied consistently across all command parameter tables in the document to ensure the documentation accurately reflects the actual CLI interface and wire protocol.docs/superpowers/plans/2026-05-21-surface-parity.md (1)
74-82:⚠️ Potential issue | 🟡 MinorUpdate REST API query struct templates in plan to match canonical field names in actual implementation.
The plan document shows inconsistency: lines 656–657 correctly document CLI flags with canonical names (
--host,--app,--since,--until), but the code templates for TimelineQuery (lines 74–82) and PatternsQuery (lines 107–115) use old field names (from,to,hostname,app_name). The actual implementation insrc/app/models/stats.rsalready uses the canonical field names.Update these struct templates to match the real implementation:
🔧 Proposed update for TimelineQuery and handler
#[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] struct TimelineQuery { bucket: Option<String>, group_by: Option<String>, - from: Option<String>, - to: Option<String>, - hostname: Option<String>, - app_name: Option<String>, + since: Option<String>, + until: Option<String>, + host: Option<String>, + app: Option<String>, severity_min: Option<String>, } async fn timeline( State(state): State<ApiState>, Query(query): Query<TimelineQuery>, ) -> impl IntoResponse { use syslog_mcp::app::TimelineRequest; respond( state .service .timeline(TimelineRequest { bucket: query.bucket, group_by: query.group_by, - from: query.from, - to: query.to, - hostname: query.hostname, - app_name: query.app_name, + since: query.since, + until: query.until, + host: query.host, + app: query.app, severity_min: query.severity_min, }) .await, ) }Similarly, update PatternsQuery (lines 107–115) to use
since,until,host,app.Also applies to: 107-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/plans/2026-05-21-surface-parity.md` around lines 74 - 82, The TimelineQuery struct (lines 74-82) and PatternsQuery struct (lines 107-115) in the plan document use outdated field names that do not match the canonical field names documented in the CLI flags section and implemented in the actual codebase. Update the TimelineQuery struct to rename the fields: from to since, to to until, hostname to host, and app_name to app. Apply the same field name updates to the PatternsQuery struct to ensure consistency with the canonical field names and the actual implementation in src/app/models/stats.rs.src/cli/help.rs (1)
904-916:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftNested command help bypasses registry examples.
Line 915 returns from the nested branch before the examples block at Lines 928-937, so nested help paths never render registry examples. That breaks the “examples for all commands” behavior and creates inconsistent
--helpUX.Also applies to: 928-937
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/help.rs` around lines 904 - 916, The nested command help path in the nested_lookup block returns early at the end of the nested branch, preventing execution of the examples block that follows. To fix this, remove the early return statement from the nested branch and instead append the registry examples to the output string before returning. Ensure that the examples rendering logic (which appears to be in the subsequent lines after the nested block) is incorporated into the nested branch so that both nested command help and registry examples are rendered consistently for all help paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 10-16: The changelog footer contains stale compare-link references
that need to be updated to match the new version sections added in the 1.26.1
and 1.26.0 entries. Update the footer by adding link definitions for both the
new versions (1.26.1 and 1.26.0) that show the git compare URLs between the
appropriate version tags, and update the Unreleased link reference to point to
the head of the repository instead of the outdated v1.20.0 reference. Ensure
each version has a corresponding link definition in the footer that follows the
existing format and points to the correct git compare paths.
In `@docs/superpowers/plans/2026-05-21-ai-abuse-incidents.md`:
- Around line 434-436: In the warning message string that mentions "candidate
scan capped at {} rows", replace the flag reference `--from` with `--since` to
match the canonical rename and maintain consistency with the documentation
examples above. This flag reference appears in the string being logged with
response.candidate_cap.
In
`@docs/superpowers/plans/2026-06-15-cortex-cli-query-safety-and-time-parsing.md`:
- Around line 278-279: The plan description at lines 278-279 states that Plan 1
keeps the current flag names `--from` and `--to`, but the example(s) shown in
the document use `--since` instead. Align the examples with the stated plan by
updating any example commands to use `--from` and `--to` flags instead of
`--since`, or alternatively, if the examples are correct and should use
`--since`, update the plan description to reflect that Plan 1 includes this flag
rename. Ensure consistency between the plan statement and all referenced
examples throughout the document.
In
`@docs/superpowers/plans/2026-06-15-cortex-cli-registry-completion-and-rename.md`:
- Around line 27-36: The "Old CLI flag" column in the flag mapping table is
showing canonical flag names instead of the actual legacy spellings that were
previously used. Restore the original legacy flag names in this column for each
row (for example, replace `--host` with `--hostname`, and ensure `source id` row
shows the actual old spelling instead of the current canonical name). The goal
is to preserve the complete rename mapping that shows what users were calling
these flags before the standardization.
- Around line 332-333: The grep patterns in the completeness-gate check are
currently searching for canonical flag names instead of legacy ones, which means
they won't detect leftover unmigrated legacy spellings. Replace the patterns in
the grep command (at the location showing
'--host|--source|--app|"--since"|"--until"|--received-since|--received-until')
with the corresponding legacy flag name patterns that the migration is moving
away from, ensuring the check will properly catch any instances of old flag
spellings that still need to be converted to their canonical forms. Apply the
same fix to the duplicate check mentioned at lines 816-818.
- Around line 284-289: The "Before" code block in the documentation is showing
the current canonical flag names (--host, --source, --app, --since, --until)
instead of the original pre-rename flag names. To properly demonstrate the
migration, replace the flag names in this "Before" example with their original
deprecated names that existed before the rename, so that the before/after
comparison clearly shows what was changed during the flag naming refactoring.
- Around line 314-317: The test function `search_rejects_legacy_hostname_flag`
is currently passing the canonical flag `--host` instead of the legacy flag that
it should be testing. Change the argument passed to parse_search from `"--host"`
to `"--hostname"` so that the test properly validates rejection of the legacy
flag and the assertion correctly verifies that the error message suggests using
the canonical `--host` flag instead.
In `@src/app/services/ai.rs`:
- Around line 8-9: Update all parse_optional_timestamp calls throughout the file
to use canonical label names that match the actual field names being parsed.
Replace the deprecated error labels "from" and "to" with "since" and "until"
respectively in all parse_optional_timestamp invocations, ensuring that when
req.since is parsed, the label is "since" (not "from"), and when req.until is
parsed, the label is "until" (not "to"). This change should be applied at all
locations mentioned: lines 8-9, 65-66, 110-111, 139-140, 171-172, 212-213,
331-332, 368-369, and 387-388.
In `@src/app/services/analytics.rs`:
- Around line 5-6: Update the validation error message labels in the
parse_optional_timestamp function calls to match the actual request field names
after the recent rename. Replace the "from" and "to" string literals with
"since" and "until" respectively in the calls at the locations mentioned (around
lines 5-6, 76-77, and 115-116). Additionally, update the error message at line
232 that instructs users to provide "hostname" to instead use "host" to match
the renamed field. This ensures error messages accurately reflect the canonical
field names users should reference.
In `@src/app/services/filters.rs`:
- Around line 89-92: Update the parameter name strings passed to the
parse_optional_timestamp function calls to use canonical names instead of legacy
ones. In the parse_optional_timestamp calls for the timestamp validation,
replace "from" with "since" and "to" with "until" to match the current field
naming convention. Apply the same fix to line 161 and surrounding code,
replacing any legacy parameter names like "hostname", "source_ip", and
"app_name" with their canonical equivalents "host", "source", and "app" in the
error message strings passed to validation functions.
In `@src/app/services/incidents.rs`:
- Around line 55-56: The error message in the incidents.rs file contains
outdated terminology that does not match the current request/CLI vocabulary. In
the error text that starts with "hostname and service cannot be combined:
journal entries are always local and cannot be filtered by remote hostname",
replace both instances of the word "hostname" with "host" to align with the
canonical wording used throughout the codebase.
In `@src/app/services/journal.rs`:
- Around line 97-107: The error messages in the timestamp validation blocks are
using inconsistent field labels compared to the actual request field names. In
the if let Some(from) block, update the error message in the map_err call to say
"invalid \`since\` timestamp" instead of "invalid \`from\` timestamp", and in
the if let Some(to) block, update the corresponding error message to say
"invalid \`until\` timestamp" instead of "invalid \`to\` timestamp". This
ensures the error messages accurately reflect the actual field names in the
request structure.
In `@src/cli/complete_tests.rs`:
- Around line 34-47: The function dynamic_value_degrades_to_ok_without_db
mutates the global CORTEX_DB_PATH environment variable without synchronization,
causing race conditions during parallel test execution. Wrap the environment
mutation logic with a static mutex to serialize access across concurrent tests,
and implement cleanup via a Drop guard so the environment variable is properly
restored even if the test panics. This ensures safe and reliable isolation of
environment state changes.
In `@src/cli/complete.rs`:
- Around line 87-91: The busy_timeout(150ms) call only limits database lock wait
time but does not enforce a timeout on the actual query execution itself,
meaning queries on unlocked databases can run indefinitely and violate the
documented guarantee that completion never blocks. Replace or supplement the
busy_timeout approach with a proper per-query deadline enforcement using
Connection::progress_handler() to register an interrupt callback that fires when
the deadline is exceeded, or alternatively use a separate timeout thread that
calls sqlite3_interrupt() to cancel long-running queries. This ensures the
documented timeout guarantee at the function level is actually enforced
regardless of database lock state.
In `@src/cli/parse_admin_tests.rs`:
- Around line 22-35: The test case for parse_service in the match block is
passing the --until=t1 argument but does not assert on the until field. Add an
assertion after the args.tail assertion to verify that args.until.as_deref()
equals Some("t1"), ensuring that the until parameter mapping is properly tested
and regressions in until handling will be caught.
---
Outside diff comments:
In `@docs/superpowers/plans/2026-05-21-surface-parity.md`:
- Around line 74-82: The TimelineQuery struct (lines 74-82) and PatternsQuery
struct (lines 107-115) in the plan document use outdated field names that do not
match the canonical field names documented in the CLI flags section and
implemented in the actual codebase. Update the TimelineQuery struct to rename
the fields: from to since, to to until, hostname to host, and app_name to app.
Apply the same field name updates to the PatternsQuery struct to ensure
consistency with the canonical field names and the actual implementation in
src/app/models/stats.rs.
In `@README.md`:
- Around line 189-220: The parameter tables in README.md for cortex commands
(search, tail, errors, sessions, correlate) are using outdated parameter names
that do not match the canonical names used in the narrative text and API
examples. Update all parameter tables to replace the old names with the new
canonical names: change `hostname` to `host`, `source_ip` to `source`,
`app_name` to `app`, `from` to `since`, and `to` to `until`. Verify this change
is applied consistently across all command parameter tables in the document to
ensure the documentation accurately reflects the actual CLI interface and wire
protocol.
In `@src/api.rs`:
- Around line 560-569: The TimelineQuery struct and the similar query struct
around line 594-603 silently ignore unknown query parameters instead of
rejecting them, which allows stale/renamed keys like from, to, hostname, and
app_name to be dropped without error and produce incorrect results. Add the
#[serde(deny_unknown_fields)] attribute to both the TimelineQuery struct and the
other query struct to make deserialization fail when unknown fields are
provided, ensuring consistency with stricter handlers in the file and preventing
silent data loss.
In `@src/app/services/logs.rs`:
- Around line 41-42: The error messages in the logs.rs file are referencing
outdated parameter names that no longer match the current handler API. Update
the error message around line 41 that currently references "hostname" to
reference "host" instead, and update the timestamp-related error messages around
lines 317-318 that currently reference "from" and "to" parameters to reference
"since" and "until" respectively to align with the actual parameter names
consumed by the handler.
In `@src/cli/dispatch_ai_tests.rs`:
- Around line 14-18: The test assertion block for the filter-preservation test
is missing assertions for the renamed time filter fields `since` and `until`.
Add assertions after the existing assert_eq calls to verify that both
`req.since` and `req.until` are correctly mapped and preserved, using the same
pattern as the other assertions like those for `query`, `project`, `tool`, and
`limit` to ensure regression detection for these time-based filters.
In `@src/cli/dispatch_surface.rs`:
- Around line 31-33: The comment in dispatch_surface.rs contains outdated
terminology referring to `from` and `to` parameters, but the current
implementation uses `since` and `until` flags instead. Update the comment text
to replace the references to `from/to` with `since/until` to align the
documentation with the actual flag names being used in the code.
In `@src/cli/dispatch_tests.rs`:
- Around line 1485-1487: The test comments and assertion messages in the
dispatch_tests.rs file still reference the old `from/to` parameter nomenclature,
but the underlying request contract has been updated to use `since/until`
instead. Update all comments and assertion messages across the specified ranges
(including the test
`timeline_applies_default_lookback_only_when_from_and_to_both_absent` and other
related assertions) to replace references to `from/to` with the new
`since/until` terminology to maintain alignment with the actual request contract
being tested.
In `@src/cli/help.rs`:
- Around line 904-916: The nested command help path in the nested_lookup block
returns early at the end of the nested branch, preventing execution of the
examples block that follows. To fix this, remove the early return statement from
the nested branch and instead append the registry examples to the output string
before returning. Ensure that the examples rendering logic (which appears to be
in the subsequent lines after the nested block) is incorporated into the nested
branch so that both nested command help and registry examples are rendered
consistently for all help paths.
In `@src/cli/parse_ai_more_tests.rs`:
- Around line 21-63: The test function
parse_ai_similar_and_ask_history_accept_all_filters is missing assertions for
the since and until fields that are being passed to the parse functions. In the
match arm for SimilarIncidents, add assertions to verify args.since and
args.until match the expected values (t0 and t1 respectively). Similarly, in the
match arm for AskHistory, add assertions to verify args.since and args.until are
correctly parsed. This strengthens the regression protection for the renamed
filter contract.
In `@src/cli/parse_ai_tests.rs`:
- Around line 57-127: The test function
parse_ai_search_abuse_and_correlate_accept_equals_forms() validates only a
subset of the fields set by the parser functions. Add assertions for all the
fields being passed in the test inputs to ensure renamed fields are fully
tested. For parse_ai_search, add assertions for the since and until date fields;
for parse_ai_abuse, add assertions for project, tool, since, until, and limit
fields; for parse_ai_correlate, add assertions for all remaining fields
including project, tool, ai_query, log_query, host, app, since, until, and
severity_min that are currently passed but not validated.
In `@src/cli/parse.rs`:
- Around line 12-47: The TOP_LEVEL_COMMANDS constant array is missing the
"completions" command that was added to the parser at line 93, causing
inconsistency between what the parser accepts and what suggestions are offered
for unknown commands at line 96. Add "completions" as a string entry to the
TOP_LEVEL_COMMANDS array to keep it synchronized with all valid parsed commands.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f492f0bd-886f-4245-9ed0-084ad32d286e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lockand included by**/*
📒 Files selected for processing (111)
CHANGELOG.mdCargo.tomlREADME.mddocker-compose.prod.ymldocs/CLI.mddocs/CONFIG.mddocs/api.mddocs/contracts/cli-surface.mddocs/contracts/credentials.mddocs/contracts/forwarder-dropins.mddocs/contracts/http-endpoints.mddocs/mcp/PATTERNS.mddocs/mcp/TESTS.mddocs/mcp/TRANSPORT.mddocs/plans/2026-05-11-mnemo-feature-port.mddocs/sessions/2026-05-21-rag-v1-similar-incidents-ask-history-incident-context.mddocs/sessions/2026-05-29-cli-performance-benchmark-and-fixes.mddocs/sessions/2026-06-02-pr65-cli-ux-help-and-merge.mddocs/sessions/2026-06-12-file-tail-ingest-closeout.mddocs/superpowers/plans/2026-05-21-ai-abuse-incidents.mddocs/superpowers/plans/2026-05-21-rag-historical-incidents.mddocs/superpowers/plans/2026-05-21-surface-parity.mddocs/superpowers/plans/2026-05-22-surface-parity-gap-closure.mddocs/superpowers/plans/2026-05-25-first-class-log-filter-surface.mddocs/superpowers/plans/2026-06-11-file-tail-ingest.mddocs/superpowers/plans/2026-06-15-cortex-cli-query-safety-and-time-parsing.mddocs/superpowers/plans/2026-06-15-cortex-cli-registry-completion-and-rename.mddocs/superpowers/specs/2026-05-16-agent-mode-design.mddocs/superpowers/specs/2026-06-15-cortex-cli-ergonomics-design.mdmcpb/manifest.jsonplugins/cortex/skills/cortex-report/SKILL.mdplugins/cortex/skills/cortex/SKILL.mdscripts/smoke-ai.shscripts/smoke-test-http.shscripts/smoke-test.shserver.jsonsrc/api.rssrc/api_tests.rssrc/app/models/ai_incidents.rssrc/app/models/ai_inventory.rssrc/app/models/ai_sessions.rssrc/app/models/context.rssrc/app/models/core.rssrc/app/models/log_query.rssrc/app/models/rag.rssrc/app/models/stats.rssrc/app/service_tests.rssrc/app/services/ai.rssrc/app/services/analytics.rssrc/app/services/assessment.rssrc/app/services/filters.rssrc/app/services/incidents.rssrc/app/services/journal.rssrc/app/services/logs.rssrc/app/services/rag.rssrc/cli.rssrc/cli/ai_watch.rssrc/cli/args.rssrc/cli/args/ai.rssrc/cli/args/surface.rssrc/cli/commands/apps.rssrc/cli/commands/file_tails.rssrc/cli/commands/host_state.rssrc/cli/complete.rssrc/cli/complete_tests.rssrc/cli/completions.rssrc/cli/completions/_cortex.zshsrc/cli/completions_tests.rssrc/cli/dispatch.rssrc/cli/dispatch_ai.rssrc/cli/dispatch_ai_tests.rssrc/cli/dispatch_surface.rssrc/cli/dispatch_surface_gap.rssrc/cli/dispatch_surface_gap_tests.rssrc/cli/dispatch_surface_tests.rssrc/cli/dispatch_tests.rssrc/cli/help.rssrc/cli/help_tests.rssrc/cli/http_client_tests.rssrc/cli/output_ai_more.rssrc/cli/output_logs.rssrc/cli/parse.rssrc/cli/parse_admin.rssrc/cli/parse_admin_tests.rssrc/cli/parse_ai.rssrc/cli/parse_ai_more.rssrc/cli/parse_ai_more_tests.rssrc/cli/parse_ai_tests.rssrc/cli/parse_logs.rssrc/cli/parse_logs_tests.rssrc/cli/parse_tests.rssrc/cli/run.rssrc/cli_tests.rssrc/db/analytics.rssrc/db/analytics_tests.rssrc/db/models.rssrc/db/queries.rssrc/db/queries_tests.rssrc/file_tail/models.rssrc/file_tail/models_tests.rssrc/file_tail/registry_tests.rssrc/main.rssrc/main_tests.rssrc/mcp.rssrc/mcp/action_flags.rssrc/mcp/actions.rssrc/mcp/actions_tests.rssrc/mcp/schemas.rssrc/mcp/schemas_tests.rssrc/mcp/tools.rssrc/mcp/tools_tests.rs
Comment on lines
+434
to
436
| "\nwarning: candidate scan capped at {} rows; narrow with --project/--tool/--from/--until", | ||
| response.candidate_cap | ||
| ) |
There was a problem hiding this comment.
Use --since here to match the canonical rename.
The warning text still mentions --from, which conflicts with the --since/--until examples above and sends readers back to the deprecated flag name.
♻️ Proposed fix
- narrow with --project/--tool/--from/--until
+ narrow with --project/--tool/--since/--until📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "\nwarning: candidate scan capped at {} rows; narrow with --project/--tool/--from/--until", | |
| response.candidate_cap | |
| ) | |
| "\nwarning: candidate scan capped at {} rows; narrow with --project/--tool/--since/--until", | |
| response.candidate_cap | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-05-21-ai-abuse-incidents.md` around lines 434 -
436, In the warning message string that mentions "candidate scan capped at {}
rows", replace the flag reference `--from` with `--since` to match the canonical
rename and maintain consistency with the documentation examples above. This flag
reference appears in the string being logged with response.candidate_cap.
Comment on lines
+278
to
279
| > The flags keep their current names (`--from`, `--to`, `--received-since`, `--received-until`) in this plan; renaming to `--since/--until` is Plan 2. Here we only normalize their *values* through `parse_time_arg`. | ||
|
|
There was a problem hiding this comment.
Align the plan’s stated flag set with the examples.
The note says Plan 1 keeps --from/--to, but the changed example uses --since. This contradiction makes the implementation target ambiguous.
Also applies to: 287-288
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/superpowers/plans/2026-06-15-cortex-cli-query-safety-and-time-parsing.md`
around lines 278 - 279, The plan description at lines 278-279 states that Plan 1
keeps the current flag names `--from` and `--to`, but the example(s) shown in
the document use `--since` instead. Align the examples with the stated plan by
updating any example commands to use `--from` and `--to` flags instead of
`--since`, or alternatively, if the examples are correct and should use
`--since`, update the plan description to reflect that Plan 1 includes this flag
rename. Ensure consistency between the plan statement and all referenced
examples throughout the document.
Comment on lines
+27
to
+36
| | host | `--host` | `--host` | `hostname` | | ||
| | literal text | `--grep` | (Plan 1) | (n/a) | | ||
| | limit | `-n`, `--limit` | `--limit` | `limit` | | ||
| | min severity | `-s`, `--severity` | `--severity` | `severity` | | ||
| | app | `--app` | `--app-name` | `app_name` | | ||
| | source id | `--source` | `--source-ip` | `source_ip` | | ||
| | app | `--app` | `--app` | `app_name` | | ||
| | source id | `--source` | `--source` | `source_ip` | | ||
| | event-time start | `--since` | `--from` | `from` | | ||
| | event-time end | `--until` | `--to` | `to` | | ||
| | received start | `--received-since` | `--received-from` | `received_from` | | ||
| | received end | `--received-until` | `--received-to` | `received_to` | | ||
| | received start | `--received-since` | `--received-since` | `received_from` | | ||
| | received end | `--received-until` | `--received-until` | `received_to` | |
There was a problem hiding this comment.
Restore legacy spellings in the “Old CLI flag” column.
These rows now show canonical values in the “Old CLI flag” column, which removes the actual rename mapping (e.g., --hostname → --host).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/superpowers/plans/2026-06-15-cortex-cli-registry-completion-and-rename.md`
around lines 27 - 36, The "Old CLI flag" column in the flag mapping table is
showing canonical flag names instead of the actual legacy spellings that were
previously used. Restore the original legacy flag names in this column for each
row (for example, replace `--host` with `--hostname`, and ensure `source id` row
shows the actual old spelling instead of the current canonical name). The goal
is to preserve the complete rename mapping that shows what users were calling
these flags before the standardization.
Comment on lines
+284
to
289
| "--host" => parsed.hostname = Some(flags.value("--host")?), | ||
| "--source" => parsed.source_ip = Some(flags.value("--source")?), | ||
| "--app" => parsed.app_name = Some(flags.value("--app")?), | ||
| "--since" => parsed.from = Some(norm_time(flags.value("--since")?)?), | ||
| "--until" => parsed.to = Some(norm_time(flags.value("--until")?)?), | ||
| ``` |
There was a problem hiding this comment.
The “Before” example no longer shows pre-rename flags.
This block is labeled “Before” but now uses canonical names, so the before/after comparison no longer demonstrates the migration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/superpowers/plans/2026-06-15-cortex-cli-registry-completion-and-rename.md`
around lines 284 - 289, The "Before" code block in the documentation is showing
the current canonical flag names (--host, --source, --app, --since, --until)
instead of the original pre-rename flag names. To properly demonstrate the
migration, replace the flag names in this "Before" example with their original
deprecated names that existed before the rename, so that the before/after
comparison clearly shows what was changed during the flag naming refactoring.
…line + help/api hardening Address CodeRabbit review on #80: - service-layer error messages used stale from/to/hostname/source_ip/app_name vocabulary (ai/analytics/filters/incidents/journal/logs/rag) -> since/until/host/source/app - api.rs: TimelineQuery/PatternsQuery now deny_unknown_fields (was silently dropping stale keys); fixed stale from/to comment - help.rs: nested-command help now renders the registry Examples block (was returning early); unified both help paths - complete.rs: enforce a real per-query deadline via progress_handler (busy_timeout only bounds lock-wait); rusqlite 'hooks' feature added - complete_tests.rs: serialize CORTEX_DB_PATH mutation + restore via Drop guard - parse.rs: TOP_LEVEL_COMMANDS includes 'completions' - README: cortex search input param table uses canonical names (output/stored-field table left unchanged); CHANGELOG compare-link footer refreshed - test assertions: dispatch_ai/parse_admin assert since/until mapping
…ntil) The Plan 2 rename migrated smoke-test.sh but missed tests/test_live.sh (the CI MCP Integration harness). With the renamed deny_unknown_fields handlers, its stale args were rejected: file_tails add used "hostname", search used "app_name", and GET /api/incident-context used ?from=&to= — causing the 3 CI failures. Now host/app/since/until.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #79 (base =
claude/eager-kowalevski-6f9fe7). Retarget tomainonce #79 merges.Summary
Plan 2 of the CLI ergonomics effort — the registry-driven completion centerpiece plus the full canonical flag/argument rename.
ACTION_SPECSflag/example metadata — one registry now carries per-action flags (with completionvalue_kind) and copy-paste examples; the CLI parser, completion, and help all derive from it.cortex completions zshinstalls a tab-completion function.cortex __completeemits candidates: command names + descriptions, per-command flags + help, fixed enums + relative-time hints, and live values (hostnames/apps/source IDs) via a bounded read-only DB query cached ~60s with a 150 ms timeout that degrades silently to static.--hostname→--host,--source-ip→--source,--app-name→--app,--from→--since,--to→--until,--received-from→--received-since,--received-to→--received-until(+-s/-nshort forms). The MCP-arg rename was done by renaming the request-arg domain fields (so wire key = field name, no serde aliases); response/output field names (LogEntry.hostname, etc.) are unchanged — the output contract is unaffected. Migrated all 9 skills + docs + smoke tests.--helpfor every command.Version: 1.25.0 → 1.26.0 (feat → minor).
Test Plan
cargo test -p cortex— 1395 lib + 434 bins + integration suites, 0 failurescargo clippy --all-targets -- -D warnings— cleancargo fmt --check— cleancortex __complete actions|flags search|value --severity|value --host(live hostnames),cortex completions zsh,cortex search --help(Examples block)🤖 Generated with Claude Code
Summary by cubic
Adds registry-driven shell completion and finalizes the canonical flag/arg rename across CLI, REST, and MCP for a consistent, discoverable experience. Also hardens help, API parsing, and completion deadlines, updates docs, and fixes lingering strings to match the new names.
New Features
cortex completions zshinstalls zsh completion;cortex __completeemits commands, flags, and values.--helpnow includes copy‑paste examples sourced from a single action registry.--hostname→--host,--source-ip→--source,--app-name→--app,--from/--to→--since/--until,--received-from/--received-to→--received-since/--received-until(response field names unchanged).Bug Fixes
ai incident-context, scan‑capped hints, adminfile_tailshelp, andsimilar_incidentsexamples (kept response fields ashostname/app_name).rusqlitehooks).timeline/patternsqueries now reject unknown fields and require canonical keys (since/until,host,app).jqvariables in the search smoke test.Written for commit 6d02d57. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Changed
--hostname→--host,--app-name→--app,--source-ip→--source,--from/--to→--since/--until)