Skip to content

feat(browserstack-service): render end-of-build summary entries (SDK-7358) - #200

Merged
Dalwin-Barnard merged 7 commits into
mainfrom
feat/SDK-7358-version-nudges
Sep 25, 2026
Merged

Dalwin-Barnard merged 7 commits into
mainfrom
feat/SDK-7358-version-nudges

Conversation

@shivam5643

@shivam5643 shivam5643 commented Sep 16, 2026 •

Copy link
Copy Markdown
Collaborator

What is this about?

Renders end-of-build customer-visible summary entries (SDK-7358).

The binary returns CustomerVisibleSummaryEntry items on StopBinSessionResponse for
end-of-build messages such as the SDK version nudge. The service had neither the proto
field nor a renderer, so those messages arrived and were silently dropped for every wdio
customer running through the CLI path.

Adds the proto field and a renderer that writes body, picks the stream from severity,
and archives a copy to the log file. Deliberately does not branch on entryType, so
future entry types need no further service change — matching node, java, python and ruby.

Stream choice: warn/warning/error -> stderr, everything else including unknown ->
stdout, so a malformed severity cannot false-alarm CI tooling watching stderr.

Colour. A warn entry (an outdated SDK) is tinted yellow and an error entry (a
deprecated one) red, with the block's first line of text emphasised. Lines are wrapped and
reset individually rather than the block as a whole, so a truncated or interleaved write
cannot leave the customer's terminal stuck in colour. Applied to the stream copy only — the
archived copy stays plain, because escape codes reach a log file as literal bytes and break
anchored searches over it. Border lines are detected as "carries no letters or digits"
rather than by matching U+2500, so a change to the binary's divider glyph cannot silently
start emphasising the wrong line.

Archive once. The archived copy now goes through BStackLogger.logToFile rather than
the info/warn/error helpers. Those helpers also call @wdio/logger, which writes to
the console, so the customer saw the block twice — once raw from the stream write, once
prefixed by the logger. logToFile writes to the log file only. (The v8 branch already
did this; v9 did not.)

Worth noting for reviewers: only wdio-mocha (non-multiremote) reaches this path today, since
CLISupportedFrameworks is ['mocha'] and the classic path bypasses the binary entirely.

Related Jira task/s

Release (mandatory for every PR — required for the ready-for-review label)

Version bump: (required — tick exactly one)

  • minor (backwards-compatible feature)
  • patch (bug fix or other small change)

Release notes type: (optional)

  • New Feature
  • Bug Fix
  • Other Improvement

Release notes (customer-facing): (optional but encouraged)

  • End-of-build messages from BrowserStack — such as a notice that your SDK version is outdated or has a known issue — are now shown at the end of your test run, highlighted in yellow for a warning and red for an error, and written to the SDK log without colour so they stay searchable.

Release notes (internal): (required — engineer-facing; what actually changed / why)

  • Adds entries + CustomerVisibleSummaryEntry to sdk-messages.proto, matching the binary's canonical definition (field 5).
  • GrpcClient.renderCustomerVisibleSummary writes body to the stream chosen by severity, not through the logger, whose per-line prefix would break the binary's box-border alignment.
  • GrpcClient.colouriseSummaryBody tints the stream copy by severity (yellow/warn, red/error, untouched otherwise), line by line with an explicit reset per line. Escapes are written as \x1b so the ESC byte stays visible in source.
  • Archiving moved from the info/warn/error helpers to logToFile. The helpers also call @wdio/logger, which writes to the console — the block was being printed twice.
  • Failure-tolerant throughout: a render or colour error is swallowed and logged at debug; stopBinSession still returns its response and the customer still gets the plain message.

Checklist

  • Ready to review
  • Has it been tested locally?

PR Validations

Run Tests: Comment RUN_TESTS to trigger sanity tests.

Testing

  • 10 tests driven through the real stopBinSession path; file passes 10/10. Covers exact escape sequences per line for both tiers, plain archiving, and no-colour for info and unknown severities.
  • tsc -p tsconfig.prod.json --noEmit and eslint both exit 0.
  • No manual changeset added — per this template the changeset is generated automatically from the Release section above.
  • Paired v8 PR raised separately against the v8 branch (feat(browserstack-service): render end-of-build summary entries (SDK-7358) [v8] #201).
  • Depends on browserstack-binary#1782 (the binary must emit the entry). Deploy order: railsApp -> binary -> SDK.

🤖 Generated with Claude Code

…7358)

