feat(cfl)!: explicit search scope, strict limits, implement page view --full - #462
feat(cfl)!: explicit search scope, strict limits, implement page view --full#462rianjs wants to merge 6 commits into
Conversation
![rianjs-bot[bot]](https://avatars.githubusercontent.com/u/7378790?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: a1c76779d6e8
Profile: claude-rianjs-bot - Posting as: rianjs-bot[bot]
Summary
| Reviewer | Findings |
|---|---|
| go:implementation-tests | 2 |
| policies:conventions | 2 |
| structure:repo-health | 0 |
| documentation:docs | 2 |
go:implementation-tests (2 findings)
Minor - tools/cfl/internal/cmd/page/view_test.go:164
The PR adds both
--full --content-onlyand--full --webrejections in the cobraArgsvalidator (view.go:88-93), which means both run beforePersistentPreRunEloads config.TestView_FullContentOnlyFailsBeforeConfigpins the pre-config contract for the--full --content-onlycase viarootCmd.Execute()with a missing config path. There is no analogous integration test for--full --web. The behavior is correct —Argsruns beforePersistentPreRunEand therefore before config load — but only half the new contract is pinned by a cobra-level pre-config test. Add a parallelTestView_FullWebFailsBeforeConfigthat passes--full --webtorootCmd.Execute()with a missing config path and asserts both the error text and that no config error appears, matching the pattern already established at line 164.
Nits - tools/cfl/api/search_test.go:118
TestClient_Search_RawCQLConflictsis the only new test in this file that omitst.Parallel()at both the outer level and in each subtest. The subtests use only an empty&Client{}(no network), so there is no correctness concern, but the inconsistency with every other test in the file (TestClient_Search_Success,TestClient_Search_EmptyResults,TestClient_Search_Pagination, theTestBuildCQL_*suite, etc.) makes the omission look unintentional. Addt.Parallel()to the outer function and to each subtest closure.
policies:conventions (2 findings)
Minor - tools/cfl/internal/cmd/OUTPUT_SPEC.md:209
The flag-specific behavior section documents
--full's incompatibilities but omits the--content-only+--webrejection thatview.goenforces (lines 127-130). The output contract should be self-consistent: any flag interaction that the code rejects before network access belongs here.Add one bullet under the existing
--fullincompatibility line:- `--content-only` is incompatible with `--web` (web opens a browser and emits no body text).
Minor - tools/cfl/README.md:286
--fullis registered as a rootPersistentFlagand applies to six commands (page list,page view,space list,space view,attachment list,search), but it is absent from the Global Flags table (lines 182-191). A user reading that section has no way to discover the flag; they would only find it buried as a prose note undercfl page view. Add a row to the Global Flags table, e.g.:| `--full` | | `false` | Inspection-oriented output; supported on view, list, and search commands |This keeps the table the authoritative surface for flags that are globally registered, consistent with how
--no-colorand--outputare documented there.
documentation:docs (2 findings)
Major - skills/Confluence/CliReference.md:27
The Global Flags table lists
jsonas a valid value for-o/--output FORMAT(table(default),json,plain), but resource-o jsonwas removed in #392.OUTPUT_SPEC.mdnow mandates thatinvalid output format: "json"is a hard error, andtools/cfl/README.md(also changed in this PR) correctly shows onlytableandplain. Two additional stale spots in this same file repeat the same incorrect claim: line 51 listscfl page view PAGE_ID -o jsonas a working command with description "Full JSON output", and lines 207-209 in the Output section tell users to "Use-o jsonfor machine-readable output" for scripting. All three will produce an immediate CLI error. Fix: removejsonfrom the flag description on line 27, drop the-o jsonrow from the page-view table, and replace the scripting guidance with-o plain(TSV) or a note that JSON output was removed and the alternative is parsing the stable plain/table columns.
Minor - tools/cfl/README.md:286
go install github.com/open-cli-collective/confluence-cli/cmd/cfl@latestpoints at the old archived single-tool repo, which no longer receives releases. Anyone following this instruction installs a stale binary and will miss all changes shipped after the consolidation. The correct install path in the monorepo isgithub.com/open-cli-collective/atlassian-cli/tools/cfl/cmd/cfl@latest(assuming the submodule is independently versioned), or ifgo installis not supported for this workspace submodule, the snippet should be replaced with the Homebrew or binary-download approach with a note that source installs should usemake build-cflfrom a clone.
Reviewer Coverage
| Reviewer | Status | Inspected | Skipped | Constraints |
|---|---|---|---|---|
| go:implementation-tests | complete_broad | tools/cfl/api/pages.go, tools/cfl/api/pages_test.go, tools/cfl/api/search.go, tools/cfl/api/search_test.go, tools/cfl/internal/cmd/attachment/list.go, tools/cfl/internal/cmd/attachment/list_test.go, tools/cfl/internal/cmd/page/view.go, tools/cfl/internal/cmd/page/view_test.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/cmd/root/root_test.go, tools/cfl/internal/cmd/search/search.go, tools/cfl/internal/cmd/search/search_test.go, tools/cfl/internal/pageview/projection.go, tools/cfl/internal/present/detail.go | unavailable | unavailable |
| policies:conventions | complete_broad | README.md, docs/ARTIFACT_CONTRACT.md, tools/cfl/README.md, tools/cfl/internal/cmd/OUTPUT_SPEC.md, tools/cfl/internal/cmd/attachment/list.go, tools/cfl/internal/cmd/page/view.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/cmd/search/search.go | unavailable | Shared cli-common docs and .github automation were not available locally; reviewed against repo-local docs only. |
| structure:repo-health | complete_broad | tools/cfl/api/pages.go, tools/cfl/api/search.go, tools/cfl/internal/cmd/attachment/list.go, tools/cfl/internal/cmd/page/view.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/cmd/search/search.go, tools/cfl/internal/pageview/projection.go, tools/cfl/internal/present/detail.go | unavailable | unavailable |
| documentation:docs | complete_broad | README.md, docs/ARTIFACT_CONTRACT.md, skills/Confluence/CliReference.md, skills/Confluence/SKILL.md, skills/Confluence/Workflows/ManageAttachments.md, skills/Confluence/Workflows/SearchPages.md, skills/Confluence/Workflows/ViewPage.md, tools/cfl/CHANGELOG.md, tools/cfl/README.md, tools/cfl/internal/cmd/OUTPUT_SPEC.md | unavailable | unavailable |
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 5m 51s | ~$2.03 (est.) | claude-sonnet-4-6 | cr 0.10.259
| Field | Value |
|---|---|
| Model | claude-sonnet-4-6 |
| Reviewers | go:implementation-tests, policies:conventions, structure:repo-health, documentation:docs |
| Engine | claude_cli · claude-sonnet-4-6 |
| Reviewed by | cr · rianjs-bot[bot] |
| Duration | 5m 51s wall · 12m 26s compute |
| Cost | ~$2.03 (est.) |
| Tokens | 31 in / 42.6k out |
Per-workstream usage
| Workstream | Model | In | Out | Cache read | Cache create | Cost | Duration |
|---|---|---|---|---|---|---|---|
| orchestrator-selection | claude-sonnet-4-6 | 4 | 3.0k | 18.4k | 20.7k | ~$0.13 (est.) | 53s |
| go:implementation-tests | claude-sonnet-4-6 | 6 | 15.5k | 151.0k | 113.9k | ~$0.70 (est.) | 4m 18s |
| policies:conventions | claude-sonnet-4-6 | 5 | 7.4k | 55.3k | 54.6k | ~$0.33 (est.) | 2m 03s |
| structure:repo-health | claude-sonnet-4-6 | 7 | 7.7k | 145.5k | 49.7k | ~$0.35 (est.) | 2m 22s |
| documentation:docs | claude-sonnet-4-6 | 5 | 8.0k | 56.0k | 60.0k | ~$0.36 (est.) | 2m 18s |
| orchestrator-rollup | claude-sonnet-4-6 | 4 | 1.1k | 18.4k | 37.7k | ~$0.16 (est.) | 30s |
| testutil.Equal(t, "", rootOpts.Stdout.(*bytes.Buffer).String()) | ||
| } | ||
|
|
||
| func TestView_FullContentOnlyFailsBeforeConfig(t *testing.T) { |
There was a problem hiding this comment.
The PR adds both --full --content-only and --full --web rejections in the cobra Args validator (view.go:88-93), which means both run before PersistentPreRunE loads config. TestView_FullContentOnlyFailsBeforeConfig pins the pre-config contract for the --full --content-only case via rootCmd.Execute() with a missing config path. There is no analogous integration test for --full --web. The behavior is correct — Args runs before PersistentPreRunE and therefore before config load — but only half the new contract is pinned by a cobra-level pre-config test. Add a parallel TestView_FullWebFailsBeforeConfig that passes --full --web to rootCmd.Execute() with a missing config path and asserts both the error text and that no config error appears, matching the pattern already established at line 164.
Reply inline to this comment.
There was a problem hiding this comment.
Added TestView_FullWebFailsBeforeConfig mirroring the content-only pre-config test: missing config path, asserts the --full/--web error text and no config error.
There was a problem hiding this comment.
Thanks — the parallel pre-config regression test covers the --full --web contract as requested.
Summary:
Added cobra-level coverage confirming --full --web fails before config loading.
| @@ -286,6 +286,7 @@ View a Confluence page. **Content is displayed as markdown by default.** | |||
|
|
|||
There was a problem hiding this comment.
File-level note: tools/cfl/README.md
--full is registered as a root PersistentFlag and applies to six commands (page list, page view, space list, space view, attachment list, search), but it is absent from the Global Flags table (lines 182-191). A user reading that section has no way to discover the flag; they would only find it buried as a prose note under cfl page view. Add a row to the Global Flags table, e.g.:
| `--full` | | `false` | Inspection-oriented output; supported on view, list, and search commands |
This keeps the table the authoritative surface for flags that are globally registered, consistent with how --no-color and --output are documented there.
Reply inline to this comment.
There was a problem hiding this comment.
Added a --full row to the Global Flags table noting view/list/search support and rejection elsewhere.
There was a problem hiding this comment.
Confirmed—this makes --full discoverable from the Global Flags table and documents its supported scope. Thanks.
Summary:
--full is now documented in the Global Flags table, including supported commands and rejection elsewhere.
|
|
||
| Flag-specific behavior: | ||
| - `--content-only` emits only the body and implies untruncated output. | ||
| - `--full` is incompatible with `--content-only` and `--web` because both discard metadata. |
There was a problem hiding this comment.
The flag-specific behavior section documents --full's incompatibilities but omits the --content-only + --web rejection that view.go enforces (lines 127-130). The output contract should be self-consistent: any flag interaction that the code rejects before network access belongs here.
Add one bullet under the existing --full incompatibility line:
- `--content-only` is incompatible with `--web` (web opens a browser and emits no body text).
Reply inline to this comment.
There was a problem hiding this comment.
Added the --content-only/--web incompatibility bullet to the flag-specific behavior list.
There was a problem hiding this comment.
Confirmed—the output spec now documents the --content-only and --web incompatibility.
Summary:
Documented the rejected --content-only + --web flag combination in the output contract.
| } | ||
|
|
||
| func TestClient_Search_RawCQLConflicts(t *testing.T) { | ||
| tests := []struct { |
There was a problem hiding this comment.
TestClient_Search_RawCQLConflicts is the only new test in this file that omits t.Parallel() at both the outer level and in each subtest. The subtests use only an empty &Client{} (no network), so there is no correctness concern, but the inconsistency with every other test in the file (TestClient_Search_Success, TestClient_Search_EmptyResults, TestClient_Search_Pagination, the TestBuildCQL_* suite, etc.) makes the omission look unintentional. Add t.Parallel() to the outer function and to each subtest closure.
Reply inline to this comment.
There was a problem hiding this comment.
Added t.Parallel() to the outer test and each subtest closure.
There was a problem hiding this comment.
Thanks for addressing this.
Summary:
Added t.Parallel() to the outer test and each subtest closure.
| @@ -286,6 +286,7 @@ View a Confluence page. **Content is displayed as markdown by default.** | |||
|
|
|||
There was a problem hiding this comment.
File-level note: tools/cfl/README.md
go install github.com/open-cli-collective/confluence-cli/cmd/cfl@latest points at the old archived single-tool repo, which no longer receives releases. Anyone following this instruction installs a stale binary and will miss all changes shipped after the consolidation. The correct install path in the monorepo is github.com/open-cli-collective/atlassian-cli/tools/cfl/cmd/cfl@latest (assuming the submodule is independently versioned), or if go install is not supported for this workspace submodule, the snippet should be replaced with the Homebrew or binary-download approach with a note that source installs should use make build-cfl from a clone.
Reply inline to this comment.
There was a problem hiding this comment.
The module path is still github.com/open-cli-collective/confluence-cli, so a monorepo go install path would not resolve; replaced the snippet with clone + make build-cfl and a note steering to Homebrew, per your alternative.
There was a problem hiding this comment.
Thanks for confirming. Replacing the stale go install instruction with the clone + make build-cfl path and steering users to Homebrew addresses the issue.
Summary:
Replaced the stale archived-repository go install instruction with supported clone-and-build guidance and a Homebrew recommendation.
| @@ -25,7 +25,7 @@ cfl config test | |||
| | Flag | Description | | |||
| |------|-------------| | |||
| | `-o, --output FORMAT` | Output format (see SKILL.md "Output Representation and Format"): `table` (default), `json`, `plain` | | |||
There was a problem hiding this comment.
The Global Flags table lists json as a valid value for -o/--output FORMAT (table (default), json, plain), but resource -o json was removed in #392. OUTPUT_SPEC.md now mandates that invalid output format: "json" is a hard error, and tools/cfl/README.md (also changed in this PR) correctly shows only table and plain. Two additional stale spots in this same file repeat the same incorrect claim: line 51 lists cfl page view PAGE_ID -o json as a working command with description "Full JSON output", and lines 207-209 in the Output section tell users to "Use -o json for machine-readable output" for scripting. All three will produce an immediate CLI error. Fix: remove json from the flag description on line 27, drop the -o json row from the page-view table, and replace the scripting guidance with -o plain (TSV) or a note that JSON output was removed and the alternative is parsing the stable plain/table columns.
Reply inline to this comment.
There was a problem hiding this comment.
Removed json from the flag description, dropped the -o json row, and replaced the scripting guidance with -o plain (TSV) plus a pointer to --body-format adf|xhtml for exact bodies.
There was a problem hiding this comment.
Thanks—this addresses the stale JSON-output guidance.
Summary:
Removed obsolete -o json documentation and updated scripting guidance to use plain TSV output with body-format options for exact bodies.
# Conflicts: # skills/Confluence/CliReference.md # skills/Confluence/SKILL.md
# Conflicts: # skills/Confluence/SKILL.md
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: 541e0ce38fae
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]
Summary
| Reviewer | Findings |
|---|---|
| go:implementation-tests | 0 |
| policies:conventions | 0 |
| documentation:docs | 2 |
| structure:repo-health | 0 |
documentation:docs (2 findings)
Major - tools/cfl/README.md:134
The newly revised source-build instructions state that Go 1.24 is sufficient, but both
go.workandtools/cfl/go.modrequire Go 1.26. A reader following this path with Go 1.24 or 1.25 will fail before building. Update the prerequisite to Go 1.26 or later.
Major - tools/cfl/README.md:131
make build-cflwrites the executable tobin/cfl; it neither installs it nor adds it toPATH. The immediately following quick-start commands invokecfl, so a source user will get command-not-found. Document invoking./bin/cflor add an explicit install/PATH step.
Reviewer Coverage
| Reviewer | Status | Inspected | Skipped | Constraints |
|---|---|---|---|---|
| go:implementation-tests | complete_broad | tools/cfl/api/pages.go, tools/cfl/api/pages_test.go, tools/cfl/api/search.go, tools/cfl/api/search_test.go, tools/cfl/internal/cmd/attachment/list.go, tools/cfl/internal/cmd/attachment/list_test.go, tools/cfl/internal/cmd/page/view.go, tools/cfl/internal/cmd/page/view_test.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/cmd/root/root_test.go, tools/cfl/internal/cmd/search/search.go, tools/cfl/internal/cmd/search/search_test.go, tools/cfl/internal/pageview/projection.go, tools/cfl/internal/present/detail.go | unavailable | Targeted Go tests could not fully run because the sandbox denies httptest local TCP listeners; root, pageview, and present packages passed before/alongside that restriction. |
| policies:conventions | complete_broad | README.md, docs/ARTIFACT_CONTRACT.md, tools/cfl/README.md, tools/cfl/internal/cmd/OUTPUT_SPEC.md, tools/cfl/internal/cmd/attachment/list.go, tools/cfl/internal/cmd/page/view.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/cmd/search/search.go | unavailable | Focused Go tests were constrained by the sandbox: root tests passed with CGO disabled, while other packages require loopback httptest listeners that the environment disallows.; Shared cli-common standards were not present locally, so review used the available repo-local development and output-contract guidance. |
| documentation:docs | complete_broad | README.md, docs/ARTIFACT_CONTRACT.md, skills/Confluence/CliReference.md, skills/Confluence/SKILL.md, skills/Confluence/Workflows/ManageAttachments.md, skills/Confluence/Workflows/SearchPages.md, skills/Confluence/Workflows/ViewPage.md, tools/cfl/CHANGELOG.md, tools/cfl/README.md, tools/cfl/internal/cmd/OUTPUT_SPEC.md | unavailable | Documentation-only review; validated changed claims against local command and build implementation. Did not perform live Confluence or external-link checks. |
| structure:repo-health | complete_broad | tools/cfl/api/pages.go, tools/cfl/api/search.go, tools/cfl/internal/cmd/attachment/list.go, tools/cfl/internal/cmd/page/view.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/cmd/search/search.go, tools/cfl/internal/pageview/projection.go, tools/cfl/internal/present/detail.go | unavailable | Targeted Go tests could not complete because the sandbox disallows httptest from binding loopback ports; source and focused test coverage were inspected. |
6 PR discussion threads considered. 6 summarized; 6 resolved.
Completed in 3m 31s | unavailable | gpt-5.6-terra | cr 0.10.260
| Field | Value |
|---|---|
| Model | gpt-5.6-terra |
| Reviewers | go:implementation-tests, policies:conventions, documentation:docs, structure:repo-health |
| Engine | codex_cli · gpt-5.6-terra |
| Reviewed by | cr · rianjs-bot[bot] |
| Duration | 3m 31s wall · 6m 32s compute |
| Cost | unavailable |
| Tokens | 1.6M in / 16.2k out |
Per-workstream usage
| Workstream | Model | In | Out | Cache read | Cache create | Cost | Duration |
|---|---|---|---|---|---|---|---|
| orchestrator-selection | gpt-5.6-terra | 16.9k | 641 | 10.5k | unavailable | unavailable | 14s |
| go:implementation-tests | gpt-5.6-terra | 529.9k | 4.7k | 455.2k | unavailable | unavailable | 1m 52s |
| policies:conventions | gpt-5.6-terra | 350.9k | 3.1k | 294.1k | unavailable | unavailable | 1m 24s |
| documentation:docs | gpt-5.6-terra | 306.4k | 4.1k | 254.7k | unavailable | unavailable | 1m 27s |
| structure:repo-health | gpt-5.6-terra | 329.2k | 3.0k | 283.4k | unavailable | unavailable | 1m 24s |
| orchestrator-rollup | gpt-5.6-terra | 36.9k | 766 | 27.1k | unavailable | unavailable | 7s |
| ``` | ||
|
|
||
| Requires Go 1.24 or later. | ||
| Requires Go 1.24 or later. (`go install` of the old `confluence-cli` module path installs a stale pre-monorepo binary; use Homebrew or a clone instead.) |
There was a problem hiding this comment.
The newly revised source-build instructions state that Go 1.24 is sufficient, but both go.work and tools/cfl/go.mod require Go 1.26. A reader following this path with Go 1.24 or 1.25 will fail before building. Update the prerequisite to Go 1.26 or later.
Reply inline to this comment.
There was a problem hiding this comment.
Fixed — prerequisite now says Go 1.26 or later, matching go.work and tools/cfl/go.mod.
There was a problem hiding this comment.
Confirmed — the prerequisite now matches the Go 1.26 requirement. Thanks for fixing this.
Summary:
Source-build prerequisite updated to Go 1.26 or later, matching go.work and tools/cfl/go.mod.
| go install github.com/open-cli-collective/confluence-cli/cmd/cfl@latest | ||
| git clone https://github.com/open-cli-collective/atlassian-cli.git | ||
| cd atlassian-cli | ||
| make build-cfl |
There was a problem hiding this comment.
make build-cfl writes the executable to bin/cfl; it neither installs it nor adds it to PATH. The immediately following quick-start commands invoke cfl, so a source user will get command-not-found. Document invoking ./bin/cfl or add an explicit install/PATH step.
Reply inline to this comment.
There was a problem hiding this comment.
Added ./bin/cfl --version to the snippet and documented copying bin/cfl onto PATH.
There was a problem hiding this comment.
Thanks—this addresses the issue by making the initial invocation explicit and documenting how to put cfl on PATH.
Summary:
Quick-start command availability is now documented.
# Conflicts: # tools/cfl/internal/cmd/OUTPUT_SPEC.md # tools/cfl/internal/cmd/page/view.go # tools/cfl/internal/cmd/root/root.go
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: d87e0a92d05e
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]
Summary
| Reviewer | Findings |
|---|---|
| go:implementation-tests | 1 |
| policies:conventions | 1 |
| documentation:docs | 2 |
| structure:repo-health | 0 |
go:implementation-tests (1 finding)
Minor - tools/cfl/internal/present/detail.go:101
--fullunconditionally renders Parent ID, Created At, and Author ID, converting unavailable values to-. These are optional API fields (and the PR promises author ID only where available); this also conflicts with the detail-output convention to omit absent optional fields. Root pages and responses without author/creation metadata therefore emit misleading empty labels. Append each full field only when its value is present (and timestamp non-zero), and add a presenter/view test covering omitted metadata.
policies:conventions (1 finding)
Minor - tools/cfl/README.md:194
This wording implies that every
vieworlistcommand supports--full, but the root allowlist only permitspage list,page view,space list,space view,attachment list, andsearch; for example,cfl page history list --fullis rejected. Enumerate the supported command paths here (or link to the authoritative output spec) so the public flag contract does not overpromise.
documentation:docs (2 findings)
Major - skills/Confluence/Workflows/SearchPages.md:12
This workflow still directs agents to use
-o jsonand parse a JSON envelope, but the current root command rejects resource JSON output (-oaccepts onlytableandplain). Any scripted search following this guidance fails before executing. Replace the JSON example and theAlways use -o jsonadvice with the supported-o plainTSV output and identify the ID column for extraction.
Minor - skills/Confluence/SKILL.md:52
The new generic scripting advice says to parse
ID: <id>lines, but list and search commands emit tables (or TSV with-o plain) and do not produceID:detail lines. This can mislead agents extracting IDs from search results. Qualify this as detail-command output, and direct list/search scripting to-o plainTSV with the first column as the ID.
Reviewer Coverage
| Reviewer | Status | Inspected | Skipped | Constraints |
|---|---|---|---|---|
| go:implementation-tests | complete_broad | tools/cfl/api/pages.go, tools/cfl/api/pages_test.go, tools/cfl/api/search.go, tools/cfl/api/search_test.go, tools/cfl/internal/cmd/attachment/list.go, tools/cfl/internal/cmd/attachment/list_test.go, tools/cfl/internal/cmd/page/view.go, tools/cfl/internal/cmd/page/view_test.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/cmd/root/root_test.go, tools/cfl/internal/cmd/search/search.go, tools/cfl/internal/cmd/search/search_test.go, tools/cfl/internal/pageview/projection.go, tools/cfl/internal/present/detail.go | unavailable | Focused on the assigned Go implementation and behavioral tests. The targeted go test invocation was blocked by the sandbox (httptest cannot bind a listener; several packages also fail to locate a clang path containing spaces). |
| policies:conventions | complete_broad | README.md, docs/ARTIFACT_CONTRACT.md, tools/cfl/README.md, tools/cfl/internal/cmd/OUTPUT_SPEC.md, tools/cfl/internal/cmd/attachment/list.go, tools/cfl/internal/cmd/page/view.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/cmd/search/search.go | unavailable | Focused Go tests could not build because the toolchain's clang invocation fails on the workspace path containing spaces.; The optional local cli-common and shared automation documentation copies were not present. |
| documentation:docs | complete_broad | README.md, docs/ARTIFACT_CONTRACT.md, skills/Confluence/CliReference.md, skills/Confluence/SKILL.md, skills/Confluence/Workflows/ManageAttachments.md, skills/Confluence/Workflows/SearchPages.md, skills/Confluence/Workflows/ViewPage.md, tools/cfl/CHANGELOG.md, tools/cfl/README.md, tools/cfl/internal/cmd/OUTPUT_SPEC.md | unavailable | Documentation-only review; implementation was inspected to verify documented command behavior. External URLs were not live-checked. |
| structure:repo-health | complete_broad | tools/cfl/api/pages.go, tools/cfl/api/search.go, tools/cfl/internal/cmd/attachment/list.go, tools/cfl/internal/cmd/page/view.go, tools/cfl/internal/cmd/root/root.go, tools/cfl/internal/cmd/search/search.go, tools/cfl/internal/pageview/projection.go, tools/cfl/internal/present/detail.go | unavailable | Focused on assigned cross-layer files and their immediate contracts/tests. Targeted Go tests could not complete because the sandbox toolchain failed to resolve a workspace path containing spaces. |
2 PR discussion threads considered. 2 summarized; 2 resolved.
Completed in 2m 43s | unavailable | gpt-5.6-terra | cr 0.10.260
| Field | Value |
|---|---|
| Model | gpt-5.6-terra |
| Reviewers | go:implementation-tests, policies:conventions, documentation:docs, structure:repo-health |
| Engine | codex_cli · gpt-5.6-terra |
| Reviewed by | cr · rianjs-bot[bot] |
| Duration | 2m 43s wall · 6m 31s compute |
| Cost | unavailable |
| Tokens | 1.8M in / 17.9k out |
Per-workstream usage
| Workstream | Model | In | Out | Cache read | Cache create | Cost | Duration |
|---|---|---|---|---|---|---|---|
| orchestrator-selection | gpt-5.6-terra | 62.1k | 1.3k | 46.8k | unavailable | unavailable | 13s |
| go:implementation-tests | gpt-5.6-terra | 474.6k | 4.0k | 397.3k | unavailable | unavailable | 1m 42s |
| policies:conventions | gpt-5.6-terra | 443.2k | 3.9k | 386.0k | unavailable | unavailable | 1m 35s |
| documentation:docs | gpt-5.6-terra | 322.5k | 3.4k | 264.2k | unavailable | unavailable | 1m 17s |
| structure:repo-health | gpt-5.6-terra | 390.0k | 3.7k | 340.5k | unavailable | unavailable | 1m 35s |
| orchestrator-rollup | gpt-5.6-terra | 90.9k | 1.6k | 71.7k | unavailable | unavailable | 7s |
| - `full` (`--full`) — inspection-oriented additions on page/space views and supported list/search commands | ||
| - **Page body format** — `--body-format markdown|adf|xhtml` on page view/create/edit. Omission means Markdown; ADF and XHTML are exact. | ||
| - **Output format** — how it's rendered: `table` (default), `plain` (`-o plain`). For scripted extraction, parse `ID: <id>` lines from command output. | ||
| - **Output format** — how it's rendered: `table` (default) or `plain` (`-o plain`). For scripted extraction, parse `ID: <id>` lines from command output. |
There was a problem hiding this comment.
The new generic scripting advice says to parse ID: <id> lines, but list and search commands emit tables (or TSV with -o plain) and do not produce ID: detail lines. This can mislead agents extracting IDs from search results. Qualify this as detail-command output, and direct list/search scripting to -o plain TSV with the first column as the ID.
Reply inline to this comment.
There was a problem hiding this comment.
Qualified the ID: guidance as detail/mutation output; list/search scripting now points at -o plain TSV with the first column as the ID.
| | `--config` | `-c` | `~/.config/cfl/config.yml` | Path to config file | | ||
| | `--output` | `-o` | `table` | Output format: `table`, `plain` | | ||
| | `--no-color` | | `false` | Disable colored output | | ||
| | `--full` | | `false` | Inspection-oriented output; supported on view, list, and search commands (rejected elsewhere) | |
There was a problem hiding this comment.
This wording implies that every view or list command supports --full, but the root allowlist only permits page list, page view, space list, space view, attachment list, and search; for example, cfl page history list --full is rejected. Enumerate the supported command paths here (or link to the authoritative output spec) so the public flag contract does not overpromise.
Reply inline to this comment.
There was a problem hiding this comment.
The --full row now enumerates the exact allowlist (page list, page view, space list, space view, attachment list, search) and notes rejection elsewhere.
| } | ||
| if proj.Full { | ||
| fields = append(fields, | ||
| sharedpresent.Field{Label: "Parent ID", Value: orDash(proj.ParentID)}, |
There was a problem hiding this comment.
--full unconditionally renders Parent ID, Created At, and Author ID, converting unavailable values to -. These are optional API fields (and the PR promises author ID only where available); this also conflicts with the detail-output convention to omit absent optional fields. Root pages and responses without author/creation metadata therefore emit misleading empty labels. Append each full field only when its value is present (and timestamp non-zero), and add a presenter/view test covering omitted metadata.
Reply inline to this comment.
There was a problem hiding this comment.
Fixed in 8fbd5ad: Parent ID, Created At, and Author ID are appended only when present (timestamps non-zero), matching the omit-absent-optional-fields convention, with presenter and page-view tests for root pages and missing metadata.
| @@ -12,10 +12,11 @@ Search, find, and filter Confluence pages using full-text search, CQL, or space- | |||
| | "search in SPACE" | `cfl search "query" --space KEY` | Space-scoped search | | |||
There was a problem hiding this comment.
File-level note: skills/Confluence/Workflows/SearchPages.md
This workflow still directs agents to use -o json and parse a JSON envelope, but the current root command rejects resource JSON output (-o accepts only table and plain). Any scripted search following this guidance fails before executing. Replace the JSON example and the Always use -o json advice with the supported -o plain TSV output and identify the ID column for extraction.
Reply inline to this comment.
There was a problem hiding this comment.
SearchPages.md now teaches -o plain TSV with the ID column identified; all -o json guidance is gone.
1 participant
Implements items 6, 8, and 9 of #455. Stacked on #459; will retarget as bases merge.
page view --fullnow emits the extended metadata promised by the output contract (parent ID, creation time, author ID where available), composes with--body-format, and--full --content-onlyis rejected before network access. Commands with no defined full projection now reject--fullinstead of silently ignoring it.default_spaceimplicitly;--spacescopes explicitly. Raw--cqlis mutually exclusive with the positional query and all query-builder filters — conflicts are rejected before config/network instead of silently dropped.attachment list --limitandsearch --limitboth reject zero and negative values before API-client construction (previously silent coercion to 25 and short-circuit-to-empty respectively).