Remove logs cached-logs alias - #60933
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The implementation, tests, and documentation consistently remove the alias without changing wildcard cache functionality.
Pull request overview
Removes the redundant --cached-logs alias while retaining wildcard caching through --cached-jsonl.
Changes:
- Simplifies CLI flag registration and option loading.
- Updates tests, help text, documentation, and ADR wording.
File summaries
| File | Description |
|---|---|
pkg/cli/logs_command.go |
Removes alias handling and updates help. |
pkg/cli/logs_command_test.go |
Verifies the alias is absent. |
docs/src/content/docs/setup/cli.md |
Updates CLI reference documentation. |
docs/adr/60702-support-wildcard-cached-logs-files.md |
Aligns the draft decision with the simplified interface. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "ab.chatgpt.com"See Network Configuration for more information.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
|
Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design and /grill-with-docs — requesting changes on process gaps around a released-flag removal.
📋 Key Themes & Highlights
Key Themes
- Missing changeset:
--cached-logswas a released alias (shipped with the wildcard-cache feature in PR #60702, called out as live in the 2026-09-14 weekly blog post). Removing a flag is always a breaking change per this repo's Breaking CLI Rules, which requires amajorchangeset with migration guidance. None was added in this diff. - ADR rewritten in place: The PR edits ADR-60702's body text to erase the original alias decision instead of superseding it, which conflicts with the repo's "immutable once accepted" ADR philosophy.
Positive Highlights
- ✅ Code, tests, help text, and docs are all kept consistent — no stray
--cached-logsreferences remain in the diff. - ✅ Clear, focused change with a good rationale in the PR description.
See inline comments for specifics. @copilot please address the review comments above.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 140.2 AIC · ⌖ 15.2 AIC · ⊞ 10.4K
Comment /matt to run again
| @@ -642,7 +642,6 @@ func addLogsCommandFlags(logsCmd *cobra.Command, validArtifactSets string) { | |||
| logsCmd.Flags().String("format", "", "Output format: console (decorated tables), tsv (tab-separated), pretty (cross-run report), markdown (cross-run Markdown). Default: compact agent-optimized output") | |||
There was a problem hiding this comment.
[/codebase-design] This removes a flag (--cached-logs) that shipped in the released wildcard-cache feature (PR #60702, referenced in the 2026-09-14 weekly blog post as already live). Per this repo's Breaking CLI Rules, removing a flag always requires a major changeset with migration guidance — I don't see a new file added under .changeset/ in this diff.
💡 Suggested changeset
Add .changeset/major-remove-cached-logs-alias.md:
---
"gh-aw": major
---
Remove the `--cached-logs` alias for `--cached-jsonl` on `gh aw logs`.
**Migration:** replace `--cached-logs` with `--cached-jsonl` (including wildcard prefixes such as `--cached-jsonl 'logs-*'`).Without this, the release script won't record the breaking change or generate migration notes in CHANGELOG.md.
@copilot please address this.
| --- | ||
|
|
||
| ### Context | ||
|
|
There was a problem hiding this comment.
[/grill-with-docs] This edits the body of an existing ADR to erase its original decision (adding the --cached-logs alias) rather than superseding it. The repo's ADR philosophy states ADRs are "immutable once accepted — never deleted; superseded ones marked 'Superseded by ADR-XXXX'". Since this PR reverses part of ADR-60702's decision, editing the original text in place hides the historical record of why the alias existed and was later removed.
💡 Suggested approach
Either:
- Leave ADR-60702 as the historical record of the original (now partially reversed) decision and add a new ADR (e.g.
docs/adr/NNNN-remove-cached-logs-alias.md) documenting this removal, updating ADR-60702's status line toSuperseded by ADR-NNNNfor the alias-related content, or - If ADR-60702 is still
Draftand was never actually accepted/released, downgrade/mark itDeprecatedinstead of silently rewriting its narrative.
This keeps the decision trail intact for future readers trying to understand why --cached-logs briefly existed.
@copilot please address this.
|
@copilot This PR looks blocked on review follow-up rather than checks. Please address the requested changes: add the required major changeset for removing the released Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed in dd5aa64:
Branch is up to date with |
|
🎉 This pull request is included in a new release. Release: |
The
logscommand should expose cached JSONL reuse only through--cached-jsonl. This removes the duplicate--cached-logsalias from the CLI and related docs.CLI
--cached-logsflag registration.--cached-jsonl.--cached-jsonl.Tests
--cached-logsis no longer registered.Docs
--cached-logsfrom the CLI option list.--cached-jsonlonly.Run: https://github.com/github/gh-aw/actions/runs/34906446596
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.laiyagushi.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
Run URL: https://github.com/github/gh-aw/actions/runs/34908198413
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.laiyagushi.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.