Skip to content

feat(workspace): add load tracing, load-failure issues and debug-load command#10418

Open
davidfirst wants to merge 11 commits into
masterfrom
component-load-observability
Open

feat(workspace): add load tracing, load-failure issues and debug-load command#10418
davidfirst wants to merge 11 commits into
masterfrom
component-load-observability

Conversation

@davidfirst

Copy link
Copy Markdown
Member

Phase 1 (observability) of the component-loading redesign — see scopes/workspace/workspace/component-loading-redesign.md for the full multi-phase plan.

  • New @teambit/harmony.modules.load-trace module: AsyncLocalStorage-based trace context. Every top-level component/aspect load gets a trace id; nested loads join as child spans, so BIT_LOG=* output reads as a tree (replaces the ad-hoc callId in the aspects loader).
  • Stage-level timing spans across the workspace/scope/legacy loaders (FS load, extension merge, env calc, dependency resolution, per-aspect onLoad handlers) with cache hit/miss attributes.
  • New LoadFailures component issue (non-tag-blocking): aspect/env load errors that were previously logged-and-ignored now surface in bit status, attached centrally via aspectLoader.handleExtensionLoadingError and loadCompsAsAspects. Control flow unchanged — previously-swallowed errors are still non-fatal, just visible.
  • New bit debug-load <id> command: loads a component fresh with tracing and prints the stage/timing tree, the extension-merge source table (which config source won each extension), the resolved env and its origin, and load issues. Supports --json.

e2e: load-failures-issue.e2e.ts, debug-load.e2e.ts. No behavioral changes to loading itself.

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 11, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (5) 📘 Rule violations (1)

Grey Divider


Action required

1. Scoped id version stripping 🐞 Bug ≡ Correctness
Description
DebugLoadCmd strips versions using id.split('@')[0], which returns an empty string for
npm-scoped ids like @scope/pkg@1.2.3, causing extension/env origin matching to collapse and
misattribute sources in bit debug-load output.
Code

scopes/workspace/workspace/debug-load.cmd.ts[R12-13]

+/** strip the version suffix from a raw id string (ids here are raw `stringId`s, not ComponentIDs). */
+const noVersion = (id: string) => id.split('@')[0];
Evidence
The current implementation explicitly uses split('@')[0] for version stripping and relies on it
for equality checks when attributing origins, which fails for npm-scoped ids that themselves start
with @.

scopes/workspace/workspace/debug-load.cmd.ts[12-13]
scopes/workspace/workspace/debug-load.cmd.ts[102-116]
scopes/workspace/workspace/debug-load.cmd.ts[123-135]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DebugLoadCmd` uses `id.split('@')[0]` to remove a version suffix. This breaks for npm-scoped package ids (e.g. `@scope/pkg@1.2.3`), where splitting on the first `@` yields an empty string, causing incorrect matching when attributing extension sources and env origin.
### Issue Context
The bug impacts `buildExtensionSources()` and `findEnvOrigin()` matching logic, and can produce wrong/missing origin attribution in `bit debug-load` output.
### Fix Focus Areas
- scopes/workspace/workspace/debug-load.cmd.ts[12-13]
- scopes/workspace/workspace/debug-load.cmd.ts[102-117]
- scopes/workspace/workspace/debug-load.cmd.ts[123-135]
### Suggested fix
Replace `id.split('@')[0]` with a helper that strips only a trailing version segment by using the **last** `@`:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Load failures misattributed 🐞 Bug ≡ Correctness
Description
WorkspaceComponentLoader.attachReportedLoadFailures() uses id.split('@')[0] when matching
failures to component/used ids; for npm-scoped ids like @scope/pkg@1.2.3 this collapses to '',
causing false matches that can attach/aggregate a failure to the wrong components.
Code

scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[R551-572]

+  private attachReportedLoadFailures(components: Component[]) {
+    if (this.workspace.inInstallContext) return;
+    const failures = currentLoadTrace()?.loadFailures;
+    if (!failures?.length) return;
+    const noVersion = (id: string) => id.split('@')[0];
+    components.forEach((component) => {
+      const componentIdStr = component.id.toString();
+      const usedIds: string[] = component.state._consumer
+        ? component.state._consumer.extensions.map((ext) => ext.stringId)
+        : [];
+      // the env may be set via the EnvsAspect config rather than as a direct extension entry, in
+      // which case its id won't appear in the extensions list above. include it explicitly so a
+      // failure loading that env still gets aggregated to this component.
+      const envId = component.state.aspects.get(EnvsAspect.id)?.data?.id;
+      if (envId) usedIds.push(envId);
+      failures.forEach((failure) => {
+        if (noVersion(failure.failedId) === noVersion(componentIdStr)) {
+          this.addLoadFailureIssue(component, failure.failedId, failure.phase, failure.error);
+          return;
+        }
+        if (usedIds.some((usedId) => noVersion(usedId) === noVersion(failure.failedId))) {
+          this.workspace.registerAggregatedLoadFailure(failure, componentIdStr);
Evidence
The matching uses the same faulty version-stripping logic as debug-load, and extension ids can be
raw strings from config (including npm-scoped packages). The codebase itself recognizes that
split('@')[0] is not valid for package ids in other areas (it branches on __type === 'package').

scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[551-573]
components/legacy/extension-data/extension-data.ts[22-34]
components/legacy/extension-data/extension-data-list.ts[194-200]
scopes/component/component-compare/component-compare.main.runtime.ts[159-166]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`attachReportedLoadFailures()` attempts to compare ids ignoring versions using `id.split('@')[0]`. For npm-scoped ids (`@scope/pkg@1.2.3`), this returns `''`, so comparisons become unreliable and can incorrectly aggregate/attach load failures.
### Issue Context
Load failures (`reportLoadFailure`) carry a `failedId` string. This string may represent an aspect/env identified by a raw string id (not necessarily a Bit `ComponentID`). `ExtensionDataEntry.stringId` can be such a raw string, so matching logic must handle npm package naming.
### Fix Focus Areas
- scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[551-573]
### Suggested fix
- Replace the local `noVersion = (id) => id.split('@')[0]` with a safe `stripVersionSuffix(id: string)` implementation (remove only the last `@...` segment, and only when it’s not the leading `@` of a scoped package).
- Example:
- `const idx = id.lastIndexOf('@');`
- `if (idx <= 0) return id;`
- `return id.slice(0, idx);`
- Use this helper consistently for `failure.failedId`, `componentIdStr`, and all `usedIds`.
- Consider adding a small unit test for `stripVersionSuffix()` with:
- `teambit.react/react@1.0.0` → `teambit.react/react`
- `@scope/pkg@1.2.3` → `@scope/pkg`
- `@scope/pkg` → `@scope/pkg`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Load failures block tagging ✓ Resolved 🐞 Bug ≡ Correctness
Description
Workspace.getWorkspaceIssues() now converts aggregated load failures into workspace issues (Error
objects), and the tag/snap flow calls BuilderMain.throwForComponentIssues(), which throws when any
workspace issue exists. As a result, previously-swallowed aspect/env failures that are aggregated at
workspace level can now abort bit tag/bit snap.
Code

scopes/workspace/workspace/workspace.ts[R1733-1740]

+    this.aggregatedLoadFailures.forEach((failure) => {
+      const affected = failure.affected.size === 1 ? '1 component' : `${failure.affected.size} components`;
+      errors.push(
+        new Error(
+          `failed loading "${failure.failedId}" (during ${failure.phase}), affects ${affected}: ${failure.error}. the load continued without it, data computed by it may be missing`
+        )
+      );
+    });
Evidence
The PR adds aggregated load failures to Workspace.getWorkspaceIssues(). Tag/snap invokes
SnappingMain.throwForVariousIssues(), which calls BuilderMain.throwForComponentIssues(), and
that method throws when workspace.getWorkspaceIssues() returns any entries—making these
newly-added aggregated load-failure entries fatal to tag/snap.

scopes/workspace/workspace/workspace.ts[1720-1741]
scopes/pipelines/builder/builder.main.runtime.ts[518-538]
scopes/component/snapping/snapping.main.runtime.ts[240-266]
scopes/component/snapping/snapping.main.runtime.ts[908-913]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Workspace.getWorkspaceIssues()` now pushes aggregated load-failure messages (from `aggregatedLoadFailures`) into the returned `Error[]`. The tag/snap pipeline (via `SnappingMain.throwForVariousIssues()` → `BuilderMain.throwForComponentIssues()`) treats any workspace issue as fatal and throws a `BitError`. This turns best-effort (swallowed) load failures into tag/snap blockers.
## Issue Context
- The new aggregated load failures are meant to reduce per-component noise in `bit status`, but the implementation uses the existing `workspace issues` mechanism, which is also used as a hard gate for tag/snap.
- The new per-component `LoadFailures` issue is explicitly non-tag-blocking, but the *workspace-level* aggregation path is currently tag-blocking because it is represented as plain `Error` objects.
## Fix Focus Areas
- scopes/workspace/workspace/workspace.ts[1720-1764]
- scopes/pipelines/builder/builder.main.runtime.ts[518-538]
- scopes/component/snapping/snapping.main.runtime.ts[240-266]
- scopes/component/snapping/snapping.main.runtime.ts[908-913]
## Suggested fix direction
Implement a non-blocking workspace-issue channel for aggregated load failures, and ensure the builder/tag/snap gate only blocks on truly blocking workspace issues (e.g. merge-config conflicts):
1. **Do not include aggregated load failures in `getWorkspaceIssues()`** (or add a parameter such as `getWorkspaceIssues({ includeNonBlocking?: boolean })`).
2. Expose aggregated load failures separately (e.g. `getWorkspaceWarnings()` / `getNonBlockingWorkspaceIssues()`), and have `bit status` render that list in the workspace-issues section (or a new section), while the builder/tag/snap gate ignores it.
3. If you must keep them in `getWorkspaceIssues()`, introduce a typed workspace-issue object with a `isTagBlocker` flag and update `BuilderMain.throwForComponentIssues()` to filter out non-blocking workspace issues.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (4)
4. debug-load env data crash ✓ Resolved 🐞 Bug ≡ Correctness
Description
DebugLoadCmd.gatherData() reads component.state.aspects.get(EnvsAspect.id)?.data.id, which will
throw when the aspect entry exists but data is undefined. This can happen because aspect entries
are upserted with handler return values that may be undefined, causing bit debug-load to fail
unexpectedly.
Code

scopes/workspace/workspace/debug-load.cmd.ts[R76-82]

+    const extensionSources = this.buildExtensionSources(mergeRes.extensions, mergeRes.beforeMerge);
+    // the merge returns an envId only when set via the envs aspect config. when the env is set as
+    // a direct extension (e.g. via variants) or not set at all, take it from the loaded
+    // component's envs data, and attribute it by searching the merge sources for it.
+    const envFromLoad = component.state.aspects.get(EnvsAspect.id)?.data.id;
+    const envId = mergeRes.envId || envFromLoad;
+    const envOrigin = this.findEnvOrigin(mergeRes.beforeMerge, envId);
Evidence
debug-load dereferences .data.id without guarding .data, and the loader code shows that aspect
entries are upserted using handler-returned data values (which may be undefined), making the crash
path feasible.

scopes/workspace/workspace/debug-load.cmd.ts[76-82]
scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[1086-1092]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`bit debug-load` can crash due to `?.data.id` (only guards the map lookup, not the `data` field). If the EnvsAspect entry exists but its data is undefined, accessing `.id` throws.
### Issue Context
Aspect entries can be updated with arbitrary handler return values, including `undefined`. The debug-load command should be resilient and never crash while trying to *explain* a load.
### Fix Focus Areas
- scopes/workspace/workspace/debug-load.cmd.ts[76-83]
- scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[1086-1092]
### Suggested fix
- Change `component.state.aspects.get(EnvsAspect.id)?.data.id` to `component.state.aspects.get(EnvsAspect.id)?.data?.id`.
- Consider additionally wrapping the env extraction in a small helper that tolerates unexpected shapes (e.g. `typeof data === 'object' && data && 'id' in data`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. debug-load hardcoded chalk formatting 📘 Rule violation ⚙ Maintainability
Description
The new DebugLoadCmd renders CLI output using direct chalk.* styling and a hardcoded Unicode
arrow () instead of relying solely on the shared CLI output formatter. This can drift from the
CLI output style guide and breaks consistency across commands.
Code

scopes/workspace/workspace/debug-load.cmd.ts[R134-172]

+  private renderStagesSection(data: DebugLoadData): string {
+    if (!data.trace) return '';
+    const lines: string[] = [];
+    const renderSpan = (span: Record<string, any>, depth: number) => {
+      const duration = span.durationMs !== undefined ? chalk.cyan(`${span.durationMs}ms`) : chalk.dim('n/a');
+      const attrs = span.attributes ? chalk.dim(` ${JSON.stringify(span.attributes)}`) : '';
+      lines.push(`   ${'  '.repeat(depth)}${span.name} ${duration}${attrs}`);
+      (span.children || []).forEach((child: Record<string, any>) => renderSpan(child, depth + 1));
+    };
+    renderSpan(data.trace, 0);
+    return [formatTitle('load stages (timings and cache hits)'), ...lines].join('\n');
+  }
+
+  private renderExtensionSourcesSection(data: DebugLoadData): string {
+    if (!data.extensionSources.length) return '';
+    const items = data.extensionSources.map((row) => {
+      const alsoIn = row.alsoIn.length ? chalk.dim(` (also in: ${row.alsoIn.join(', ')})`) : '';
+      return formatItem(`${row.extensionId} ${chalk.dim('←')} ${chalk.green(row.winner)}${alsoIn}`);
+    });
+    return [formatTitle(`extension sources (${items.length})`), ...items].join('\n');
+  }
+
+  private renderEnvSection(data: DebugLoadData): string {
+    const origin = data.envOrigin || 'computed during load (not set in config)';
+    const envLine = data.envId
+      ? formatItem(`${chalk.cyan(data.envId)} ${chalk.dim('determined by')} ${chalk.green(origin)}`)
+      : formatItem(chalk.dim('no env configured'));
+    return [formatTitle('environment'), envLine].join('\n');
+  }
+
+  private renderIssuesSection(data: DebugLoadData): string {
+    if (!data.issues.length) {
+      return [formatTitle('load issues'), formatItem(chalk.dim('none'))].join('\n');
+    }
+    const items = data.issues.map((issue) =>
+      formatItem(`${chalk.yellow(issue.type)}: ${issue.description} ${chalk.dim(JSON.stringify(issue.data))}`)
+    );
+    return [formatTitle(`load issues (${items.length})`), ...items].join('\n');
+  }
Evidence
PR Compliance ID 1 forbids hardcoded chalk styles/symbols for CLI output in favor of the shared
output formatter. In DebugLoadCmd, the new rendering functions apply direct chalk styling and
embed a Unicode arrow () in the output strings.

CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols): CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols)
scopes/workspace/workspace/debug-load.cmd.ts[134-172]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DebugLoadCmd` uses direct `chalk` styling (e.g. `chalk.cyan`, `chalk.dim`, `chalk.green`, `chalk.yellow`) and hardcoded symbols (e.g. `←`) in its human output rendering. Per the CLI output compliance rule, section titles/symbols and styling should follow the CLI output style guide and use the shared output formatter utilities from `@teambit/cli`, avoiding ad-hoc chalk and symbols.
## Issue Context
The command already uses `formatTitle`, `formatItem`, and `joinSections` from `@teambit/cli`, but still applies manual coloring and symbols inside the content lines.
## Fix Focus Areas
- scopes/workspace/workspace/debug-load.cmd.ts[134-172]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Ungated span log cost ✓ Resolved 🐞 Bug ➹ Performance
Description
The span emitter always JSON.stringifys attributes and formats messages for every closed span before
calling the trace logger, so this CPU work happens even when trace logging is disabled. In the
component-load hot path (spans are always collected), this can add significant overhead without
producing any output.
Code

components/legacy/logger/logger.ts[R318-325]

+// emit closed load-trace spans to the log at trace level. the emitter fires after the span's
+// async context has exited, so the message carries the trace-id and span path explicitly
+// (bypassing the prefix in BitLogger methods to avoid double-prefixing).
+setSpanEmitter((span, traceId) => {
+  const attrs = Object.keys(span.attributes).length ? ` ${JSON.stringify(span.attributes)}` : '';
+  const duration = span.durationMs !== undefined ? `${span.durationMs.toFixed(2)}ms` : 'n/a';
+  logger.logger.trace(`load-trace [trace:${traceId}] ${span.path}: ${duration}${attrs}`);
+});
Evidence
The emitter formats/serializes span data unconditionally before issuing a trace log call, so the
overhead occurs even when the logger level suppresses trace output (default level is debug).

components/legacy/logger/logger.ts[31-35]
components/legacy/logger/logger.ts[318-325]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`setSpanEmitter` builds strings and `JSON.stringify`s attributes unconditionally for every closed span, even when the underlying pino logger is not configured to output `trace` level logs. This adds avoidable overhead to the load path.
## Issue Context
Span collection is always-on; span emission is intended to be trace-level only. The current code performs expensive work regardless of whether `trace` output is enabled.
## Fix Focus Areas
- components/legacy/logger/logger.ts[318-325]
### Suggested fix
Add an early return in the emitter (or only register the emitter) when `logger.logger` is not trace-enabled, e.g. check `logger.logger.level`/equivalent before computing `attrs`/`duration`/message. Ensure `JSON.stringify(span.attributes)` is only executed when the trace call will actually be emitted.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Env failures may be missed ✓ Resolved 🐞 Bug ≡ Correctness
Description
WorkspaceComponentLoader.attachReportedLoadFailures only associates a failure with a component
when failure.failedId matches one of the component's extension entry ids (or the component id). If
an env is configured via the EnvsAspect config (where env id is handled separately), a
require-aspects failure for that env id may not match and therefore may not surface as a
LoadFailures issue.
Code

scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[R548-563]

+  private attachReportedLoadFailures(components: Component[]) {
+    if (this.workspace.inInstallContext) return;
+    const failures = currentLoadTrace()?.loadFailures;
+    if (!failures?.length) return;
+    const noVersion = (id: string) => id.split('@')[0];
+    components.forEach((component) => {
+      const extensionIds: string[] = component.state._consumer
+        ? component.state._consumer.extensions.map((ext) => ext.stringId)
+        : [];
+      const relevantIds = [...extensionIds, component.id.toString()].map(noVersion);
+      failures.forEach((failure) => {
+        if (relevantIds.includes(noVersion(failure.failedId))) {
+          this.addLoadFailureIssue(component, failure.failedId, failure.phase, failure.error);
+        }
+      });
+    });
Evidence
The attachment logic only considers extension entry ids, while the workspace code explicitly models
env id as a separate concept (and the debug-load command notes that envId is returned only when set
via EnvsAspect config), so a failure for the env id can be missed if it isn't present as an
extension id string.

scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[548-563]
scopes/workspace/workspace/workspace.ts[155-160]
scopes/workspace/workspace/debug-load.cmd.ts[75-81]
scopes/workspace/workspace/debug-load.cmd.ts[125-131]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`attachReportedLoadFailures()` matches failures only against `component.state._consumer.extensions[*].stringId` plus the component id. For envs that are set via the EnvsAspect config (tracked as `envId` separately), the env id may not appear as an extension entry id, so failures reported with `failedId=<envId>` can be skipped and never attached as `IssuesClasses.LoadFailures`.
## Issue Context
The codebase itself documents that `envId` is returned separately when set via EnvsAspect config, and `findEnvOrigin()` explicitly inspects `envsAspect.config.env`.
## Fix Focus Areas
- scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[548-563]
- scopes/workspace/workspace/debug-load.cmd.ts[75-81]
- scopes/workspace/workspace/debug-load.cmd.ts[125-131]
- scopes/workspace/workspace/workspace.ts[155-160]
### Suggested fix
Augment `relevantIds` with the env id when present, e.g.:
- Extract env id from `component.state._consumer.extensions.findCoreExtension(EnvsAspect.id)?.config.env` (or from `component.state.aspects.get(EnvsAspect.id)?.data.id` when available), normalize with `noVersion`, and include it in the `relevantIds` set.
This ensures failures reported as `failedId=<envId>` get attached even when the env is configured via EnvsAspect config rather than as a direct extension entry.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

8. Stale aggregated load failures 🐞 Bug ☼ Reliability ⭐ New
Description
Workspace-level aggregated load failures are stored in a long-lived Map and only cleared via
clearAllComponentsCache(), so in long-running processes earlier failures can continue to appear in
later bit status output even after the underlying issue is fixed. Additionally, aggregation is
keyed only by failedId, so the displayed phase/error can be whichever was recorded first and
become misleading if later reports differ.
Code

scopes/workspace/workspace/workspace.ts[R1761-1769]

+  private aggregatedLoadFailures: Map<string, LoadFailure & { affected: Set<string> }> = new Map();
+
+  registerAggregatedLoadFailure(failure: LoadFailure, affectedComponentId: string) {
+    // keyed by the failing id only: the same root cause may be reported from several load phases,
+    // and it should still render as a single workspace issue.
+    const existing = this.aggregatedLoadFailures.get(failure.failedId);
+    if (existing) existing.affected.add(affectedComponentId);
+    else this.aggregatedLoadFailures.set(failure.failedId, { ...failure, affected: new Set([affectedComponentId]) });
+  }
Evidence
The aggregated failures are surfaced by getWorkspaceIssues() and not cleared anywhere except
clearAllComponentsCache(), which means the state can persist across multiple commands in
long-running modes (interactive CLI/server). The aggregation also only keys by failedId, so later
reports can’t update the stored phase/error.

scopes/workspace/workspace/workspace.ts[1721-1751]
scopes/workspace/workspace/workspace.ts[830-836]
scopes/workspace/workspace/workspace.ts[1761-1769]
scopes/harmony/cli/cli.cmd.ts[74-101]
scopes/harmony/api-server/cli-raw.route.ts[47-99]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Workspace.aggregatedLoadFailures` persists across loads/commands unless `clearAllComponentsCache()` is called. In long-running Bit processes (e.g. `bit cli`, cli-raw server), this can cause stale/misleading workspace issues.

### Issue Context
- The map is appended to via `registerAggregatedLoadFailure()` and read by `getWorkspaceIssues()`.
- It is only cleared in `clearAllComponentsCache()`.
- The aggregation key is currently only `failedId`, so `phase`/`error` come from the first recorded failure.

### How to fix
- Clear `aggregatedLoadFailures` at the beginning of each *top-level* load request (e.g. when starting a new `startOrJoinLoadTrace` root for workspace commands) or at the beginning of commands that render workspace issues (e.g. status).
- Consider keying by `failedId + phase` (or storing a set of phases/errors) so the rendered message can’t become incorrect when the same failing id is reported from multiple phases.

### Fix Focus Areas
- scopes/workspace/workspace/workspace.ts[830-845]
- scopes/workspace/workspace/workspace.ts[1728-1751]
- scopes/workspace/workspace/workspace.ts[1761-1769]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. debug-load splits trace 🐞 Bug ◔ Observability ⭐ New
Description
DebugLoadCmd starts a trace only around workspace.get(), but then runs workspace.scope.get()
and workspace.componentExtensions() after the trace ends, so these stages won’t be correlated
under the same trace id/span tree. Because scope.get() starts its own trace when none is active,
debug-load can emit multiple unrelated trace ids, undermining the command’s purpose.
Code

scopes/workspace/workspace/debug-load.cmd.ts[R69-78]

+    let trace: LoadTrace | undefined;
+    const component: Component = await startOrJoinLoadTrace('debug-load', { id: componentId.toString() }, async () => {
+      trace = currentLoadTrace();
+      return this.workspace.get(componentId);
+    });
+
+    const componentFromScope = await this.workspace.scope.get(componentId);
+    const mergeRes = await this.workspace.componentExtensions(componentId, componentFromScope, undefined, {
+      loadExtensions: false,
+    });
Evidence
The debug-load trace scope ends before workspace.scope.get() is executed; since scope.get()
itself starts or joins a trace, it will start a new trace when called outside any active trace
context, leading to multiple trace ids and missing correlation in debug-load output.

scopes/workspace/workspace/debug-load.cmd.ts[60-87]
scopes/scope/scope/scope-component-loader.ts[27-31]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`bit debug-load` wraps only `workspace.get()` in `startOrJoinLoadTrace()`, but subsequent load-related calls happen outside that trace. This fragments correlation and can create an extra trace id for `scope.get()`.

### Issue Context
- `ScopeComponentLoader.get()` always calls `startOrJoinLoadTrace('scope.get', ...)`.
- When invoked after the `debug-load` trace scope has exited, it will start a new trace.

### How to fix
- Move `workspace.scope.get()` and `workspace.componentExtensions()` into the same `startOrJoinLoadTrace('debug-load', ...)` callback so they become child spans of the debug-load trace.
- Optionally wrap those steps in `loadSpan('scope-load-for-debug', ...)` / `loadSpan('extension-merge-for-debug', ...)` so they appear as explicit stages in the printed tree.

### Fix Focus Areas
- scopes/workspace/workspace/debug-load.cmd.ts[69-87]
- scopes/scope/scope/scope-component-loader.ts[27-58]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


10. No-trace spans still timed 🐞 Bug ➹ Performance
Description
When no trace is active, loadSpan still constructs a LoadSpan which captures
process.hrtime.bigint(); the span is not ended/emitted in this branch, but the timing/object
allocation overhead still occurs. This adds unnecessary overhead to non-traced usage and contradicts
the function comment that it runs without recording when no trace is active.
Code

scopes/harmony/modules/load-trace/load-trace.ts[R161-168]

+export async function loadSpan<T>(
+  name: string,
+  attributes: SpanAttributes,
+  fn: (span: LoadSpan) => Promise<T>
+): Promise<T> {
+  const store = traceStorage.getStore();
+  if (!store) return fn(new LoadSpan(name, undefined, attributes));
+  const span = new LoadSpan(name, store.span, attributes);
Evidence
The !store branch creates a real LoadSpan, and the LoadSpan constructor captures hrtime
immediately, so overhead exists even without an active trace and the span is not ended/emitted in
that path.

scopes/harmony/modules/load-trace/load-trace.ts[161-168]
scopes/harmony/modules/load-trace/load-trace.ts[17-25]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`loadSpan()` creates a real `LoadSpan` even when there is no active trace; this captures hrtime and allocates objects but never ends/emits the span. The contract/comment says it should run without recording when untraced.
## Issue Context
This affects any caller that uses `loadSpan` outside a `startOrJoinLoadTrace` context (or when ALS context is lost), causing overhead without any benefit.
## Fix Focus Areas
- scopes/harmony/modules/load-trace/load-trace.ts[161-168]
- scopes/harmony/modules/load-trace/load-trace.ts[17-25]
### Suggested fix
In the `!store` branch, avoid constructing `LoadSpan` entirely:
- Either call `fn` with a lightweight `NoopSpan` implementation (only `setAttribute()` is needed by call sites), or
- Change the API so the callback span can be optional when untraced.
If you keep passing a span object, ensure it does not call `process.hrtime.bigint()` and does not retain children/parent state.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit 395e3de

Results up to commit N/A


🐞 Bugs (4) 📘 Rule violations (2) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0)


Action required
1. Scoped id version stripping 🐞 Bug ≡ Correctness
Description
DebugLoadCmd strips versions using id.split('@')[0], which returns an empty string for
npm-scoped ids like @scope/pkg@1.2.3, causing extension/env origin matching to collapse and
misattribute sources in bit debug-load output.
Code

scopes/workspace/workspace/debug-load.cmd.ts[R12-13]

+/** strip the version suffix from a raw id string (ids here are raw `stringId`s, not ComponentIDs). */
+const noVersion = (id: string) => id.split('@')[0];
Evidence
The current implementation explicitly uses split('@')[0] for version stripping and relies on it
for equality checks when attributing origins, which fails for npm-scoped ids that themselves start
with @.

scopes/workspace/workspace/debug-load.cmd.ts[12-13]
scopes/workspace/workspace/debug-load.cmd.ts[102-116]
scopes/workspace/workspace/debug-load.cmd.ts[123-135]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DebugLoadCmd` uses `id.split('@')[0]` to remove a version suffix. This breaks for npm-scoped package ids (e.g. `@scope/pkg@1.2.3`), where splitting on the first `@` yields an empty string, causing incorrect matching when attributing extension sources and env origin.
### Issue Context
The bug impacts `buildExtensionSources()` and `findEnvOrigin()` matching logic, and can produce wrong/missing origin attribution in `bit debug-load` output.
### Fix Focus Areas
- scopes/workspace/workspace/debug-load.cmd.ts[12-13]
- scopes/workspace/workspace/debug-load.cmd.ts[102-117]
- scopes/workspace/workspace/debug-load.cmd.ts[123-135]
### Suggested fix
Replace `id.split('@')[0]` with a helper that strips only a trailing version segment by using the **last** `@`:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Load failures misattributed 🐞 Bug ≡ Correctness
Description
WorkspaceComponentLoader.attachReportedLoadFailures() uses id.split('@')[0] when matching
failures to component/used ids; for npm-scoped ids like @scope/pkg@1.2.3 this collapses to '',
causing false matches that can attach/aggregate a failure to the wrong components.
Code

scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[R551-572]

+  private attachReportedLoadFailures(components: Component[]) {
+    if (this.workspace.inInstallContext) return;
+    const failures = currentLoadTrace()?.loadFailures;
+    if (!failures?.length) return;
+    const noVersion = (id: string) => id.split('@')[0];
+    components.forEach((component) => {
+      const componentIdStr = component.id.toString();
+      const usedIds: string[] = component.state._consumer
+        ? component.state._consumer.extensions.map((ext) => ext.stringId)
+        : [];
+      // the env may be set via the EnvsAspect config rather than as a direct extension entry, in
+      // which case its id won't appear in the extensions list above. include it explicitly so a
+      // failure loading that env still gets aggregated to this component.
+      const envId = component.state.aspects.get(EnvsAspect.id)?.data?.id;
+      if (envId) usedIds.push(envId);
+      failures.forEach((failure) => {
+        if (noVersion(failure.failedId) === noVersion(componentIdStr)) {
+          this.addLoadFailureIssue(component, failure.failedId, failure.phase, failure.error);
+          return;
+        }
+        if (usedIds.some((usedId) => noVersion(usedId) === noVersion(failure.failedId))) {
+          this.workspace.registerAggregatedLoadFailure(failure, componentIdStr);
Evidence
The matching uses the same faulty version-stripping logic as debug-load, and extension ids can be
raw strings from config (including npm-scoped packages). The codebase itself recognizes that
split('@')[0] is not valid for package ids in other areas (it branches on __type === 'package').

scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[551-573]
components/legacy/extension-data/extension-data.ts[22-34]
components/legacy/extension-data/extension-data-list.ts[194-200]
scopes/component/component-compare/component-compare.main.runtime.ts[159-166]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`attachReportedLoadFailures()` attempts to compare ids ignoring versions using `id.split('@')[0]`. For npm-scoped ids (`@scope/pkg@1.2.3`), this returns `''`, so comparisons become unreliable and can incorrectly aggregate/attach load failures.
### Issue Context
Load failures (`reportLoadFailure`) carry a `failedId` string. This string may represent an aspect/env identified by a raw string id (not necessarily a Bit `ComponentID`). `ExtensionDataEntry.stringId` can be such a raw string, so matching logic must handle npm package naming.
### Fix Focus Areas
- scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[551-573]
### Suggested fix
- Replace the local `noVersion = (id) => id.split('@')[0]` with a safe `stripVersionSuffix(id: string)` implementation (remove only the last `@...` segment, and only when it’s not the leading `@` of a scoped package).
- Example:
- `const idx = id.lastIndexOf('@');`
- `if (idx <= 0) return id;`
- `return id.slice(0, idx);`
- Use this helper consistently for `failure.failedId`, `componentIdStr`, and all `usedIds`.
- Consider adding a small unit test for `stripVersionSuffix()` with:
- `teambit.react/react@1.0.0` → `teambit.react/react`
- `@scope/pkg@1.2.3` → `@scope/pkg`
- `@scope/pkg` → `@scope/pkg`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Load failures block tagging ✓ Resolved 🐞 Bug ≡ Correctness
Description
Workspace.getWorkspaceIssues() now converts aggregated load failures into workspace issues (Error
objects), and the tag/snap flow calls BuilderMain.throwForComponentIssues(), which throws when any
workspace issue exists. As a result, previously-swallowed aspect/env failures that are aggregated at
workspace level can now abort bit tag/bit snap.
Code

scopes/workspace/workspace/workspace.ts[R1733-1740]

+    this.aggregatedLoadFailures.forEach((failure) => {
+      const affected = failure.affected.size === 1 ? '1 component' : `${failure.affected.size} components`;
+      errors.push(
+        new Error(
+          `failed loading "${failure.failedId}" (during ${failure.phase}), affects ${affected}: ${failure.error}. the load continued without it, data computed by it may be missing`
+        )
+      );
+    });
Evidence
The PR adds aggregated load failures to Workspace.getWorkspaceIssues(). Tag/snap invokes
SnappingMain.throwForVariousIssues(), which calls BuilderMain.throwForComponentIssues(), and
that method throws when workspace.getWorkspaceIssues() returns any entries—making these
newly-added aggregated load-failure entries fatal to tag/snap.

scopes/workspace/workspace/workspace.ts[1720-1741]
scopes/pipelines/builder/builder.main.runtime.ts[518-538]
scopes/component/snapping/snapping.main.runtime.ts[240-266]
scopes/component/snapping/snapping.main.runtime.ts[908-913]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Workspace.getWorkspaceIssues()` now pushes aggregated load-failure messages (from `aggregatedLoadFailures`) into the returned `Error[]`. The tag/snap pipeline (via `SnappingMain.throwForVariousIssues()` → `BuilderMain.throwForComponentIssues()`) treats any workspace issue as fatal and throws a `BitError`. This turns best-effort (swallowed) load failures into tag/snap blockers.
## Issue Context
- The new aggregated load failures are meant to reduce per-component noise in `bit status`, but the implementation uses the existing `workspace issues` mechanism, which is also used as a hard gate for tag/snap.
- The new per-component `LoadFailures` issue is explicitly non-tag-blocking, but the *workspace-level* aggregation path is currently tag-blocking because it is represented as plain `Error` objects.
## Fix Focus Areas
- scopes/workspace/workspace/workspace.ts[1720-1764]
- scopes/pipelines/builder/builder.main.runtime.ts[518-538]
- scopes/component/snapping/snapping.main.runtime.ts[240-266]
- scopes/component/snapping/snapping.main.runtime.ts[908-913]
## Suggested fix direction
Implement a non-blocking workspace-issue channel for aggregated load failures, and ensure the builder/tag/snap gate only blocks on truly blocking workspace issues (e.g. merge-config conflicts):
1. **Do not include aggregated load failures in `getWorkspaceIssues()`** (or add a parameter such as `getWorkspaceIssues({ includeNonBlocking?: boolean })`).
2. Expose aggregated load failures separately (e.g. `getWorkspaceWarnings()` / `getNonBlockingWorkspaceIssues()`), and have `bit status` render that list in the workspace-issues section (or a new section), while the builder/tag/snap gate ignores it.
3. If you must keep them in `getWorkspaceIssues()`, introduce a typed workspace-issue object with a `isTagBlocker` flag and update `BuilderMain.throwForComponentIssues()` to filter out non-blocking workspace issues.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (7)
4. debug-load env data crash ✓ Resolved 🐞 Bug ≡ Correctness
Description
DebugLoadCmd.gatherData() reads component.state.aspects.get(EnvsAspect.id)?.data.id, which will
throw when the aspect entry exists but data is undefined. This can happen because aspect entries
are upserted with handler return values that may be undefined, causing bit debug-load to fail
unexpectedly.
Code

scopes/workspace/workspace/debug-load.cmd.ts[R76-82]

+    const extensionSources = this.buildExtensionSources(mergeRes.extensions, mergeRes.beforeMerge);
+    // the merge returns an envId only when set via the envs aspect config. when the env is set as
+    // a direct extension (e.g. via variants) or not set at all, take it from the loaded
+    // component's envs data, and attribute it by searching the merge sources for it.
+    const envFromLoad = component.state.aspects.get(EnvsAspect.id)?.data.id;
+    const envId = mergeRes.envId || envFromLoad;
+    const envOrigin = this.findEnvOrigin(mergeRes.beforeMerge, envId);
Evidence
debug-load dereferences .data.id without guarding .data, and the loader code shows that aspect
entries are upserted using handler-returned data values (which may be undefined), making the crash
path feasible.

scopes/workspace/workspace/debug-load.cmd.ts[76-82]
scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[1086-1092]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`bit debug-load` can crash due to `?.data.id` (only guards the map lookup, not the `data` field). If the EnvsAspect entry exists but its data is undefined, accessing `.id` throws.
### Issue Context
Aspect entries can be updated with arbitrary handler return values, including `undefined`. The debug-load command should be resilient and never crash while trying to *explain* a load.
### Fix Focus Areas
- scopes/workspace/workspace/debug-load.cmd.ts[76-83]
- scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[1086-1092]
### Suggested fix
- Change `component.state.aspects.get(EnvsAspect.id)?.data.id` to `component.state.aspects.get(EnvsAspect.id)?.data?.id`.
- Consider additionally wrapping the env extraction in a small helper that tolerates unexpected shapes (e.g. `typeof data === 'object' && data && 'id' in data`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. debug-load hardcoded chalk formatting 📘 Rule violation ⚙ Maintainability
Description
The new DebugLoadCmd renders CLI output using direct chalk.* styling and a hardcoded Unicode
arrow () instead of relying solely on the shared CLI output formatter. This can drift from the
CLI output style guide and breaks consistency across commands.
Code

scopes/workspace/workspace/debug-load.cmd.ts[R134-172]

+  private renderStagesSection(data: DebugLoadData): string {
+    if (!data.trace) return '';
+    const lines: string[] = [];
+    const renderSpan = (span: Record<string, any>, depth: number) => {
+      const duration = span.durationMs !== undefined ? chalk.cyan(`${span.durationMs}ms`) : chalk.dim('n/a');
+      const attrs = span.attributes ? chalk.dim(` ${JSON.stringify(span.attributes)}`) : '';
+      lines.push(`   ${'  '.repeat(depth)}${span.name} ${duration}${attrs}`);
+      (span.children || []).forEach((child: Record<string, any>) => renderSpan(child, depth + 1));
+    };
+    renderSpan(data.trace, 0);
+    return [formatTitle('load stages (timings and cache hits)'), ...lines].join('\n');
+  }
+
+  private renderExtensionSourcesSection(data: DebugLoadData): string {
+    if (!data.extensionSources.length) return '';
+    const items = data.extensionSources.map((row) => {
+      const alsoIn = row.alsoIn.length ? chalk.dim(` (also in: ${row.alsoIn.join(', ')})`) : '';
+      return formatItem(`${row.extensionId} ${chalk.dim('←')} ${chalk.green(row.winner)}${alsoIn}`);
+    });
+    return [formatTitle(`extension sources (${items.length})`), ...items].join('\n');
+  }
+
+  private renderEnvSection(data: DebugLoadData): string {
+    const origin = data.envOrigin || 'computed during load (not set in config)';
+    const envLine = data.envId
+      ? formatItem(`${chalk.cyan(data.envId)} ${chalk.dim('determined by')} ${chalk.green(origin)}`)
+      : formatItem(chalk.dim('no env configured'));
+    return [formatTitle('environment'), envLine].join('\n');
+  }
+
+  private renderIssues...

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 11, 2026

Copy link
Copy Markdown

Code Review by Qodo

Grey Divider

New Review Started

This review has been superseded by a new analysis

Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 11, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (1)

Grey Divider


Action required

1. debug-load hardcoded chalk formatting 📘 Rule violation ⚙ Maintainability
Description
The new DebugLoadCmd renders CLI output using direct chalk.* styling and a hardcoded Unicode
arrow () instead of relying solely on the shared CLI output formatter. This can drift from the
CLI output style guide and breaks consistency across commands.
Code

scopes/workspace/workspace/debug-load.cmd.ts[R134-172]

+  private renderStagesSection(data: DebugLoadData): string {
+    if (!data.trace) return '';
+    const lines: string[] = [];
+    const renderSpan = (span: Record<string, any>, depth: number) => {
+      const duration = span.durationMs !== undefined ? chalk.cyan(`${span.durationMs}ms`) : chalk.dim('n/a');
+      const attrs = span.attributes ? chalk.dim(` ${JSON.stringify(span.attributes)}`) : '';
+      lines.push(`   ${'  '.repeat(depth)}${span.name} ${duration}${attrs}`);
+      (span.children || []).forEach((child: Record<string, any>) => renderSpan(child, depth + 1));
+    };
+    renderSpan(data.trace, 0);
+    return [formatTitle('load stages (timings and cache hits)'), ...lines].join('\n');
+  }
+
+  private renderExtensionSourcesSection(data: DebugLoadData): string {
+    if (!data.extensionSources.length) return '';
+    const items = data.extensionSources.map((row) => {
+      const alsoIn = row.alsoIn.length ? chalk.dim(` (also in: ${row.alsoIn.join(', ')})`) : '';
+      return formatItem(`${row.extensionId} ${chalk.dim('←')} ${chalk.green(row.winner)}${alsoIn}`);
+    });
+    return [formatTitle(`extension sources (${items.length})`), ...items].join('\n');
+  }
+
+  private renderEnvSection(data: DebugLoadData): string {
+    const origin = data.envOrigin || 'computed during load (not set in config)';
+    const envLine = data.envId
+      ? formatItem(`${chalk.cyan(data.envId)} ${chalk.dim('determined by')} ${chalk.green(origin)}`)
+      : formatItem(chalk.dim('no env configured'));
+    return [formatTitle('environment'), envLine].join('\n');
+  }
+
+  private renderIssuesSection(data: DebugLoadData): string {
+    if (!data.issues.length) {
+      return [formatTitle('load issues'), formatItem(chalk.dim('none'))].join('\n');
+    }
+    const items = data.issues.map((issue) =>
+      formatItem(`${chalk.yellow(issue.type)}: ${issue.description} ${chalk.dim(JSON.stringify(issue.data))}`)
+    );
+    return [formatTitle(`load issues (${items.length})`), ...items].join('\n');
+  }
Evidence
PR Compliance ID 1 forbids hardcoded chalk styles/symbols for CLI output in favor of the shared
output formatter. In DebugLoadCmd, the new rendering functions apply direct chalk styling and
embed a Unicode arrow () in the output strings.

CLAUDE.md: CLI Output Must Follow the CLI Output Style Guide and Use the Shared Output Formatter (No Hardcoded Chalk Styles/Symbols)
scopes/workspace/workspace/debug-load.cmd.ts[134-172]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DebugLoadCmd` uses direct `chalk` styling (e.g. `chalk.cyan`, `chalk.dim`, `chalk.green`, `chalk.yellow`) and hardcoded symbols (e.g. `←`) in its human output rendering. Per the CLI output compliance rule, section titles/symbols and styling should follow the CLI output style guide and use the shared output formatter utilities from `@teambit/cli`, avoiding ad-hoc chalk and symbols.

## Issue Context
The command already uses `formatTitle`, `formatItem`, and `joinSections` from `@teambit/cli`, but still applies manual coloring and symbols inside the content lines.

## Fix Focus Areas
- scopes/workspace/workspace/debug-load.cmd.ts[134-172]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Ungated span log cost 🐞 Bug ➹ Performance
Description
The span emitter always JSON.stringifys attributes and formats messages for every closed span before
calling the trace logger, so this CPU work happens even when trace logging is disabled. In the
component-load hot path (spans are always collected), this can add significant overhead without
producing any output.
Code

components/legacy/logger/logger.ts[R318-325]

+// emit closed load-trace spans to the log at trace level. the emitter fires after the span's
+// async context has exited, so the message carries the trace-id and span path explicitly
+// (bypassing the prefix in BitLogger methods to avoid double-prefixing).
+setSpanEmitter((span, traceId) => {
+  const attrs = Object.keys(span.attributes).length ? ` ${JSON.stringify(span.attributes)}` : '';
+  const duration = span.durationMs !== undefined ? `${span.durationMs.toFixed(2)}ms` : 'n/a';
+  logger.logger.trace(`load-trace [trace:${traceId}] ${span.path}: ${duration}${attrs}`);
+});
Evidence
The emitter formats/serializes span data unconditionally before issuing a trace log call, so the
overhead occurs even when the logger level suppresses trace output (default level is debug).

components/legacy/logger/logger.ts[31-35]
components/legacy/logger/logger.ts[318-325]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`setSpanEmitter` builds strings and `JSON.stringify`s attributes unconditionally for every closed span, even when the underlying pino logger is not configured to output `trace` level logs. This adds avoidable overhead to the load path.

## Issue Context
Span collection is always-on; span emission is intended to be trace-level only. The current code performs expensive work regardless of whether `trace` output is enabled.

## Fix Focus Areas
- components/legacy/logger/logger.ts[318-325]

### Suggested fix
Add an early return in the emitter (or only register the emitter) when `logger.logger` is not trace-enabled, e.g. check `logger.logger.level`/equivalent before computing `attrs`/`duration`/message. Ensure `JSON.stringify(span.attributes)` is only executed when the trace call will actually be emitted.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Env failures may be missed 🐞 Bug ≡ Correctness
Description
WorkspaceComponentLoader.attachReportedLoadFailures only associates a failure with a component
when failure.failedId matches one of the component's extension entry ids (or the component id). If
an env is configured via the EnvsAspect config (where env id is handled separately), a
require-aspects failure for that env id may not match and therefore may not surface as a
LoadFailures issue.
Code

scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[R548-563]

+  private attachReportedLoadFailures(components: Component[]) {
+    if (this.workspace.inInstallContext) return;
+    const failures = currentLoadTrace()?.loadFailures;
+    if (!failures?.length) return;
+    const noVersion = (id: string) => id.split('@')[0];
+    components.forEach((component) => {
+      const extensionIds: string[] = component.state._consumer
+        ? component.state._consumer.extensions.map((ext) => ext.stringId)
+        : [];
+      const relevantIds = [...extensionIds, component.id.toString()].map(noVersion);
+      failures.forEach((failure) => {
+        if (relevantIds.includes(noVersion(failure.failedId))) {
+          this.addLoadFailureIssue(component, failure.failedId, failure.phase, failure.error);
+        }
+      });
+    });
Evidence
The attachment logic only considers extension entry ids, while the workspace code explicitly models
env id as a separate concept (and the debug-load command notes that envId is returned only when set
via EnvsAspect config), so a failure for the env id can be missed if it isn't present as an
extension id string.

scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[548-563]
scopes/workspace/workspace/workspace.ts[155-160]
scopes/workspace/workspace/debug-load.cmd.ts[75-81]
scopes/workspace/workspace/debug-load.cmd.ts[125-131]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`attachReportedLoadFailures()` matches failures only against `component.state._consumer.extensions[*].stringId` plus the component id. For envs that are set via the EnvsAspect config (tracked as `envId` separately), the env id may not appear as an extension entry id, so failures reported with `failedId=<envId>` can be skipped and never attached as `IssuesClasses.LoadFailures`.

## Issue Context
The codebase itself documents that `envId` is returned separately when set via EnvsAspect config, and `findEnvOrigin()` explicitly inspects `envsAspect.config.env`.

## Fix Focus Areas
- scopes/workspace/workspace/workspace-component/workspace-component-loader.ts[548-563]
- scopes/workspace/workspace/debug-load.cmd.ts[75-81]
- scopes/workspace/workspace/debug-load.cmd.ts[125-131]
- scopes/workspace/workspace/workspace.ts[155-160]

### Suggested fix
Augment `relevantIds` with the env id when present, e.g.:
- Extract env id from `component.state._consumer.extensions.findCoreExtension(EnvsAspect.id)?.config.env` (or from `component.state.aspects.get(EnvsAspect.id)?.data.id` when available), normalize with `noVersion`, and include it in the `relevantIds` set.
This ensures failures reported as `failedId=<envId>` get attached even when the env is configured via EnvsAspect config rather than as a direct extension entry.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. No-trace spans still timed 🐞 Bug ➹ Performance
Description
When no trace is active, loadSpan still constructs a LoadSpan which captures
process.hrtime.bigint(); the span is not ended/emitted in this branch, but the timing/object
allocation overhead still occurs. This adds unnecessary overhead to non-traced usage and contradicts
the function comment that it runs without recording when no trace is active.
Code

scopes/harmony/modules/load-trace/load-trace.ts[R161-168]

+export async function loadSpan<T>(
+  name: string,
+  attributes: SpanAttributes,
+  fn: (span: LoadSpan) => Promise<T>
+): Promise<T> {
+  const store = traceStorage.getStore();
+  if (!store) return fn(new LoadSpan(name, undefined, attributes));
+  const span = new LoadSpan(name, store.span, attributes);
Evidence
The !store branch creates a real LoadSpan, and the LoadSpan constructor captures hrtime
immediately, so overhead exists even without an active trace and the span is not ended/emitted in
that path.

scopes/harmony/modules/load-trace/load-trace.ts[161-168]
scopes/harmony/modules/load-trace/load-trace.ts[17-25]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`loadSpan()` creates a real `LoadSpan` even when there is no active trace; this captures hrtime and allocates objects but never ends/emits the span. The contract/comment says it should run without recording when untraced.

## Issue Context
This affects any caller that uses `loadSpan` outside a `startOrJoinLoadTrace` context (or when ALS context is lost), causing overhead without any benefit.

## Fix Focus Areas
- scopes/harmony/modules/load-trace/load-trace.ts[161-168]
- scopes/harmony/modules/load-trace/load-trace.ts[17-25]

### Suggested fix
In the `!store` branch, avoid constructing `LoadSpan` entirely:
- Either call `fn` with a lightweight `NoopSpan` implementation (only `setAttribute()` is needed by call sites), or
- Change the API so the callback span can be optional when untraced.
If you keep passing a span object, ensure it does not call `process.hrtime.bigint()` and does not retain children/parent state.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

feat(workspace): add load tracing, load-failure issues and debug-load command
✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

Walkthroughs

Description
• Introduces @teambit/harmony.modules.load-trace: an AsyncLocalStorage-based trace context that
  assigns a unique id to every top-level component/aspect load and propagates it through nested loads
  as a hierarchical span tree.
• Adds stage-level timing spans across WorkspaceComponentLoader, ScopeComponentLoader,
  WorkspaceAspectsLoader, and the legacy ComponentLoader, recording cache hit/miss attributes per
  cache (workspace, scope, extensions, legacy).
• Adds a new LoadFailures component issue (non-tag-blocking) that surfaces previously-swallowed
  aspect/env load errors in bit status, attached at aspectLoader.handleExtensionLoadingError,
  loadCompsAsAspects, and resolveInstalledAspectRecursively.
• Adds bit debug-load  command: clears the component's cache, loads it fresh inside a trace, and
  prints a four-section report (load stages/timings, extension-merge source table, resolved env +
  origin, load issues); supports --json.
• Integrates trace prefixes into BitLogger so all log levels emit [trace: ] when a trace is
  active, replacing the ad-hoc callId in workspace-aspects-loader.ts.
• Adds e2e tests (debug-load.e2e.ts, load-failures-issue.e2e.ts) and unit tests for the
  load-trace module; no behavioral changes to loading itself.
Diagram
graph TD
  A["CLI: bit debug-load"] --> B["DebugLoadCmd"]
  B --> C["WorkspaceComponentLoader"]
  C --> D["load-trace module"]
  C --> E["WorkspaceAspectsLoader"]
  C --> F["ScopeComponentLoader"]
  C --> G["Legacy ComponentLoader"]
  E --> D
  F --> D
  G --> D
  H["AspectLoaderMain"] --> D
  D --> I["BitLogger"]
  C --> J["LoadFailures Issue"]
  H --> J
  J --> K["bit status"]

  subgraph Legend
    direction LR
    _cmd["CLI Command"] ~~~ _loader["Loader"] ~~~ _mod(["New Module"]) ~~~ _issue{{"Issue Type"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. OpenTelemetry SDK
  • ➕ Industry-standard API; tooling ecosystem (Jaeger, Zipkin) works out of the box
  • ➕ Automatic context propagation across async boundaries including some edge cases
  • ➖ Heavy dependency; requires a collector/exporter even for local use
  • ➖ Overkill for in-process, in-memory, debug-only tracing
  • ➖ Would conflict with the explicit non-goal of 'no new persistent telemetry backend'
2. Thread traceId through function signatures
  • ➕ Explicit; no hidden async context magic
  • ➕ Easier to reason about in isolation
  • ➖ Touches ~12 entry points and legacy code slated for deletion in later phases
  • ➖ Cannot flow into loadOpts (serialized into cache keys)
  • ➖ Rejected in the design doc for exactly these reasons

Recommendation: The AsyncLocalStorage approach is the right choice for this codebase. The main alternative worth considering is OpenTelemetry, but it would introduce a heavy external dependency and persistent telemetry infrastructure that the design explicitly rules out. The current approach is lightweight, zero-dependency, and perfectly scoped to the observability goals of Phase 1.

Grey Divider

File Changes

Enhancement (8)
issues-list.ts Register LoadFailures in the global IssuesClasses registry +2/-0

Register LoadFailures in the global IssuesClasses registry

• Imports and adds 'LoadFailures' to the 'IssuesClasses' map so the existing component-issues mechanism picks it up automatically.

components/component-issues/issues-list.ts


workspace-component-loader.ts Instrument workspace component loader with trace spans and load-failure issue attachment +168/-40

Instrument workspace component loader with trace spans and load-failure issue attachment

• Wraps 'get' and 'getMany' entry points in 'startOrJoinLoadTrace'; adds 'loadSpan' calls around id-resolution, build-load-groups, load-group, consumer-fs-load, scope-load, extension-merge, load-components-extensions, load-comps-as-aspects, populate-scope-and-extensions-cache, execute-load-slot, env-calc, dependency-resolution, env-manifest, and each on-load handler. Adds 'attachReportedLoadFailures' to pull trace-collected failures onto affected components, and 'addLoadFailureIssue' for direct attachment at catch sites.

scopes/workspace/workspace/workspace-component/workspace-component-loader.ts


workspace-aspects-loader.ts Replace ad-hoc callId with load-trace and report swallowed aspect failures +18/-6

Replace ad-hoc callId with load-trace and report swallowed aspect failures

• Wraps 'loadAspects' in 'startOrJoinLoadTrace', removing the ad-hoc 'callId'/'profileTrace' pattern. Adds 'reportLoadFailure' at the 'resolveInstalledAspectRecursively' catch site so swallowed resolution errors surface as component issues.

scopes/workspace/workspace/workspace-aspects-loader.ts


scope-component-loader.ts Instrument scope component loader with trace spans and cache hit/miss attributes +19/-2

Instrument scope component loader with trace spans and cache hit/miss attributes

• Wraps 'get' in 'startOrJoinLoadTrace' and adds 'loadSpan' around 'scope-import' and 'state-from-version' stages; records 'componentsCache' hit/miss attribute on the span.

scopes/scope/scope/scope-component-loader.ts


scope.main.runtime.ts Wrap ScopeMain.getMany in a load trace +8/-4

Wrap ScopeMain.getMany in a load trace

• Adds an early-exit for empty id lists and wraps the 'mapSeries' body in 'startOrJoinLoadTrace('scope.getMany', ...)' so scope-level bulk loads participate in the trace hierarchy.

scopes/scope/scope/scope.main.runtime.ts


component-loader.ts Add cache hit/miss span attributes and wrap legacy dep loading in a span +15/-7

Add cache hit/miss span attributes and wrap legacy dep loading in a span

• Records 'legacyComponentsCache:hits/misses' on the active span when returning cached components, and wraps 'ComponentLoader.loadDeps' in a 'legacy-load-deps' span.

components/legacy/consumer-component/component-loader.ts


logger.ts Prefix all BitLogger log calls with the active load-trace path +16/-6

Prefix all BitLogger log calls with the active load-trace path

• Imports 'getLoadTraceLogPrefix' and prepends it to every log level (trace/debug/warn/info/error/fatal). Registers a 'setSpanEmitter' callback that emits closed spans at 'trace' level via the raw pino logger to avoid double-prefixing.

components/legacy/logger/logger.ts


workspace.main.runtime.ts Register DebugLoadCmd in the workspace command list +2/-0

Register DebugLoadCmd in the workspace command list

• Imports and instantiates 'DebugLoadCmd' with the workspace instance, adding it to the commands array alongside other workspace debug commands.

scopes/workspace/workspace/workspace.main.runtime.ts


Bug fix (1)
aspect-loader.main.runtime.ts Report swallowed extension load errors to the active load trace +4/-0

Report swallowed extension load errors to the active load trace

• Adds a 'reportLoadFailure' call in 'handleExtensionLoadingError' after the best-effort swallow path, so require-time aspect failures are recorded on the trace and later attached as 'LoadFailures' issues.

scopes/harmony/aspect-loader/aspect-loader.main.runtime.ts


Tests (3)
load-trace.spec.ts Unit tests for load-trace: nesting, isolation, prefixes, error propagation +77/-0

Unit tests for load-trace: nesting, isolation, prefixes, error propagation

• Seven unit tests covering trace start/join semantics, distinct ids for independent loads, nested span tree with durations, log prefix format, no-op when no trace is active, and error propagation with span end guarantee.

scopes/harmony/modules/load-trace/load-trace.spec.ts


debug-load.e2e.ts E2E tests for bit debug-load: human output, JSON output, unknown id error +65/-0

E2E tests for bit debug-load: human output, JSON output, unknown id error

• Three describe blocks: verifies all four sections appear in human output with correct env attribution and cache info; validates JSON output structure with trace tree and extension sources; confirms a clear error (no stack) for an unknown component id.

e2e/harmony/debug-load.e2e.ts


load-failures-issue.e2e.ts E2E test: non-requireable aspect surfaces as LoadFailures issue in bit status +48/-0

E2E test: non-requireable aspect surfaces as LoadFailures issue in bit status

• Sets up a workspace with a non-requireable aspect configured on comp1, runs 'bit status', and asserts the component has a 'LoadFailures' issue naming the failing aspect and error. Also verifies 'bit tag' proceeds without '--ignore-issues'.

e2e/harmony/load-failures-issue.e2e.ts


Documentation (7)
component-loading-redesign.md Multi-phase component-loading redesign master document +279/-0

Multi-phase component-loading redesign master document

• Comprehensive design document covering the six-phase redesign plan: problem statement (all-or-nothing loading, 11 uncoordinated caches, legacy roundtrip spine, mutual recursion), target architecture (staged loading S0-S4, single cache manager, legacy inversion, env planner), phase plan, benchmark table, and status log.

scopes/workspace/workspace/component-loading-redesign.md


design.md OpenSpec design document for Phase 1 observability +71/-0

OpenSpec design document for Phase 1 observability

• Records the four key decisions (AsyncLocalStorage vs parameter threading, always-on in-memory spans, debug-load command placement, non-blocking LoadFailure issue type) with rationale, risks, and migration plan.

openspec/changes/component-load-observability/design.md


proposal.md OpenSpec proposal: why and what changes in Phase 1 +34/-0

OpenSpec proposal: why and what changes in Phase 1

• Describes the motivation (9 silent swallow sites, interleaved logs, no env-attribution tool) and lists the four new capabilities introduced in this phase.

openspec/changes/component-load-observability/proposal.md


spec.md Spec: swallowed load errors become non-blocking component issues +47/-0

Spec: swallowed load errors become non-blocking component issues

• Formal requirements for the LoadFailures issue: attachment at catch sites, visibility in bit status, non-tag-blocking severity, deduplication, and install-context suppression.

openspec/changes/component-load-observability/specs/component-load-issues/spec.md


spec.md Spec: trace context per top-level load with hierarchical log correlation +47/-0

Spec: trace context per top-level load with hierarchical log correlation

• Formal requirements for the load-trace module: unique id per top-level load, nested loads join as child spans, log prefix format, stage-level timing, no behavioral impact.

openspec/changes/component-load-observability/specs/component-load-tracing/spec.md


spec.md Spec: bit debug-load command requirements and scenarios +51/-0

Spec: bit debug-load command requirements and scenarios

• Formal requirements for the debug-load command: four-section report, clear error for unknown id, stage timings, extension-merge source table, env resolution explanation, and JSON flag.

openspec/changes/component-load-observability/specs/debug-load-command/spec.md


tasks.md Task checklist for Phase 1 implementation +30/-0

Task checklist for Phase 1 implementation

• Tracks completion of all Phase 1 tasks across three sub-PRs: trace context + spans, load-failure issues, and debug-load command.

openspec/changes/component-load-observability/tasks.md


Other (6)
load-trace.ts New AsyncLocalStorage-based load trace module with spans, failures, and log prefix +198/-0

New AsyncLocalStorage-based load trace module with spans, failures, and log prefix

• Implements the core 'LoadTrace' and 'LoadSpan' classes backed by 'AsyncLocalStorage'. Exposes 'startOrJoinLoadTrace', 'loadSpan', 'loadSpanSync', 'reportLoadFailure', 'currentLoadTrace', 'currentLoadSpan', and 'getLoadTraceLogPrefix'. Spans record start/duration/attributes and nest into a tree; failures are deduplicated by (failedId, phase).

scopes/harmony/modules/load-trace/load-trace.ts


index.ts Public API barrel export for load-trace module +13/-0

Public API barrel export for load-trace module

• Re-exports all public types and functions from 'load-trace.ts' as the module's public API.

scopes/harmony/modules/load-trace/index.ts


load-failures.ts New LoadFailures component issue type (non-tag-blocking) +30/-0

New LoadFailures component issue type (non-tag-blocking)

• Defines 'LoadFailures' extending 'ComponentIssue' with 'isTagBlocker = false' and 'isCacheBlocker = false'. Each entry carries 'failedId', 'phase', and 'error'; 'dataToString' formats them for 'bit status' output.

components/component-issues/load-failures.ts


debug-load.cmd.ts New bit debug-load command: fresh load with trace, extension sources, env origin, issues +173/-0

New bit debug-load command: fresh load with trace, extension sources, env origin, issues

• Implements 'DebugLoadCmd' registered in the workspace aspect. Clears the target component's cache, loads it inside a fresh 'startOrJoinLoadTrace', then renders four sections: load stages tree with timings, extension-merge source table (winner + also-in), resolved env + origin, and load issues. Supports '--json' for machine-readable output.

scopes/workspace/workspace/debug-load.cmd.ts


.bitmap Register new modules/load-trace component in .bitmap +8/-0

Register new modules/load-trace component in .bitmap

• Adds the 'modules/load-trace' component entry with 'defaultScope: teambit.harmony' and 'rootDir: scopes/harmony/modules/load-trace'.

.bitmap


.openspec.yaml OpenSpec metadata file for component-load-observability change +2/-0

OpenSpec metadata file for component-load-observability change

• Declares the spec schema type and creation date for the component-load-observability OpenSpec change.

openspec/changes/component-load-observability/.openspec.yaml


Grey Divider

Qodo Logo

Comment thread scopes/workspace/workspace/debug-load.cmd.ts
Comment thread components/legacy/logger/logger.ts
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 11, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit d07de94

Comment thread scopes/workspace/workspace/debug-load.cmd.ts
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 11, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit bd53d5b

Comment thread scopes/workspace/workspace/workspace.ts Outdated
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 11, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit f75d97c

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 11, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit c85b920

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 11, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit e493ad9

…ient batch ones

the loadCompsAsAspects batch catch could report a load-failure issue for an
aspect that loads successfully on a later pass (clearComponentCache resets the
self-load gate and the load retries, e.g. after on-the-fly compilation), and
could not attribute a batch throw to a single id. drop its reporting; genuine
terminal failures still surface via the aspect-loader require-aspects path
(never silently re-attempted) and installed-aspect resolution.
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code Review by Qodo

Grey Divider

New Review Started

This review has been superseded by a new analysis

Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit e8882fa

Comment on lines +12 to +13
/** strip the version suffix from a raw id string (ids here are raw `stringId`s, not ComponentIDs). */
const noVersion = (id: string) => id.split('@')[0];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Scoped id version stripping 🐞 Bug ≡ Correctness

DebugLoadCmd strips versions using id.split('@')[0], which returns an empty string for
npm-scoped ids like @scope/pkg@1.2.3, causing extension/env origin matching to collapse and
misattribute sources in bit debug-load output.
Agent Prompt
### Issue description
`DebugLoadCmd` uses `id.split('@')[0]` to remove a version suffix. This breaks for npm-scoped package ids (e.g. `@scope/pkg@1.2.3`), where splitting on the first `@` yields an empty string, causing incorrect matching when attributing extension sources and env origin.

### Issue Context
The bug impacts `buildExtensionSources()` and `findEnvOrigin()` matching logic, and can produce wrong/missing origin attribution in `bit debug-load` output.

### Fix Focus Areas
- scopes/workspace/workspace/debug-load.cmd.ts[12-13]
- scopes/workspace/workspace/debug-load.cmd.ts[102-117]
- scopes/workspace/workspace/debug-load.cmd.ts[123-135]

### Suggested fix
Replace `id.split('@')[0]` with a helper that strips only a trailing version segment by using the **last** `@`:

```ts
const stripVersion = (id: string) => {
  const at = id.lastIndexOf('@');
  // at===0 covers "@scope/pkg" (no version)
  if (at <= 0) return id;
  return id.slice(0, at);
};
```

Then use `stripVersion()` in both `buildExtensionSources` and `findEnvOrigin` comparisons.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

reverts the over-correction in a572c2f. removing the loadCompsAsAspects
reporting silenced a genuinely-terminal failure: an aspect that throws on
require (never recovers) is surfaced via that path, not the require-aspects
path, so dropping it made bit status silent about a real error that bit show
already shows - an inconsistency. the transient-recovery concern is largely
self-correcting (a successful later load replaces the cached component object,
clearing the stale issue), whereas under-reporting hid real, terminal errors.
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 395e3de

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.

2 participants