The binary returns CustomerVisibleSummaryEntry items on StopBinSessionResponse
for end-of-build messages such as the SDK version nudge. The service had neither
the proto field nor a renderer, so those messages were dropped for every wdio
customer running through the CLI path.

Adds the proto field and a renderer that writes `body` verbatim, picks the
stream from `severity` (warn/warning/error -> stderr, everything else including
unknown -> stdout so a malformed severity cannot trip CI stderr watchers), and
archives a copy to the log file for runners that keep only the log directory.

Deliberately does not branch on `entry_type`, so future entry types need no
further service change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@shivam5643
shivam5643 requested a review from a team as a code owner September 16, 2026 08:06
@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited), Workspace UI (inherited)

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 0550aa5e-3c5b-4994-a0ac-286ca8cc12e7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

shivamku-BS and others added 2 commits September 24, 2026 13:12
…e (SDK-7358)

Two changes to the end-of-build summary rendering added in this PR.

Colour: a warn entry (an outdated SDK) is tinted yellow and an error entry
(a deprecated one) red, with the block's first line of text emphasised.
Lines are wrapped and reset individually rather than the block as a whole,
so a truncated or interleaved write cannot leave the customer's terminal
stuck in colour. Applied to the stream copy only — the archived copy stays
plain, because escape codes reach a log file as literal bytes and break
anchored searches over it.

Archiving: switch from the info/warn/error helpers to logToFile. Those
helpers also call @wdio/logger, which writes to the console, so the
customer saw the block twice — once raw from the stream write, once
prefixed by the logger. logToFile writes to the log file only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@shivam5643
shivam5643 requested review from harshit-browserstack and removed request for kamal-kaur04 September 24, 2026 10:41
@shivam5643

shivam5643 commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

🔴 Blocking findings — fix required

See the SDK PR Review Agent's report from your local run.

Change map (generated deterministically from the diff)

graph LR
  subgraph nwdio_service["wdio-service"]
    npackages_browserstack_service_src_cli_grpcClient_ts["⚠ grpcClient.ts<br/>~151 lines"]
    npackages_browserstack_service_tests_cli_grpcClient_test_ts["⚠ grpcClient.test.ts<br/>~151 lines"]
    npackages_browserstack_service_src_proto_browserstack_sdk_v1_sdk_messages_proto["⚠ sdk-messages.proto<br/>~18 lines"]
    n_changeset_pr_200_md["pr-200.md<br/>~5 lines"]
  end
  nsvc_every_SDK___binary(["every SDK + binary"])
  npackages_browserstack_service_src_proto_browserstack_sdk_v1_sdk_messages_proto --> nsvc_every_SDK___binary
Loading

↻ This verdict comment is the review anchor — it's updated in place on each run (the gate posts its status separately).

— SDK PR Review Agent

shivamku-BS and others added 3 commits September 24, 2026 17:04
…over attachment fields

Review follow-up on SDK-7358.

1. renderCustomerVisibleSummary wrapped the whole `for (const entry of
   entries)` loop in one try/catch, so a stream that rejected entry N
   aborted the loop: entries N+1.. were neither written nor archived.
   Only one entry exists today (the version nudge), but the proto is
   explicitly built for more entry types, so the gap widens as they are
   added. The catch now sits inside the loop body, and the archived copy
   is written before the stream write so archival no longer depends on
   the write succeeding. The outer catch stays — stopBinSession rethrows,
   so a throw escaping this method would cost the caller its response.

   This is the same defect and fix as the v8 port (#201).

2. Adds the missing test for the fileName/fileSize/filePath passthrough
   in logCreatedEvent. Those fields are unrelated to the summary-entry
   feature and arrived in this PR undocumented and uncovered; this locks
   their behaviour rather than leaving an untested drive-by. Whether the
   change belongs in its own PR is still worth a call.

Both new tests fail without their respective fix.

Package suite on Node 25: 56 files, 1259 tests, 0 failures.
tsc --noEmit exit 0, eslint exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: failure).

This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge.

@Dalwin-Barnard
Dalwin-Barnard merged commit 33b8ce5 into main Sep 25, 2026
20 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants