packages/bruno-js/src/runtime/vars-runtime.js (1)

40-40: 14 positional args to Bru is getting unwieldy.

The change itself is correct — passing request?.url aligns with the other runtime call sites. However, this constructor now takes 14 positional arguments with undefined placeholders for unused params. Consider moving to an options object in a follow-up to prevent ordering mistakes.

packages/bruno-js/src/bru.js (1)

31-31: 14 positional constructor parameters — existing debt, but worth noting.

This constructor keeps growing. A single options/config object would make call sites far more readable and resilient to parameter ordering bugs. I see this is pre-existing debt (flagged in script-runtime.js:15-16), so not blocking.

packages/bruno-js/src/readonly-property-list.js (2)

157-159: reduce() behaves differently from native Array.reduce when called without an accumulator.

list.reduce(fn) passes undefined as the initial value to Array.prototype.reduce, which differs from calling items.reduce(fn) directly (native reduce without a second arg uses the first element as the initial accumulator and throws on empty arrays). If this is intentional to mirror Postman's PropertyList, it's fine — just worth a note.

Optional: match native semantics

- reduce(fn, acc) {
-   return this._getItems().reduce(fn, acc);
+ reduce(fn, ...args) {
+   return this._getItems().reduce(fn, ...args);
  }

---

36-38: Dynamic mode: _dataSource() errors will bubble up unhandled.

If a dataSource function throws, every read method will propagate the exception. This is likely fine for the cookie use case (where getCookiesForUrl is trusted internal code), but worth noting if this base class will be used more broadly.

packages/bruno-js/src/cookie-list.js (1)

39-75: Write methods are clean — consistent null-URL guard pattern.

Each method properly short-circuits on falsy URL with callback/Promise fallback. The delete → remove alias is straightforward.

One small note: these public methods lack individual JSDoc (only the class-level doc lists them). Consider adding brief JSDoc per method. As per coding guidelines: "Add JSDoc comments to abstractions for additional details."

packages/bruno-js/src/property-list.js (2)

124-136: populate() and repopulate() are identical implementations.

Both methods assign this._items = Array.isArray(items) ? [...items] : []. Since JS assignment is atomic, the "clear then populate" semantics of repopulate collapse to the same operation. If this mirrors Postman's SDK surface, keeping both is fine for API compatibility, but a deduplication would be cleaner.

Optional: deduplicate

  populate(items) {
    this._assertStatic('populate');
    this._items = Array.isArray(items) ? [...items] : [];
  }

  repopulate(items) {
    this._assertStatic('repopulate');
-   this._items = Array.isArray(items) ? [...items] : [];
+   this.populate(items);
  }

---

66-76: insertAfter() silently appends when after is undefined, unlike insert() which handles it explicitly.

insert(item, before) explicitly checks before === undefined and appends. insertAfter(item, after) doesn't — it falls through to indexOf(undefined) === -1 → append. The behavior is the same, but the intent is implicit. Consider adding an explicit guard for consistency, or documenting the fallback.

packages/bruno-js/src/bru.js (1)

32-32: 14 positional constructor parameters is getting unwieldy.

This isn't new to this PR, but adding requestUrl as the 14th positional arg increases the risk of callers passing arguments in the wrong order. Consider consolidating into an options object in a follow-up.

packages/bruno-js/src/sandbox/quickjs/shims/bru.js (1)

550-580: Duplicate callWithCallback definition — consider extracting to a shared helper.

callWithCallback is defined identically both inside the block scope (Line 553) and in the jar() function (Line 585). This isn't a bug due to block scoping, but it's copy-pasted logic.