Skip to content

fix(log-view): support multiple log IDs and validate hex format#362

Merged
BYK merged 8 commits intomainfrom
fix/cli-bc-multi-log-view
Mar 6, 2026
Merged

fix(log-view): support multiple log IDs and validate hex format#362
BYK merged 8 commits intomainfrom
fix/cli-bc-multi-log-view

Conversation

@BYK
Copy link
Member

@BYK BYK commented Mar 6, 2026

Fixes CLI-BC

Problem

An AI agent called sentry log view brandai/brandai "<7 log IDs joined by newlines>". The newline-delimited blob was treated as a single log ID, interpolated into the API query sentry.item_id:${logId}, and the Sentry API rejected it with 400 Bad Request.

Solution

Instead of just validating and rejecting, accept multiple log IDs. The Sentry Explore API already supports bracket syntax (sentry.item_id:[id1,id2,...]), and the Stricli positional is already kind: "array".

Changes

  • src/lib/hex-id.ts (new) — Shared 32-char hex ID validator with whitespace trimming and truncated error messages
  • src/lib/trace-id.ts — Delegates to hex-id.ts (backward-compatible re-export)
  • src/lib/api-client.ts — New getLogs() with bracket query syntax; getLog() becomes a convenience wrapper
  • src/commands/log/view.tsparsePositionalArgs() returns logIds[]; splits newline-in-arg; validates each ID; displays multiple logs with --- separators; warns about missing IDs on stderr

Both input styles now work:

  • sentry log view org/proj id1 id2 id3 (space-separated)
  • sentry log view org/proj "id1\nid2\nid3" (newline-in-arg)

Backward Compatibility

  • Single-ID usage unchanged
  • Single-ID --json output remains a single object (not wrapped in array)
  • Multiple IDs: --json outputs an array; human output separates entries with ---
  • --web opens only the first log

@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Bug Fixes 🐛

  • (log-view) Support multiple log IDs and validate hex format by BYK in #362
  • (logs) Harden log schemas against API response format variations by BYK in #361

Internal Changes 🔧

  • (delta-upgrade) Lazy chain walk, GHCR retry, parallel I/O, offline cache by BYK in #360

🤖 This preview updates automatically when you update the PR.

@BYK BYK force-pushed the fix/cli-bc-multi-log-view branch from b3f25b1 to e48527c Compare March 6, 2026 12:06
@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026

Codecov Results 📊

104 passed | Total: 104 | Pass Rate: 100% | Execution Time: 0ms

📊 Comparison with Base Branch

Metric Change
Total Tests 📈 +3
Passed Tests 📈 +3
Failed Tests
Skipped Tests

All tests are passing successfully.

✅ Patch coverage is 89.01%. Project has 3895 uncovered lines.
❌ Project coverage is 80.7%. Comparing base (base) to head (head).

Files with missing lines (3)
File Patch % Lines
api-client.ts 63.98% ⚠️ 416 Missing
view.ts 92.27% ⚠️ 16 Missing
logs.ts 97.69% ⚠️ 3 Missing
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
- Coverage    81.50%    80.70%     -0.8%
==========================================
  Files          128       129        +1
  Lines        19767     20180      +413
  Branches         0         0         —
==========================================
+ Hits         16111     16285      +174
- Misses        3656      3895      +239
- Partials         0         0         —

Generated by Codecov Action

…CLI-BC)

- Add shared hex ID validator (src/lib/hex-id.ts) for 32-char hex IDs
- Refactor trace-id.ts to delegate to hex-id.ts (backward compatible)
- Add getLogs() to api-client.ts with bracket query syntax for multi-ID
- Update parsePositionalArgs to return logIds[] array instead of logId
- Split newline-separated IDs within a single argument
- Validate each log ID is a valid 32-char hex string
- Display multiple logs with --- separator (human) or array (JSON)
- Warn to stderr when some IDs are not found
- Preserve backward compat: single ID JSON output stays a single object
- Update e2e mock to use valid hex TEST_LOG_ID
@BYK BYK force-pushed the fix/cli-bc-multi-log-view branch from 5c891b6 to 3d0cf65 Compare March 6, 2026 12:35
@BYK BYK marked this pull request as ready for review March 6, 2026 12:39
- Normalize hex IDs to lowercase in validateHexId for consistent API comparison
- Remove dead getLog convenience wrapper (unused)
- Simplify getLogs to always use bracket syntax for ID filter
- Add per_page comment explaining why it's needed
- Open all IDs in browser with --web (warn about multiple tabs)
- Use markdown list format for error messages (- `<id>`)
- Use consola logger for missing IDs warning instead of stderr.write
- Use logIds.join(' ') in usage hints
- Extract resolveTarget, warnMissingIds, formatIdList helpers for complexity
- Update trace-id tests for lowercase normalization
Copy link
Member Author

@BYK BYK left a comment

Choose a reason for hiding this comment

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

Addressed all review comments.

- Cap per_page at API_MAX_PER_PAGE (100) in getLogs to prevent silent
  truncation when >100 log IDs are requested
- Fix parsePositionalArgs docstring: remove false claim about multi-ID
  auto-detect pattern (2+ args always treats first as target)
- Fix trace/logs.ts callers to use normalized return value from
  validateTraceId instead of discarding it
- Update trace/logs test for lowercase normalization
BYK added 2 commits March 6, 2026 15:09
- --web multi-tab: prompt for confirmation in interactive mode, refuse
  with warning in non-interactive mode (using consola prompt)
- Remove direct stderr parameter from resolveTarget — resolveProjectBySlug
  tip is already optional, no need to pipe stderr through
- getLogs: batch requests in parallel when >API_MAX_PER_PAGE IDs are
  requested, instead of silently capping at 100
- Extract handleWebOpen() and throwNotFoundError() helpers to keep
  func() body under cognitive complexity limit of 15
- Add getLogs unit tests in api-client.test.ts: bracket syntax URL
  verification, empty array handling, and batching for >100 IDs
- Add auto-detect and null-target path tests in view.func.test.ts
- Add isolated test for interactive --web prompt path (mock.module
  for node:tty and logger.js to simulate TTY + consola prompt)
consola's prompt() returns Symbol(clack:cancel) on Ctrl+C, which is
truthy in JavaScript. Use strict `confirmed !== true` check instead
of `!confirmed` to prevent opening browser tabs on cancel.

Adds isolated test for cancel-symbol behavior.
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

resolveTarget's project-search case now logs the 'Tip: Resolved
project ID X to org/slug' message via consola info instead of
relying on the stderr param that was removed from this command.
@BYK BYK merged commit a5fa054 into main Mar 6, 2026
20 checks passed
@BYK BYK deleted the fix/cli-bc-multi-log-view branch March 6, 2026 17:17
BYK added a commit that referenced this pull request Mar 6, 2026
Merge origin/main's fix(log-view): support multiple log IDs and validate
hex format (#362) into the argument parsing improvements branch.

Key conflict resolutions:
- src/commands/log/view.ts: Keep main's multi-ID structure (logIds[],
  validateHexId, resolveTarget helper), add swap detection and
  looksLikeIssueShortId suggestion for 2-arg case using flat ternaries
  to stay under cognitive complexity limit
- test/commands/log/view.test.ts: Update func test spies from getLog to
  getLogs with array return values; swap-detection func test now expects
  ValidationError since hex validation fires first
- test/commands/log/view.property.test.ts: Keep main's typed arbitraries
  (slugArb, logIdArb), remove unnecessary pre() guard
- AGENTS.md: Keep all lore entries from both sides
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