Skip to content

fix(cli): stop claiming logs --follow implies JSONL - #3322

Open
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/logs-follow-capability-contract
Open

fix(cli): stop claiming logs --follow implies JSONL#3322
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/logs-follow-capability-contract

Conversation

@luvs01

@luvs01 luvs01 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Align the ocx logs --follow capability description with the CLI: follow mode polls for new rows and keeps human-readable output unless --jsonl is explicitly supplied.
  • Correct the --follow --json usage error so it names the invalid combination without implying that follow mode enables JSONL.
  • Update the generated management-surface reference and add regressions for both --follow --json rejection and bare --follow human-readable output.

Verification

  • Exact head db75b8df88104b8fa1cf7bac72807f41de61fe60 is rebased directly onto current dev 1cc2ff22dfc42d2100e2330ff3d2ebe7dc2331db.
  • bun test --isolate --timeout 60000 tests/cli-capabilities.test.ts tests/cli-usage-report.test.ts — 36 pass, 0 fail, 256 assertions on Bun 1.4.0.
  • The usage-report suite was also repeated 20 times against the same source and test bytes — 380 pass, 0 fail, with no hang.
  • The broader changed-impact run had 148 passing tests; its only 5-second timeout was re-run as the complete affected file and passed 6/6 with a 60-second cap.
  • bun x tsc --noEmit — passed.
  • bun run skill:surface:check — generated management reference is current.
  • git diff --check origin/dev...HEAD — clean.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

This changes capability metadata, its generated reference, the contradictory usage error, and focused regression coverage only; log streaming and authentication behavior are unchanged.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Documentation

    • Clarified that logs --follow polls for new rows.
    • Documented that --jsonl must be added explicitly to produce JSONL output.
    • Clarified that --follow with --json is invalid and that --jsonl is the streaming alternative.
  • Bug Fixes

    • Improved the usage error shown when incompatible output options are combined.
  • Tests

    • Added coverage for follow-mode output contracts, including human-readable output by default and JSONL guidance.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: bcc5feca-fda2-4e4c-9fe8-a07dc9cc81c2

📥 Commits

Reviewing files that changed from the base of the PR and between 5ffb266 and 483d2c8.

📒 Files selected for processing (4)
  • skills/ocx/references/01_management_surface.md
  • src/cli/capabilities.ts
  • src/cli/observe.ts
  • tests/cli-usage-report.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The ocx logs --follow contract now distinguishes human-readable polling output from JSONL output. The CLI reports that --jsonl is required for streaming JSONL and includes tests for both output modes.

Changes

Logs follow contract

Layer / File(s) Summary
Update follow output behavior
src/cli/observe.ts:75-77, tests/cli-usage-report.test.ts:9, tests/cli-usage-report.test.ts:225-284
--follow --json now reports that --jsonl is the streaming alternative. --follow alone remains human-readable. Tests verify the error, output format, exit codes, and polling stop behavior.
Update follow description and validation
src/cli/capabilities.ts:277, skills/ocx/references/01_management_surface.md:127, tests/cli-capabilities.test.ts:77-82
The --follow summary now states Poll for new rows; add --jsonl to emit JSONL. The management reference matches this behavior. The test asserts the exact summary.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 483d2

The logs follow documentation and usage guidance now match the existing polling behavior: human-readable output remains the default and JSONL requires --jsonl. The covered flag contract introduces no remaining merge risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the CLI documentation and capability correction for logs --follow. It accurately states that the change removes the incorrect implication that follow mode automatically …
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. (1 skipped: 1 unsupported.)

Full details: Title check

Explanation

The title clearly identifies the CLI documentation and capability correction for logs --follow. It accurately states that the change removes the incorrect implication that follow mode automatically uses JSONL.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added bug Something isn't working review-ready labels Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

@github-actions
github-actions Bot marked this pull request as ready for review September 3, 2026 04:13
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 54 / 80

설명

이 PR은 ocx logs --follow가 JSONL을 강제한다고 적혀 있던 capability 문구를, 실제 CLI 동작에 맞게 고친다. 지금 dev HEAD는 ff1ac6b8c(Meta Model API 직접 프로바이더 #3321)이고, src/cli/observe.tslogs()--follow만 켜면 사람 읽기용 formatLog 줄을 찍고, JSONL은 --jsonl을 따로 줄 때만 쓴다. --json--follow만 충돌한다. 그런데 src/cli/capabilities.ts 플래그 summary는 아직도 “Stream new rows as JSONL; implies --jsonl.”이라고 거짓말하고, 그 테이블에서 만들어지는 skills/ocx/references/01_management_surface.md도 같은 말을 에이전트에게 보여 준다.

고치는 범위는 작다. capability 한 줄, 생성 문서 한 줄, 그리고 summary가 그 문장인지 고정하는 회귀 테스트 하나다. bun run skill:surface로 문서를 다시 뽑은 흔적이 보이고, 런타임 폴링·인증·로그 API는 건드리지 않는다. types.ts/config.ts 분할과도 안 겹친다. close-don't-rebase 대상이 아니다.

에이전트와 사람이 ocx capabilities / skill surface만 보고 --follow만 치면 JSONL이 나온다고 믿을 수 있는 상태라, 문서-계약 불일치를 끊는 값은 있다. 다만 제품 버그 수정은 아니고 메타데이터 정합이라 우선순위는 중간에 둔다.

src/cli/capabilities.ts --follow summary - 예전 문구 “implies --jsonl”은 observe.tswantsJsonl 분기와 반대다. 이번 문구 “Poll for new rows; add --jsonl to emit JSONL.”이 실제 출력 경로와 맞다.

src/cli/observe.ts 에러 문장 “--follow uses --jsonl, not --json” - 이 PR이 고치진 않았다. --follow만 쓰면 JSONL이 아니라 human 줄이다, 문구는 여전히 “follow면 jsonl을 쓴다”처럼 들릴 수 있다. summary와 톤이 살짝 어긋난다.

tests/cli-capabilities.test.ts - summary 문자열 전체를 고정한다. 나중에 문장만 다듬으면 테스트가 깨지니, 의도가 “JSONL을 암시하지 말 것”이면 not.toContain("implies") 같은 약한 단언도 후보지만, 지금 크기에서는 허용 가능하다.

skills/ocx/references/01_management_surface.md - 생성물이라 capability와 같이 가는 것이 맞다. 손편집만 하고 생성을 빼먹으면 tests/skill-ocx.test.ts가 잡는 구조다.

메인테이너의 판단이 필요한 지점

  • observe.ts의 “--follow uses --jsonl, not --json” 에러 문구를 이번 PR에서 “--follow cannot be combined with --json”처럼 같이 맞출지, 후속으로 둘지
  • review-ready이고 hygiene 초록이면 바로 머지할지

너의 추천
머지해도 된다. dev의 실제 logs --follow 계약과 capability/skill surface를 맞추는 작은 정합 수정이고, 회귀 테스트까지 있다. 에러 문구 다듬기는 필수는 아니니 원하면 한 줄 follow-up으로 남겨도 된다.

이 댓글은 grok-bot이 작성했습니다

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lidge-jun @luvs01 현재 HEAD 5ffb26635은 capability 표와 생성 문서는 고쳤지만, 같은 CLI의 실제 사용자 오류 문구가 아직 반대 의미를 말하고 있습니다.

src/cli/observe.ts에는 지금도 다음 문장이 남아 있습니다:
--follow uses --jsonl, not --json

하지만 실제 동작은 --follow만 쓰면 사람이 읽는 출력이고, --jsonl을 함께 지정할 때만 JSONL입니다. 즉 이 PR의 제목인 “follow가 JSONL을 암시한다고 주장하지 않기”가 실제 오류 경로에서는 아직 완료되지 않았습니다.

해당 문구를 예를 들어 --follow cannot be combined with --json; use --jsonl for streaming JSONL처럼 실제 계약에 맞게 고치고, --follow --json 오류와 --follow 단독 human output 계약 테스트를 추가해 주세요. 그 뒤 exact-head CI가 green이면 승인 가능합니다.

@luvs01
luvs01 force-pushed the fix/logs-follow-capability-contract branch from 5ffb266 to 483d2c8 Compare September 3, 2026 13:54
@github-actions
github-actions Bot marked this pull request as draft September 3, 2026 13:54
@luvs01
luvs01 force-pushed the fix/logs-follow-capability-contract branch from 483d2c8 to db75b8d Compare September 3, 2026 14:08
@github-actions
github-actions Bot marked this pull request as ready for review September 3, 2026 14:13
@luvs01

luvs01 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

@Ingwannu @lidge-jun — the requested follow-up from review 5099974325 is now addressed on exact head db75b8df88104b8fa1cf7bac72807f41de61fe60, rebased directly onto current dev 1cc2ff22dfc42d2100e2330ff3d2ebe7dc2331db.

  • corrected the --follow --json error so it no longer implies that follow mode enables JSONL;
  • added regressions for --follow --json rejection and bare --follow human-readable output;
  • passed the focused suites (36/36), typecheck, generated-surface check, repeated no-hang coverage, and diff check;
  • passed the exact-head hygiene, target, label, and CodeRabbit gates; CodeRabbit generated no actionable comments.

The PR is Ready with the 4/4 attestation bound to this head. The remaining Cross-platform CI run 33765100066 and React Doctor run 33765099727 are action_required because the fork workflows still need maintainer approval. Please approve those runs and re-review when convenient.

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

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants