Repository navigation
chore: maintenance sweep - retire Dependabot, refresh dependencies, fix Sonar, dedup, tests and docs - #93
Merged
Conversation
Dependencies now move in periodic maintainer bulk PRs: bun outdated, edit the ranges, bun install, run the full checks. Dependabot alerts stay on; on the GitHub side its open PRs are already closed and its security-update PRs are disabled in the repository settings. - Delete .github/dependabot.yml. What its comments encoded survives as human guidance in CONTRIBUTING.md, section "Updating dependencies": commit bun.lock with package.json because CI's frozen-lockfile install rejects a mismatch; the sdk moves in its own PR while dev dependencies can share one; and codeql-action init + analyze must share a SHA. - cla.yml: drop the Dependabot-only skip step and the two "!= 'dependabot[bot]'" step conditions. Those conditions were true for every other run, since pull_request.user.login is empty on issue_comment, so human PRs and sign/recheck comments behave exactly as before. The required `cla` context is unchanged: same job id, triggers and job-level condition. A short comment records that turning Dependabot back on means restoring the skip first. - CLAUDE.md and CONTRIBUTING.md: point at the new section, and correct the sdk sentence that said to bump the range and the floor together and quoted `^1.6.0`, stale since the range moved to ^2.0.0. Since #81 the test asserts 1.6.0 as a FLOOR; telling a bulk updater to move it with the range is the re-pinning that test was rewritten to stop. - scripts/check-package-identity.test.ts: comments only, since the range no longer moves with Dependabot. The assertion is untouched. All six required contexts are still produced: build-test on the three OS legs, Analyze (javascript-typescript), SonarQube Cloud analysis, cla. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bulk dependency update (CONTRIBUTING.md § Updating dependencies): - @biomejs/biome ^2.5.13 -> ^2.5.15 (dev; patch releases only: new nursery rules, which the `recommended` preset does not enable, plus bug fixes). `biome check .` is clean on the new version. - biome.json `$schema` 2.5.11 -> 2.5.15, written by `biome migrate --write`. The schema had already drifted two patches behind the installed CLI, which `biome check` reported as an info on every run; that info is gone now. - Transitive refresh: @types/node 26.1.1 -> 26.6.4 (pulled in by bun-types as `*`) and undici-types 8.3.0 -> 8.9.0 (the only 8.9.x, which is what @types/node 26.6.4 pins with `~8.9.0`). Already at latest and unchanged: @nimbus-dev/sdk 2.0.0 (the one runtime dependency, so its published range does not move), @types/bun and bun-types 1.4.2, typescript 7.0.2. Verified: bun install --frozen-lockfile, build, typecheck, lint, the test suite under CI=true (527 pass, 1 skip, 0 fail, same as before), test:coverage, and both Node checks from ci.yml (CJS require smoke and the no-embedded-build-path check on dist/index.cjs). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each new SHA is the commit the release tag resolves to (annotated tags dereferenced), and each comment names that exact release: - github/codeql-action/init + /analyze: v4.38.0 (b96794f, commented `# v4`) -> v4.38.2 (2892aa5). Both steps move together, as CONTRIBUTING.md requires; patch releases, no input changes. - SonarSource/sonarqube-scan-action: v8.2.1 (2291811) -> v8.3.0 (d209202). action.yml is unchanged between the two, so no input changes. v8.3.0's GPG change only alters the path taken when HTTPS_PROXY is set, which this workflow does not set, so the harden-runner allow-list still covers the keyserver fetch. - nimbus-agent/.github/actions/verify-npm-provenance: 5fb4279 -> 7592125 (that repo's `main` tip). The repository publishes no releases or tags, so `main` is the latest version. The only change to the action in between is f85d29c, which writes its outputs in the heredoc delimiter form. With the old `key=value` form, a newline in the registry-derived `detail` value could emit a second `status=` line and turn a source-mismatch into a reported ok. action.yml is byte-identical (same blob 518864f), so this step's inputs are unchanged. - oven-sh/setup-bun: the SHA was already v2.2.0, the latest release; only the comment changes from `# v2` to `# v2.2.0`, to match the documented `# vX.Y.Z` convention. Already at their latest release and unchanged: harden-runner v2.21.1, checkout v7.0.1, setup-node v7.0.0, create-github-app-token v3.2.0, contributor-assistant/github-action v2.6.1, release-please-action v5.0.0. No other action inputs change; the `bun-version: 1.3.14` input is a toolchain pin and is left alone. CONTRIBUTING.md § Updating dependencies now records how the provenance action's comment-less pin moves, since it has no release to put in a comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…et check
Resolves the one open SonarCloud issue, AaC0NAn3RE0367qDCAZp
(typescript:S3735, "Remove this use of the void operator", src/agents.ts:76).
The flagged `void _supportedAgentsAreKnownToTheSdk;` existed only to mark a
never-called arrow function as used, so `noUnusedLocals` would accept the
compile-time assertion that every `SupportedAgentName` is still an SDK
`AgentName`. The rule's own remedy (drop the `void`) cannot be applied as
written: the bare statement left behind has no effect, and without it the
unused constant fails typecheck. So the assertion moves to the type level:
type KnownToTheSdk<Name extends AgentName> = Name;
export type SupportedAgentName = KnownToTheSdk<keyof AgentParamsMap>;
Same guarantee, at the same declaration, with nothing emitted at runtime.
Proved with a temporary tenth `AgentParamsMap` key the SDK does not know
(reverted): the old code failed at the assertion site with TS2322, and the new
code fails at the `KnownToTheSdk` site with TS2344. The downstream errors are
identical, 16 in both cases.
Public surface unchanged: dist/index.cjs and dist/index.d.ts are
byte-identical to the pre-change build. dist/agents.js loses only the two dead
lines (the constant was never exported). dist/agents.d.ts gains the
non-exported alias, and a consumer-side check over the built declarations
with skipLibCheck: false proves that `SupportedAgentName` is still exactly the
nine-name union (a negative control including "decisions" fails as it
should). It adds no new consumer-side failure mode, because the SDK's
`BriefFor<A extends AgentName>` already carries the same constraint through
`AgentBriefEvent`. The new alias cannot trip S6564 (redundant type alias):
SonarJS reports an alias whose body is a bare reference to another type
alias, and here the body is a type parameter and the use site has type
arguments.
Also drops a dead `void req;` from test/ipc-transport.test.ts. It is the
same S3735 pattern, held to the same standard although Sonar does not apply
the rule to test code; `req` is read two lines later, so the statement
suppressed nothing.
eslint-plugin-sonarjs 4.2.2, with every rule enabled, run over the pre- and
post-change text of both files: it reproduces the S3735 finding at 76:1 (and
373:7 in the test) before and reports neither after, with no new finding in
either file.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…am mappings This removes all four clones jscpd reports at --min-tokens 50 over src/ and scripts/ (three copies of the queryItems signature and one of the searchRanked wire mapping), plus two same-concept copies below that threshold: - queryItems' parameter and result shapes were spelled out inline four times: in NimbusClientLike, NimbusClient, MockClient and the return type of validateQueryItems. They are now the QueryItemsParams and QueryItemsResult aliases in nimbus-client.ts. Both are structurally identical to the inline types and are not added to the package entry, so the published export surface is unchanged: dist/index.d.ts is byte-identical. - searchRanked and searchRankedWithRetrieval each mapped RankedSearchParams onto the index.searchRanked wire params by hand. A module-private rankedSearchWireParams now does it once, and the envelope variant adds `envelope: true` on top. - workflowRun and the streaming path in workflow-stream.ts each mapped WorkflowRunParams onto workflow.run by hand. WorkflowRunStreamParams is Omit<WorkflowRunParams, "stream">, so a field added to WorkflowRunParams would reach the streaming handle's input and then be silently dropped by its hand-written literal. workflowRunWireParams now builds both, and the stream path still overrides `stream: true` and its minted streamId. It lives in workflow-stream.ts because that module imports nimbus-client.ts for types only, so sharing it adds no runtime import cycle. - MockClient's workflowRun and workflowRunStream each built the same no-fixture default run. A private defaultWorkflowRun now builds it, beside the class's existing private default*() helpers. Behaviour is unchanged. A differential run of the previous and new builds through a recording fake transport put byte-identical params on the wire for workflowRun, workflowRunStream, searchRanked, searchRankedWithRetrieval and queryItems, including key order and keys whose value is undefined. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d checks validateConnectorAuth, validateConnectorAddMcp and validateConnectorRemove each repeated the same guard that a success arm's `ok` is the literal true, and connector.auth's scopesGranted and connector.remove's vaultKeysRemoved each repeated the same element-type loop with the same message template. They are now requireOk and strArr, beside validate.ts's other field readers (str, num, bool, arr). requireOk is a statement rather than a field read on purpose. Called inside the return literal, it would run after the array check and change which error a payload wrong in two places reports. New tests pin that order for both validators, and each order test fails when its validator's checks are swapped. Further new tests cover a missing `ok` and a field that is not an array. All the new tests also pass against the previous validate.ts, so they describe existing behaviour rather than new behaviour. Left alone on purpose: "notes" and the drift "lines" array are string arrays too, but their error text differs, and that text is observable; validateOk and the audit and egress readers accept `ok: false`, which is a different rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ream handles askStream and workflowRunStream each carried their own copy of the same buffer: a queue of events, a list of parked next() resolvers, a done flag, a push that hands an event to the oldest waiter or buffers it, a finish that settles every waiter, and a next that drains the buffer before reporting done. Both copies also carried the same never-called `reject` in their Pending type. That logic is now createEventQueue in src/event-queue.ts, a runtime module covered by tests, unlike the type-only stream-events.ts. What differs between the two handles stays in each one: the notifications that feed the queue, askStream's pre-streamId buffering and upstream cancel, and the teardown each finish() runs. close(teardown) marks the queue closed, runs the teardown, then settles parked next() calls. That is the order both finish() functions used, so a teardown still sees the stream closed, and an event it provokes is still dropped. test/event-queue.test.ts covers the queue directly. Each of six mutations to it was caught: settling only the first waiter, reporting done before draining the buffer, accepting a push after close, a close that is not idempotent, marking the queue closed after the teardown, and serving waiters newest first. Every existing askStream and workflowRunStream test passes unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…anches
The node:net half of IPCClient (connectWindows, connectUnixNode,
attachNetSocket, onNetSocketClosed) is what every Windows user and every
Node consumer runs. Under `bun test` on Linux and macOS every socket test
goes through connect() and so through Bun.connect, so the Linux run
SonarCloud measures never executed it. test/ipc-transport-net.test.ts now
dials both net arms by name on every platform. Node's net speaks both a
named pipe and a unix socket, and nothing public can select those arms
under Bun. Each arm is tested for a round trip with notifications, a
gateway dying under a pending call, a failed dial (ENOENT), an oversized
frame ending the socket, and disconnect() not firing onClose. Every wait
in the file is bounded: an unbounded dial that never settled wedged the
whole run instead of failing at the runner's timeout.
Branch gaps found with an istanbul pass over the suite (Bun's lcov has no
branch records) are now covered through public seams:
- validate: optional connector-health, policy, peer, connector and
namespace fields carried through; a turn without auditLogId; retrieval
without reason/partial/notes; whyPeek with null or absent
subject/pr/ticket; a non-finite lastFiredAtMs (1e999 parses to Infinity).
- The test named "accepts a null linux_helper" passed the fixture's
present helper and never took the null arm. It now sends null and
asserts it.
- agents/nimbus-client: malformed brief payloads are dropped by
subscribeAgentBrief; agentsCatchup()/agentsHuddle() with no argument
send {}; NimbusClient.open forwards requestTimeoutMs.
- ask-stream: a rejected engine.cancelStream no longer fails cancel() or
leaves an unhandled rejection; breaking out after a failed start sends
no cancelStream; a non-Error start failure is reported by its string.
- workflow-stream, discovery, mock-client: non-Error failure message;
non-object state files and bad or empty logPath; setConfig nulls an
omitted depth.
- verify:sdk: the real-filesystem defaults, a non-Error read failure, and
the entry block itself, run in a subprocess from a temp checkout whose
sibling sdk cannot be packed. Nothing is built, packed or rewritten.
Linux bun lcov (SonarCloud's input): src/ipc-transport.ts 88.24% -> 99.65%
lines. ask-stream and verify-against-local-sdk now execute every
function. Istanbul branches: validate 93.75% -> 100%, agents 76.47% ->
100%, nimbus-client 93.44% -> 100%. No production code changed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tor docs Each changed statement was re-derived from the code on this branch, from the GitHub and npm state, or from Nimbus main, not from the text it replaces. There were no delivered design specs or implementation plans to prune: this repo has never tracked one, so this commit is the whole docs pass. Wrong before this change: - CONTRIBUTING.md and the boundaries skill said Nimbus pins the client at two sites. Nimbus#1432 removed the unused root package.json pin on 2026-09-03, so packages/cli/package.json is the only one. - ROADMAP.md and the skill said the gateway serves 14 agent kinds and that exposing any of the rest is an sdk change first. It serves 17, plus agents.whyPeek. glossary, decisions and ownership have been in the sdk's AGENT_NAMES since sdk 2.0.0, so exposing those three is a client change. - The skill's agent-promotion steps described a conformance test that iterates AGENT_NAMES and an AgentParamsFor lookup. The test now iterates SUPPORTED_AGENT_NAMES and pins the unimplemented names, the dispatch is AgentParamsMap plus SUPPORTED_AGENTS, and the sdk file lives under sdks/typescript. - The skill said node-compat.test.ts was the only coverage of connectUnixNode. test/ipc-transport-net.test.ts now drives both node:net dials on every OS. - The skill's consumer-pins table (dated 2026-08-24, with 0.17.x pins and a ^1.6.0 range), its CJS bundle size (built against sdk 1.16.0) and its "two" tag rulesets (there is one) were stale. - The PR template and .coderabbit.yaml still said the sdk is consumed as the published ^1.6.0. They now describe the caret range and the floor test without a version literal for the next bump to leave behind. - SECURITY.md called the dependency pinned. It is a caret range, and the CJS bundle inlines the sdk source it was built against. - CLAUDE.md claimed a warm-standby GitLab mirror. None exists (checked with an authenticated GitLab lookup); the line came from the satellite scaffold. - The issue template's install link (nimbus-agent.dev/install) returned 404. It now points at /user-guide/install/. - The fixtures README pointed at a "branch note above" that #41 removed, and cited plan steps that no longer exist anywhere. - The README said merging the release PR tags the release and then triggers release.yml. release.yml is what tags it. Added, each checked against the source: build-first guidance in CLAUDE.md and the PR template, the pinned Bun version, nimbus-vscode as a consumer, the commit-message merge model, NIMBUS_DEMO as a third variable this client's paths.ts ignores, Bun's lcov carrying no branch records, the ruleset's up-to-date requirement, and a bounded-wait rule for socket tests. The last one was reproduced here: on Windows, Bun 1.3.14 hangs bun test on an awaited promise that never settles instead of failing it at the 5 s per-test timeout. The same probe under Linux Bun 1.3.14 fails at 5 s as expected, so the note names the platform. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test/ipc-transport-net.test.ts says every wait in it is bounded, and the dial, each call and each socket event were. The gateway accepting the connection was not: three tests awaited serveAndCapture's socket promise directly. If that accept never arrives, a test stalls instead of failing with a reason, which is the failure shape the file's within() helper was written to prevent. The accept now goes through within() as well, via a small accepted() helper, and the header comment lists what it covers. Checked against a harness mutated so the accept never resolves, on Windows and on Linux with Bun 1.3.14. Before this change each affected test failed at 2 s on an unhandled request timeout, or at the 5 s runner timeout, with nothing naming the missing accept. After it, the six tests that wait on the accept fail at 2 s with "the gateway accepting the connection did not happen within 2000ms". On Linux an induced failure there also makes the next test, the failed dial, report its ENOENT as unhandled; that happens with or without this change. Unmutated, the file passes 10 of 10 on Windows and passed 15 consecutive runs on Linux. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The boundaries skill said that on Windows, Bun 1.3.14 hangs the whole bun test run on an awaited promise that never settles. Re-run on the same Bun, that holds only when nothing else keeps the event loop busy: - Windows, a test awaiting a bare never-settling promise: the run hung until killed at 40 s. - Windows, the same kind of await with sockets still open: the 5 s per-test timeout fired and the test failed normally. - Linux: 5 s in both cases. The advice to bound every wait in a socket test stands. The sentence now says "can hang" and names the condition that was observed, so it does not overstate what was measured. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning This pull request changes a CodeRabbit configuration file. Because it comes from a fork or its author is not a repository collaborator, reviews use only the configuration from the target branch. The proposed configuration will take effect after it is merged. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. 2 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughThis change updates client stream handling, request and validation helpers, and regression tests. It also revises SDK and repository guidance, updates workflow action pins, removes Dependabot configuration, and changes selected package and documentation settings. ChangesClient runtime and verification
Repository guidance and automation
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Other Merge Risk: ⚪ Minimal · up to Disconnect continues to return a Promise, and no supported cleanup failure was found that would make it throw synchronously. No actionable merge-blocking risk is established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
…ise contract SonarCloud fails PR #93's quality gate on new_maintainability_rating with 50 typescript:S7503 issues ("Async functions should use async features"), all in src/mock-client.ts. The rule was not active when main was last analysed (2026-09-18, one code smell: S3735). This branch touches the file, so all 50 were reported, including methods on lines the branch never changed. None of those methods awaits anything: each was async only so that a fixture or a default came back wrapped in a promise. They now drop async and return Promise.resolve(...), the fix the rule documents for a method that must keep a Promise return type to satisfy an interface. Every public MockClient method is a NimbusClientLike member, so typecheck still holds each one to its Promise return type, and the emitted declarations are byte-identical to HEAD's. - agentsWhyPeek is the one body that throws by design. It now returns Promise.reject(new Error(...)), so a missing whyPeek fixture is still a rejected promise and never a synchronous throw. brief() already does the same for the nine agents.* methods. - Each stream handle's next() now wraps a synchronous nextResult() closure that advances the iteration. askStream's cancel() sets its flag and returns Promise.resolve(); workflowRunStream's resolves to the configured result. - Still async, because S7503 exempts them: the nine agents.* methods, which forward brief()'s promise, and close(), whose body is empty. For every call the declared types allow, a method resolves to the same value, or rejects with the same error, as before. Only calls the types already reject behave differently: a method called without the params object it reads, on a MockClient built with null fixtures, or detached from its instance now throws synchronously instead of rejecting, because Promise.resolve evaluates its argument before any promise exists. A new Promise((resolve) => ...) wrapper would keep even those as rejections, but that is exactly what S4634 ("Shorthand promises should be used") flags. Checked locally against the "Sonar way comprehensive" TypeScript profile, approximated by eslint-plugin-sonarjs 4.2.2 plus the eslint, typescript-eslint and unicorn rules SonarJS wraps: the same 50 S7503 lines SonarCloud reports before this change, none after, and no new finding from any rule, S7746, S4123, S4634, S6671 and S9383 included. No file this branch changes has an S9382 or S9383 finding. Red-proved by mutation, each one reverted: making agentsWhyPeek throw instead of reject fails its existing rejection test; returning a bare value fails typecheck; a changed default, an askStream that ignores cancel(), a cancel() that ignores its fixture and a dropped done event each fail an existing test; and re-adding async around Promise.resolve is reported as S7746. jscpd over src/ and scripts/, tests excluded, still finds no clone at 50 tokens. At 30 it finds one more than before: the mock's async-iterator literal now has the same tokens as ask-stream.ts's handle (5 lines, 48 tokens). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…anging its contract SonarCloud switched on five TypeScript rules on 2026-09-29: S7503, S9381, S9382, S9383 and S9380. That was after main's last analysis on 2026-09-18. A PR analysis reports only the lines the PR changes, and this branch never touches src/ipc-transport.ts, so a finding there could not show on PR #93's board. It would have appeared on main as soon as the PR merged. I swept the whole repo as Sonar scopes it: src and scripts as sources, and *.test.ts under src, scripts and test as tests. All five rules apply to main code only. The rule set is the live "Sonar way comprehensive" TypeScript profile, 495 rules. It includes S9381 and 23 others that the profile's activation=true search had omitted. The profile is approximated by eslint-plugin-sonarjs 4.2.2 plus the eslint, typescript-eslint and unicorn rules SonarJS wraps, and S9381 by a port of eslint-plugin-promise no-nesting. A canary file confirmed that all four relevant rules fire, including a floating promise whose type resolves through NimbusClient. The sweep found one finding: S7503 on `async disconnect()`. There is no S9381, S9382, S9383 or S9380 finding anywhere in the repo. disconnect() now drops async and returns Promise.resolve(). This is the remedy the rule documents for a method that must keep a Promise return type. IPCClient is a public export, so disconnect() stays Promise<void>, and every emitted .d.ts is byte-identical to before. The teardown is synchronous and cannot throw: - failAll() only settles pending promises. - Both end helpers return early on a null socket. - Ending a live socket reports errors through callbacks or 'error' events, not exceptions. The method resolves to the same undefined at the same point. The one behaviour change is a detached call with no `this`. The types allow it, but nothing makes one. It now throws synchronously instead of rejecting. The no-op disconnect test in test/ipc-transport.test.ts had a comment saying that a missing null guard makes the second call reject. Now the call throws instead. The comment says so, and the assertion still fails in that case. Red-proved by mutation, each one reverted: - Dropping failAll() fails "disconnect() rejects in-flight calls". - Dropping the Promise.resolve() return fails typecheck with TS2355. - Dropping both null guards fails the no-op test with a TypeError. The sweep also flagged seven lines under other rules. None of them is a SonarCloud issue. Each line was already on main before its last analysis, each rule had been active since 2021 or 2025-09-26, and main lists none of them as open or resolved. Six disappear once the profile's own parameters are applied: - S107 allows up to 7 parameters. - S7718's ignore list accepts `e` and `err`. - S1186 exempts arrow functions. The seventh, S7721 on ask-stream's matchesStream, is identical on main and SonarCloud never raised it. Checks: bun run build, typecheck, lint, and bun test both plain and with CI=true: 574 pass, 1 skip, 0 fail. Also run: ci.yml's CJS require smoke under Node and its no-embedded-path check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
89784b6 justified dropping `async` from disconnect() with "The teardown is synchronous ... so there is nothing to await". That holds for the steps disconnect() itself runs. It does not hold uniformly for the socket close those steps start, whose timing depends on the transport arm. Measured with an idle socket under Bun 1.3.14 on Windows and on Linux (WSL2), and under Node 22 and 24: - Bun's native socket (Bun.connect, the Linux/macOS arm under Bun) fires its close handler inside end(), before end() returns. - node:net (the Windows arm, and the Linux/macOS arm under Node) fires 'close' only after end() returns, under both Bun and Node. The comment now says that every step is synchronous and none of them throws. It also says that disconnect() does not wait for the close event on either arm, and why the notification is consumed first. src/ipc-transport-close.test.ts justified its 50 ms sleep with "The socket teardown disconnect() triggers is asynchronous". That holds only on the Windows arm, so the comment now says which arm does what, and why the sleep still matters. That ordering is enforced by the suite only on the Bun arm. Moving `closeNotified = true` after the teardown fails "does NOT fire on a deliberate disconnect()" on Linux (expected 0 calls, received 1). It passes on Windows, where the close event arrives after closeNotified has been set regardless. Comment-only. No code, types or emitted declarations change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A maintenance sweep of
@nimbus-dev/clientwith no change to the published API or to runtime behaviour.dist/index.d.tsanddist/index.jsare byte-identical to a build ofmain, and the CJS and ESM entry points export the same 18 names under Node. The only runtime dependency,@nimbus-dev/sdk, keeps its^2.0.0range.@biomejs/biometo 2.5.15, refreshes the lockfile, and moves every GitHub Action pin to its latest release.src/andscripts/: 3 clones to 0 at 50 tokens.src/ipc-transport.tsgoes from 88.24% to 99.65% line coverage on Linux, and all source files from 98.56% to 99.88%.This repo keeps commit messages on squash, so each commit's message explains that commit's reasoning.
Dependabot
.github/dependabot.ymlis deleted. The rules its comments encoded now live in CONTRIBUTING.md § Updating dependencies: commitbun.lockwithpackage.json, give the sdk its own PR, and move CodeQL init and analyze together.cla.yml, the Dependabot-only skip step and its two!= 'dependabot[bot]'conditions are gone. Those conditions were true for every other run, so human PRs and sign/recheck comments behave as before. Theclajob id, triggers and job condition are unchanged. A comment says to restore the skip before turning Dependabot back on.Dependencies
@biomejs/biome(dev)biome.json$schema@types/node(transitive)undici-types(transitive)github/codeql-actioninit + analyzeSonarSource/sonarqube-scan-actionnimbus-agent/.githubverify-npm-provenance5fb42797592125, the tip ofmainoven-sh/setup-bun# v2→# v2.2.0Already at latest and unchanged:
@nimbus-dev/sdk2.0.0,@types/bun1.4.2,typescript7.0.2, plus the harden-runner, checkout, setup-node, create-github-app-token, CLA assistant and release-please actions. Every pinned SHA matches the tag in its comment.Held back:
packageManager,.bun-versionand three workflows'bun-versioninput; the latest is 1.4.2. This is a toolchain pin, not a package. The Nimbus monorepo's CI runs Bun 1.3, so moving this repo alone would put the two out of step. That needs a maintainer decision.undici-types8.11.2: transitive, and pinned by@types/node26.6.4 as~8.9.0.engines.node >= 18, although every sdk the range can resolve declares Node 22. Changing it is a public-metadata and semver decision; the repo skill documents the gap.SonarCloud
typescript:S3735atsrc/agents.ts:76, "Remove this use of the void operator", the only open issue. Thevoidexisted only to keep a never-called function that checked, at compile time, that every supported agent is still an sdkAgentName. That check now lives in the type itself, asKnownToTheSdk<keyof AgentParamsMap>. It emits nothing at runtime and still fails to compile for an agent name the sdk does not know.sonar-project.propertiesis unchanged.typescript:S7503(async method withoutawait) insrc/mock-client.ts. The rule entered the project's profile on 2026-09-29, aftermainwas last analysed, and this branch touches that file. All 50 are fixed in code, with no suppression: 45 methods now returnPromise.resolve(...)withoutasync,agentsWhyPeekreturnsPromise.reject(...)so a missing fixture still rejects, and the two stream handles wrap a synchronous step. The 12 emitted.d.tsfiles are byte-identical, so the published types are unchanged. One deliberate difference: a call the types already forbid now throws synchronously rather than rejecting. Before that fix, a local run of the profile (eslint-plugin-sonarjs plus the eslint, typescript-eslint and unicorn rules SonarJS wraps) had found nothing new, because sonarjs has no native S7503.Duplication
jscpd 4 over
src/andscripts/, tests excluded, with--max-lines 100000 --max-size 10mb. Without those two flags, jscpd's defaults silently skipnimbus-client.tsandvalidate.ts.mainSonarCloud already reported 0 duplication groups on
main. What is now shared:queryItems' parameter and result types, written out four times, areQueryItemsParamsandQueryItemsResult. They are not exported from the package entry, so the public surface is unchanged.searchRankedandsearchRankedWithRetrievalbuild their wire params in one place.workflowRunandworkflowRunStreamshareworkflowRunWireParams. Before, a field added toWorkflowRunParamswould have been silently dropped by the stream path.MockClient's default workflow run is defined once.validate.ts,requireOkandstrArrreplace three and two copies. New tests pin the order the checks run in.askStreamandworkflowRunStreamshare one push-to-pull queue,createEventQueueinsrc/event-queue.ts, which has its own tests.Left as is: the two remaining 30-token clones. One is a one-line predicate, and sharing it would add an import cycle. The other is
index.ts's export list mirroring an import list. Two string-array checks also stay separate, because their error messages differ and callers can see the difference.Coverage
Bun's lcov on Linux with Bun 1.3.14, which is what SonarCloud reads. Source files only:
mainsrc/ipc-transport.tssrc/ask-stream.tsscripts/verify-against-local-sdk.tssrc/event-queue.tssrc/paths.tsThe biggest gap was the
node:nethalf of the transport, which every Windows user and every Node consumer runs and which no Linuxbun testrun reached.test/ipc-transport-net.test.tsnow drives bothnetdials on every OS, with every connection wait bounded.Bun's lcov has no branch records, so a branch-aware istanbul pass was used to find untested branch arms. New tests cover those arms in validate, agents, ask-stream, workflow-stream, discovery, MockClient and the
verify:sdkscript. One existing test, "accepts a null linux_helper", never actually passed a null; it now does and asserts the result. Passing tests go from 527 to 574, with 1 skip:node-compat, which needsNIMBUS_GATEWAY_BIN.Still uncovered:
ipc-transport.tsline 204, Bun's socketerrorcallback: it cannot be reached without a real socket fault.paths.tsline 67: a Bun line-counting quirk; istanbul reports the file at 100%.verify-against-local-sdk.tsline 236, theimport.meta.mainblock: it is now exercised in a subprocess, which Bun's coverage cannot see.Docs
packages/cli, not two.agents.whyPeek.glossary,decisionsandownershipare already in the sdk'sAGENT_NAMES, so exposing them is a client change..coderabbit.yamlno longer quote the stale^1.6.0range.nimbus-vscodelisted as a consumer.NIMBUS_DEMOnoted as a variable this client'spaths.tsdoes not read.main.Verification
All on the branch tip, with
CI=true:git statusis clean afterwards. The two Node checks are read out ofci.ymlitself, plus a negative control: the embedded-path check fails on a plantedfile:///path.dist/index.d.tsanddist/index.jsare byte-identical tomain's build, with the same 18 export names under Node for both CJS and ESM.dist/index.cjsgoes from 98,298 to 97,574 bytes.build-testlegs,Analyze (javascript-typescript),SonarQube Cloud analysisandcla..catchthat absorbs a failedengine.cancelStreamfails both new cancel tests. With a test server that never accepts, the six net tests that wait on the accept now fail at 2 s and say why.Caveats
main'sdependabot.ymluntil this merges. It last opened PRs on 2026-09-24; close any it opens before then.mainalready declares@nimbus-dev/sdk^2.0.0from chore(deps): bump @nimbus-dev/sdk from 1.34.0 to 2.0.0 #88, while the published 0.18.0 declares^1.34.0. Users get that only with the next release. Whether that release should be a patch or a minor is a maintainer decision, since sdk 2.0.0 widens the re-exportedAgentName.node:netpath, a failedconnect()also fires anyonClosehandlers registered before it. The Bun unix-socket path does not..d.tsfiles of non-entry modules change in text but not in types, and the tarball gainsdist/event-queue.*. None of these is reachable through theexportsmap.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation
Tests