Skip to content

refactor(cli): extract read-only formatting primitives - #3071

Open
tang-vu wants to merge 2 commits into
Osmantic:mainfrom
tang-vu:refactor/ods-cli-formatting-module
Open

refactor(cli): extract read-only formatting primitives#3071
tang-vu wants to merge 2 commits into
Osmantic:mainfrom
tang-vu:refactor/ods-cli-formatting-module

Conversation

@tang-vu

@tang-vu tang-vu commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Why this matters

ods-cli is a roughly 6,500-line production entry point. The repository's decomposition roadmap calls for behavior-preserving extraction of read-only formatting before stateful commands are split. Keeping status-color mapping, JSON escaping, and separator rendering inline makes their ownership unclear and forces future command extraction to move implementation and behavior at the same time.

This PR completes the first narrow Phase 1 slice: it moves only those pure primitives into lib/cli-format.sh, sources that installed module from the existing CLI, and preserves the command syntax and byte-level output contracts. ods list --json, human list/status output, template output, and table separators remain owned by the same CLI callers.

The behavioral invariant is: extracting a pure helper must not change any public CLI output, status mapping, JSON escaping, or separator width.

What changes

  • add lib/cli-format.sh with the three side-effect-free formatting primitives
  • source the module after the CLI initializes color variables, preserving NO_COLOR behavior
  • remove only the corresponding inline definitions from ods-cli
  • update the isolated installed-CLI reconciliation fixture to deploy the required module
  • add BATS boundary coverage for module loading, status mapping, all JSON control escapes, and separator rendering
  • keep .bats files LF-normalized so Windows checkouts remain executable by Bash
  • record the completed slice and remaining Phase 1 ownership in docs/ODS_CLI_DECOMPOSITION.md

Overlap check

Searched open and closed PRs for ods-cli decomposition formatting, cli-format, the three helper names, and every open patch touching ods/ods-cli. There are 25 open PRs touching the large CLI, but none of their patches contains _status_color, _json_escape, or hr; the closest historical result is merged PR #1100, which standardized logging/banner style rather than these helpers.

Six open PRs (#3019-#3024) touch .gitattributes; they add or consume rules for Makefiles/workflows/scripts and do not add the *.bats contract. No open PR touches lib/cli-format.sh, the new BATS contract, the reconciliation fixture, or the decomposition roadmap. The scope is independently useful and does not replace an existing implementation.

Regression coverage

Focused validation from the repository root / ods/:

  • bash tests/test-ods-list-json-escaping.sh — pass; exercises the public ods list --json boundary
  • normalized Linux-tooling run of test-cli-formatting.bats — 4/4 pass
  • bash tests/test-phase-c-p1.sh — 7 passed, 0 failed, 9 authentication-dependent warnings
  • bash -n ods-cli lib/cli-format.sh — pass
  • shellcheck --rcfile=/dev/null --severity=error ods-cli lib/cli-format.sh — pass
  • git diff --check — pass
  • bash tests/test-cli-enable-compose-reconciliation.sh — pass after the isolated installed-CLI fixture was updated

The repository-downloaded BATS runner in this Windows checkout had CRLF in its own ignored clone, so the same pinned runner/support/assert trees were copied to a verified /tmp/ods-bats.* directory, mechanically LF-normalized, and used for the 4/4 receipt. CI's fresh Linux checkout remains the authoritative literal tests/run-bats.sh execution.

A broad tests/contracts/test-installer-contracts.sh probe also reached an existing failure at Hermes template bounds each model turn. Neither that test nor extensions/services/hermes/cli-config.yaml.template is changed by this branch; the focused CLI and formatting contracts above pass.

Design and rollback

The module intentionally has no set -euo pipefail: it is sourced into the CLI and inherits caller policy. It expects the already-initialized color variables and performs no I/O other than returning formatted bytes. Service aliases and config masking remain in their current owners until a later independently testable slice.

Rollback is a single revert: restore the three inline functions and remove the source line/module. No persisted state, command syntax, or runtime migration is involved.

CI follow-up

The first integration-smoke run failed because test-cli-enable-compose-reconciliation.sh intentionally builds an isolated installed footprint but copied only ods-cli plus two older library dependencies. Commit bff5cbfc adds the new required module to that fixture; the exact previously failing test now passes for stale, empty, missing, and dual-marker compose states. All other test fixtures that execute a copied CLI already copy lib/*.sh or the full lib/ tree. The batch integration branch includes the fix at 2c82aaec. The rerun is fully green: 28 successful checks, 4 intentional skips, including the 3m58s integration-smoke job that originally exposed the missing fixture dependency.

@tang-vu

tang-vu commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Batch compatibility receipt (2026-08-24)

Built the inspectable synthetic branch tang-vu:validation/batch-20260824-architecture from upstream main (6ff9b4fc) and merged exact candidate heads in this order:

  1. fix(dashboard): keep extension progress polls single-flight #3065 bad25e7f
  2. feat(extensions): preview and confirm lifecycle change plans #2903 829dd48f
  3. feat(observability): stream scoped service health transitions #2911 d8aed4c6
  4. feat(extensions): inspect and gate dependency graphs #2909 6e9bd227
  5. feat(release): publish commit-bound deterministic runtime SBOM #2894 9115d147
  6. refactor(cli): extract read-only formatting primitives #3071 bff5cbfc

No manual merge resolution was required, including the shared Extensions.jsx / Extensions.test.jsx surface in #3065 and #2903. Synthetic head: 2c82aaec.

Combined validation:

  • Dashboard: 28 files / 262 tests passed; ESLint passed; production Vite build passed
  • Dashboard API: 350 passed, 5 skipped across service events, main routing, extension plans, lifecycle, and dependency suites
  • dependency graph + manifest gates: 4 pytest cases, 8 shell cases, full canonical/library schema contract passed
  • runtime SBOM: 5 tests passed; shipped dependency-pin validation passed
  • CLI: JSON public boundary, Phase C smoke, Bash syntax, ShellCheck, 4/4 BATS formatting contract, and enable/reconciliation fixture passed

The stale Browserslist notice and authentication-dependent Phase C skips are unchanged environmental disclosures. #2903 remains draft because synthetic compatibility does not replace its declared live host-agent lifecycle gate and independent human review.

@tang-vu

tang-vu commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

CI follow-up complete: head bff5cbfc passed the full rerun, including integration-smoke (3m58s). The initial missing-module fixture failure is resolved; no failing or pending checks remain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant