fix(arkor): keeping original error when training entry import fails by ARYAN0-work · Pull Request #222 · arkorlab/arkor

packages/arkor/src/core/runner.ts[58-68]
eslint.config.ts[145-151]
packages/arkor/package.json[44-49]

+  } catch(error){
+    throw new Error(`Failed to load training entry: ${abs}`,{
+      cause:error,
+    })

Evidence

The modified runTrainer code uses try{ / catch(error){ and an options object literal without
spaces, which conflicts with the repo’s ESLint configuration enabling @stylistic/keyword-spacing
and @stylistic/object-curly-spacing as errors. The arkor package lint script runs eslint .,
so these violations will surface as failing lint.

Agent prompt

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

## Issue description
`packages/arkor/src/core/runner.ts` introduces spacing that violates repo ESLint rules:
- `try{` / `} catch(error){` violates `@stylistic/keyword-spacing` (expects `try {` and `catch (error)`).
- `new Error(...,{ cause:error, })` violates `@stylistic/object-curly-spacing` (expects spaces inside `{ ... }`).
These rules are configured as `error`, and the `arkor` package lint script runs `eslint .`, so the PR likely fails lint.
## Issue Context
Repo ESLint config enables these stylistic rules at error level.
## Fix Focus Areas
- packages/arkor/src/core/runner.ts[58-69]
- eslint.config.ts[145-151]
- packages/arkor/package.json[44-49]

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

2. Error coercion masks failures 🐞 Bug ☼ Reliability ⭐ New

Description

When an imported entry throws a non-Error value whose string coercion fails, String(error) throws
before the contextual Error is created. For example, throw Object.create(null) replaces the entry
failure with a coercion TypeError and loses both the entry-path context and original cause.

Code

packages/arkor/src/core/runner.ts[63]
+    const causeMessage = error instanceof Error ? error.message : String(error);

Evidence

runTrainer dynamically executes the user-provided entry, catches its arbitrary thrown value, and
then coercively formats that unknown value before constructing the contextual Error. Because
coercion can itself throw, the subsequent Error with cause: error is never reached in that case.

packages/arkor/src/core/runner.ts[47-65]

Agent prompt

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

## Issue description
A training entry can throw any JavaScript value. If a non-Error value cannot be converted to a string, the current `String(error)` call throws before the contextual error is constructed, masking the original failure and omitting the entry path.

## Issue Context
Keep the original thrown value as `cause`, but make formatting best-effort so creation of the contextual error cannot be interrupted. Add coverage using a module that throws a value such as `Object.create(null)`.

## Fix Focus Areas
- packages/arkor/src/core/runner.ts[60-66]
- packages/arkor/src/core/runner.test.ts[62-76]

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

3. ~~Cause hidden in CLI~~ ✓ Resolved 🐞 Bug ◔ Observability

Description

runTrainer now rethrows import failures as a new Error with the original failure stored only in
cause, but the CLI entrypoint logs only err.stack ?? err.message, which does not print the cause
chain. As a result, arkor start failures will typically show only “Failed to load training entry
…” and omit the underlying import error details needed to debug (e.g. missing module / syntax
error).

Code

packages/arkor/src/core/runner.ts[47-66]
packages/arkor/src/bin.ts[14-42]

+  } catch (error) {
+    throw new Error(`Failed to load training entry: ${abs}`, {
+      cause: error,
+    });

Evidence

The PR change wraps the dynamic import error into a new Error whose only link to the original
failure is via cause. The CLI’s top-level handler prints only err.stack ?? err.message and never
prints cause, so the underlying import failure details are not emitted by the CLI path.

Agent prompt

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

## Issue description
`runTrainer()` now wraps import errors with a new `Error(..., { cause })`, but the CLI’s top-level handler prints only `err.stack ?? err.message`. Since the underlying import failure is now in `err.cause`, users typically won’t see the real import error message/stack when `arkor start` fails.
## Issue Context
The PR intentionally moves the original import failure into `Error.cause`. The CLI should print the entire cause chain (at least message + stack) so the wrapped error adds context *without* hiding the root cause.
## Fix Focus Areas
- packages/arkor/src/bin.ts[14-43]
- packages/arkor/src/core/runner.ts[58-66]
## Suggested fix
Update the CLI error printing to include `err.cause` recursively (e.g., print `err.stack` then append `Caused by: <cause.stack>` while `cause` exists). This keeps the wrapper context while restoring the original import failure details in CLI output.

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

4. ~~Test omits entry path assert~~ ✓ Resolved 📎 Requirement gap ☼ Reliability

Description

The new regression test checks for the contextual error prefix but does not assert that the thrown
error message includes the failing training entry path. This weakens the guarantee required by the
compliance checklist and could allow future changes to drop the entry-path context without failing
tests.

Code

Add automated test covering failed training entry import
packages/arkor/src/core/runner.test.ts[62-76]

+        expect((error as Error).message).toMatch(/Failed to load training entry/);
+        expect((error as Error).cause).toBeInstanceOf(Error);
+        expect(((error as Error).cause as Error).message).toMatch(
+          /Cannot find module/,

Evidence

PR Compliance ID 2 requires an automated test that verifies both contextual messaging including the
entry path and original error preservation. In the added test, the assertions check only for the
generic message prefix and the presence/content of cause, but never assert that the message
contains the entry path.

Agent prompt

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

## Issue description
The new test validates `Failed to load training entry` but does not verify the error message includes the specific training entry path, which is required to prevent regressions.
## Issue Context
`runTrainer()` is expected to throw an error that both (1) provides context and identifies the entry path and (2) preserves the underlying import error as `cause`. The implementation in `runner.ts` includes the absolute path (`${abs}`), but the test should explicitly assert the path is present.
## Fix Focus Areas
- packages/arkor/src/core/runner.test.ts[62-77]

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

Context sources

✅ Tickets: 🎫 Improve error context when training entry import fails

Tip of the day

💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 51b1efe ⚖️ Balanced

Results up to commit 4112052

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. ~~ESLint spacing rule break~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new try/catch and Error options object in runTrainer omit required keyword/object spacing,
which will trigger @stylistic/keyword-spacing and @stylistic/object-curly-spacing lint errors.
This is likely to fail the arkor package lint script (oxlint ... && eslint .) and block CI/merge
checks.

Code

packages/arkor/src/core/runner.ts[58-68]
eslint.config.ts[145-151]
packages/arkor/package.json[44-49]

+  } catch(error){
+    throw new Error(`Failed to load training entry: ${abs}`,{
+      cause:error,
+    })

Evidence

The modified runTrainer code uses try{ / catch(error){ and an options object literal without
spaces, which conflicts with the repo’s ESLint configuration enabling @stylistic/keyword-spacing
and @stylistic/object-curly-spacing as errors. The arkor package lint script runs eslint .,
so these violations will surface as failing lint.

Agent prompt

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

## Issue description
`packages/arkor/src/core/runner.ts` introduces spacing that violates repo ESLint rules:
- `try{` / `} catch(error){` violates `@stylistic/keyword-spacing` (expects `try {` and `catch (error)`).
- `new Error(...,{ cause:error, })` violates `@stylistic/object-curly-spacing` (expects spaces inside `{ ... }`).
These rules are configured as `error`, and the `arkor` package lint script runs `eslint .`, so the PR likely fails lint.
## Issue Context
Repo ESLint config enables these stylistic rules at error level.
## Fix Focus Areas
- packages/arkor/src/core/runner.ts[58-69]
- eslint.config.ts[145-151]
- packages/arkor/package.json[44-49]

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

2. ~~Cause hidden in CLI~~ ✓ Resolved 🐞 Bug ◔ Observability

Description

runTrainer now rethrows import failures as a new Error with the original failure stored only in
cause, but the CLI entrypoint logs only err.stack ?? err.message, which does not print the cause
chain. As a result, arkor start failures will typically show only “Failed to load training entry
…” and omit the underlying import error details needed to debug (e.g. missing module / syntax
error).

Code

packages/arkor/src/core/runner.ts[47-66]
packages/arkor/src/bin.ts[14-42]

+  } catch (error) {
+    throw new Error(`Failed to load training entry: ${abs}`, {
+      cause: error,
+    });

Evidence

The PR change wraps the dynamic import error into a new Error whose only link to the original
failure is via cause. The CLI’s top-level handler prints only err.stack ?? err.message and never
prints cause, so the underlying import failure details are not emitted by the CLI path.

Agent prompt

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

## Issue description
`runTrainer()` now wraps import errors with a new `Error(..., { cause })`, but the CLI’s top-level handler prints only `err.stack ?? err.message`. Since the underlying import failure is now in `err.cause`, users typically won’t see the real import error message/stack when `arkor start` fails.
## Issue Context
The PR intentionally moves the original import failure into `Error.cause`. The CLI should print the entire cause chain (at least message + stack) so the wrapped error adds context *without* hiding the root cause.
## Fix Focus Areas
- packages/arkor/src/bin.ts[14-43]
- packages/arkor/src/core/runner.ts[58-66]
## Suggested fix
Update the CLI error printing to include `err.cause` recursively (e.g., print `err.stack` then append `Caused by: <cause.stack>` while `cause` exists). This keeps the wrapper context while restoring the original import failure details in CLI output.

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

3. ~~Test omits entry path assert~~ ✓ Resolved 📎 Requirement gap ☼ Reliability

Description

The new regression test checks for the contextual error prefix but does not assert that the thrown
error message includes the failing training entry path. This weakens the guarantee required by the
compliance checklist and could allow future changes to drop the entry-path context without failing
tests.

Code

Add automated test covering failed training entry import
packages/arkor/src/core/runner.test.ts[62-76]

+        expect((error as Error).message).toMatch(/Failed to load training entry/);
+        expect((error as Error).cause).toBeInstanceOf(Error);
+        expect(((error as Error).cause as Error).message).toMatch(
+          /Cannot find module/,

Evidence

PR Compliance ID 2 requires an automated test that verifies both contextual messaging including the
entry path and original error preservation. In the added test, the assertions check only for the
generic message prefix and the presence/content of cause, but never assert that the message
contains the entry path.

Agent prompt

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

## Issue description
The new test validates `Failed to load training entry` but does not verify the error message includes the specific training entry path, which is required to prevent regressions.
## Issue Context
`runTrainer()` is expected to throw an error that both (1) provides context and identifies the entry path and (2) preserves the underlying import error as `cause`. The implementation in `runner.ts` includes the absolute path (`${abs}`), but the test should explicitly assert the path is present.
## Fix Focus Areas
- packages/arkor/src/core/runner.test.ts[62-77]

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

Context sources

✅ Tickets: 🎫 Improve error context when training entry import fails

Results up to commit 6e492b1

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. ~~ESLint spacing rule break~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new try/catch and Error options object in runTrainer omit required keyword/object spacing,
which will trigger @stylistic/keyword-spacing and @stylistic/object-curly-spacing lint errors.
This is likely to fail the arkor package lint script (oxlint ... && eslint .) and block CI/merge
checks.

Code

packages/arkor/src/core/runner.ts[58-68]
eslint.config.ts[145-151]
packages/arkor/package.json[44-49]

+  } catch(error){
+    throw new Error(`Failed to load training entry: ${abs}`,{
+      cause:error,
+    })

Evidence

The modified runTrainer code uses try{ / catch(error){ and an options object literal without
spaces, which conflicts with the repo’s ESLint configuration enabling @stylistic/keyword-spacing
and @stylistic/object-curly-spacing as errors. The arkor package lint script runs eslint .,
so these violations will surface as failing lint.

Agent prompt

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

## Issue description
`packages/arkor/src/core/runner.ts` introduces spacing that violates repo ESLint rules:
- `try{` / `} catch(error){` violates `@stylistic/keyword-spacing` (expects `try {` and `catch (error)`).
- `new Error(...,{ cause:error, })` violates `@stylistic/object-curly-spacing` (expects spaces inside `{ ... }`).
These rules are configured as `error`, and the `arkor` package lint script runs `eslint .`, so the PR likely fails lint.
## Issue Context
Repo ESLint config enables these stylistic rules at error level.
## Fix Focus Areas
- packages/arkor/src/core/runner.ts[58-69]
- eslint.config.ts[145-151]
- packages/arkor/package.json[44-49]

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

2. ~~Cause hidden in CLI~~ ✓ Resolved 🐞 Bug ◔ Observability

Description

runTrainer now rethrows import failures as a new Error with the original failure stored only in
cause, but the CLI entrypoint logs only err.stack ?? err.message, which does not print the cause
chain. As a result, arkor start failures will typically show only “Failed to load training entry
…” and omit the underlying import error details needed to debug (e.g. missing module / syntax
error).

Code

packages/arkor/src/core/runner.ts[47-66]
packages/arkor/src/bin.ts[14-42]

+  } catch (error) {
+    throw new Error(`Failed to load training entry: ${abs}`, {
+      cause: error,
+    });

Evidence

The PR change wraps the dynamic import error into a new Error whose only link to the original
failure is via cause. The CLI’s top-level handler prints only err.stack ?? err.message and never
prints cause, so the underlying import failure details are not emitted by the CLI path.

Agent prompt

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

## Issue description
`runTrainer()` now wraps import errors with a new `Error(..., { cause })`, but the CLI’s top-level handler prints only `err.stack ?? err.message`. Since the underlying import failure is now in `err.cause`, users typically won’t see the real import error message/stack when `arkor start` fails.
## Issue Context
The PR intentionally moves the original import failure into `Error.cause`. The CLI should print the entire cause chain (at least message + stack) so the wrapped error adds context *without* hiding the root cause.
## Fix Focus Areas
- packages/arkor/src/bin.ts[14-43]
- packages/arkor/src/core/runner.ts[58-66]
## Suggested fix
Update the CLI error printing to include `err.cause` recursively (e.g., print `err.stack` then append `Caused by: <cause.stack>` while `cause` exists). This keeps the wrapper context while restoring the original import failure details in CLI output.

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

3. ~~Test omits entry path assert~~ ✓ Resolved 📎 Requirement gap ☼ Reliability

Description

The new regression test checks for the contextual error prefix but does not assert that the thrown
error message includes the failing training entry path. This weakens the guarantee required by the
compliance checklist and could allow future changes to drop the entry-path context without failing
tests.

Code

Add automated test covering failed training entry import
packages/arkor/src/core/runner.test.ts[62-76]

+        expect((error as Error).message).toMatch(/Failed to load training entry/);
+        expect((error as Error).cause).toBeInstanceOf(Error);
+        expect(((error as Error).cause as Error).message).toMatch(
+          /Cannot find module/,

Evidence

PR Compliance ID 2 requires an automated test that verifies both contextual messaging including the
entry path and original error preservation. In the added test, the assertions check only for the
generic message prefix and the presence/content of cause, but never assert that the message
contains the entry path.

Agent prompt

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

## Issue description
The new test validates `Failed to load training entry` but does not verify the error message includes the specific training entry path, which is required to prevent regressions.
## Issue Context
`runTrainer()` is expected to throw an error that both (1) provides context and identifies the entry path and (2) preserves the underlying import error as `cause`. The implementation in `runner.ts` includes the absolute path (`${abs}`), but the test should explicitly assert the path is present.
## Fix Focus Areas
- packages/arkor/src/core/runner.test.ts[62-77]

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

Context

✅ Tickets: 🎫 Improve error context when training entry import fails

Results up to commit f372dba

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. ~~ESLint spacing rule break~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The new try/catch and Error options object in runTrainer omit required keyword/object spacing,
which will trigger @stylistic/keyword-spacing and @stylistic/object-curly-spacing lint errors.
This is likely to fail the arkor package lint script (oxlint ... && eslint .) and block CI/merge
checks.

Code

packages/arkor/src/core/runner.ts[58-68]
eslint.config.ts[145-151]
packages/arkor/package.json[44-49]

+  } catch(error){
+    throw new Error(`Failed to load training entry: ${abs}`,{
+      cause:error,
+    })

Evidence

The modified runTrainer code uses try{ / catch(error){ and an options object literal without
spaces, which conflicts with the repo’s ESLint configuration enabling @stylistic/keyword-spacing
and @stylistic/object-curly-spacing as errors. The arkor package lint script runs eslint .,
so these violations will surface as failing lint.

Agent prompt

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

## Issue description
`packages/arkor/src/core/runner.ts` introduces spacing that violates repo ESLint rules:
- `try{` / `} catch(error){` violates `@stylistic/keyword-spacing` (expects `try {` and `catch (error)`).
- `new Error(...,{ cause:error, })` violates `@stylistic/object-curly-spacing` (expects spaces inside `{ ... }`).
These rules are configured as `error`, and the `arkor` package lint script runs `eslint .`, so the PR likely fails lint.
## Issue Context
Repo ESLint config enables these stylistic rules at error level.
## Fix Focus Areas
- packages/arkor/src/core/runner.ts[58-69]
- eslint.config.ts[145-151]
- packages/arkor/package.json[44-49]

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

2. Cause hidden in CLI 🐞 Bug ◔ Observability ⭐ New

Description

runTrainer now rethrows import failures as a new Error with the original failure stored only in
cause, but the CLI entrypoint logs only err.stack ?? err.message, which does not print the cause
chain. As a result, arkor start failures will typically show only “Failed to load training entry
…” and omit the underlying import error details needed to debug (e.g. missing module / syntax
error).

Code

packages/arkor/src/core/runner.ts[47-66]
packages/arkor/src/bin.ts[14-42]

+  } catch (error) {
+    throw new Error(`Failed to load training entry: ${abs}`, {
+      cause: error,
+    });

Evidence

The PR change wraps the dynamic import error into a new Error whose only link to the original
failure is via cause. The CLI’s top-level handler prints only err.stack ?? err.message and never
prints cause, so the underlying import failure details are not emitted by the CLI path.

Agent prompt

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

## Issue description
`runTrainer()` now wraps import errors with a new `Error(..., { cause })`, but the CLI’s top-level handler prints only `err.stack ?? err.message`. Since the underlying import failure is now in `err.cause`, users typically won’t see the real import error message/stack when `arkor start` fails.

## Issue Context
The PR intentionally moves the original import failure into `Error.cause`. The CLI should print the entire cause chain (at least message + stack) so the wrapped error adds context *without* hiding the root cause.

## Fix Focus Areas
- packages/arkor/src/bin.ts[14-43]
- packages/arkor/src/core/runner.ts[58-66]

## Suggested fix
Update the CLI error printing to include `err.cause` recursively (e.g., print `err.stack` then append `Caused by: <cause.stack>` while `cause` exists). This keeps the wrapper context while restoring the original import failure details in CLI output.

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

3. ~~Test omits entry path assert~~ ✓ Resolved 📎 Requirement gap ☼ Reliability

Description

The new regression test checks for the contextual error prefix but does not assert that the thrown
error message includes the failing training entry path. This weakens the guarantee required by the
compliance checklist and could allow future changes to drop the entry-path context without failing
tests.

Code

Add automated test covering failed training entry import
packages/arkor/src/core/runner.test.ts[62-76]

+        expect((error as Error).message).toMatch(/Failed to load training entry/);
+        expect((error as Error).cause).toBeInstanceOf(Error);
+        expect(((error as Error).cause as Error).message).toMatch(
+          /Cannot find module/,

Evidence

PR Compliance ID 2 requires an automated test that verifies both contextual messaging including the
entry path and original error preservation. In the added test, the assertions check only for the
generic message prefix and the presence/content of cause, but never assert that the message
contains the entry path.

Agent prompt

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

## Issue description
The new test validates `Failed to load training entry` but does not verify the error message includes the specific training entry path, which is required to prevent regressions.
## Issue Context
`runTrainer()` is expected to throw an error that both (1) provides context and identifies the entry path and (2) preserves the underlying import error as `cause`. The implementation in `runner.ts` includes the absolute path (`${abs}`), but the test should explicitly assert the path is present.
## Fix Focus Areas
- packages/arkor/src/core/runner.test.ts[62-77]

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

Context used

✅ Tickets: 🎫 Improve error context when training entry import fails

Results up to commit d4b547a

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (1) 📜 Skill insights (0)

1. ESLint spacing rule break 🐞 Bug ⚙ Maintainability

Description

The new try/catch and Error options object in runTrainer omit required keyword/object spacing,
which will trigger @stylistic/keyword-spacing and @stylistic/object-curly-spacing lint errors.
This is likely to fail the arkor package lint script (oxlint ... && eslint .) and block CI/merge
checks.

Code

packages/arkor/src/core/runner.ts[58-68]
eslint.config.ts[145-151]
packages/arkor/package.json[44-49]

+  } catch(error){
+    throw new Error(`Failed to load training entry: ${abs}`,{
+      cause:error,
+    })

Evidence

The modified runTrainer code uses try{ / catch(error){ and an options object literal without
spaces, which conflicts with the repo’s ESLint configuration enabling @stylistic/keyword-spacing
and @stylistic/object-curly-spacing as errors. The arkor package lint script runs eslint .,
so these violations will surface as failing lint.

Agent prompt

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

## Issue description
`packages/arkor/src/core/runner.ts` introduces spacing that violates repo ESLint rules:
- `try{` / `} catch(error){` violates `@stylistic/keyword-spacing` (expects `try {` and `catch (error)`).
- `new Error(...,{ cause:error, })` violates `@stylistic/object-curly-spacing` (expects spaces inside `{ ... }`).

These rules are configured as `error`, and the `arkor` package lint script runs `eslint .`, so the PR likely fails lint.

## Issue Context
Repo ESLint config enables these stylistic rules at error level.

## Fix Focus Areas
- packages/arkor/src/core/runner.ts[58-69]
- eslint.config.ts[145-151]
- packages/arkor/package.json[44-49]

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

2. Test omits entry path assert 📎 Requirement gap ☼ Reliability

Description

The new regression test checks for the contextual error prefix but does not assert that the thrown
error message includes the failing training entry path. This weakens the guarantee required by the
compliance checklist and could allow future changes to drop the entry-path context without failing
tests.

Code