j4k-oss/review-wrapper
Watch
2
0
Fork
You've already forked review-wrapper
 0
Code Issues 12 Pull requests Releases Packages Activity Actions

feat: tell users to wait out a full sandbox fleet #48

Merged
jercik merged 2 commits from feat/sandbox-capacity-remedy into main 2026-10-05 11:01:43 +00:00
Conversation 6 Commits 2 Files changed 4 +40 -19
jercik commented 2026-10-05 07:04:11 +00:00
Owner
Copy link

Adds the recovery advice for review's new sandbox-capacity-exhausted failure (j4k/review#216): wait for the review burst to drain, and ask the operator to check axsandbox capacity and leaked sandboxes if the failure repeats.

A code the bundled @j4k/review can't name now keeps its (code "…") even when the wrapper has a remedy for it. Until the bump, the log reads execution failed for an unrecognized reason (code "sandbox-capacity-exhausted") — wait for the review burst to drain; …. The README already sends readers to the remedy printed on that line, so it gains no new row.

Merge after j4k/review#216.

Adds the recovery advice for review's new `sandbox-capacity-exhausted` failure (j4k/review#216): wait for the review burst to drain, and ask the operator to check axsandbox capacity and leaked sandboxes if the failure repeats. A code the bundled `@j4k/review` can't name now keeps its `(code "…")` even when the wrapper has a remedy for it. Until the bump, the log reads `execution failed for an unrecognized reason (code "sandbox-capacity-exhausted") — wait for the review burst to drain; …`. The README already sends readers to the remedy printed on that line, so it gains no new row. Merge after j4k/review#216.
feat: suggest a re-ask when the sandbox fleet stays full
All checks were successful
commit-msg / commitlint (pull_request) Successful in 24s
Details
Checks / quality-checks (pull_request) Successful in 47s
Details
Review / Review (pull_request_target) Successful in 8m6s
Details
67ef4e4cba
forgejo-actions commented 2026-10-05 07:12:17 +00:00
Copy link

Review 01M45QBS1WFBV7E2NAZFXYZ9ZD — head dd1ff4b30df32cc6b6fb707acc25c7f50216c3cb

Review — j4k-oss/review-wrapper @ c01ba82f21

Scope: diff against base tree 8869a4f97f96
Status: dispatched — coverage complete (5/5 slots terminal)
Facts: current review-wide projection

Computed under:

{
  "abandonment": "abandonment-v1",
  "anchor_recipe": 1,
  "batch_policy": "batch-v1",
  "coverage": "coverage-v3",
  "dispatch_policy": "dispatch-v2",
  "grounder_version": 1,
  "grounding_read_rule": "grounding-read-v1",
  "promotion_policy": "promotion-v1",
  "report": "report-v4",
  "tally": "tally-v1",
  "triage_settle": "triage-settle-v2"
}

Findings (1)

low — formatExecutionRecovery comment says an unnamed code "keeps its identity" instead of saying the code is appended

  • claim: 01M45QF6WKVH09QZH5YYBX0602
  • anchor: src/wrapper/remedies.ts (snippet)
  // The bundled service package can lag this table; a code it cannot name keeps its identity.
  • lens: writing-quality · arm: default
  • verdicts: 1 valid / 0 invalid / 0 uncertain
    • pass 01M45QG80KSK8JREWE0XQ0PJQN · valid: The grounded comment ('a code it cannot name keeps its identity') uses a metaphor where the adjacent branch does something specific: per the reviewer's quoted body, a schema-rejected failure gets '(code "...")' appended because formatExecutionFailure prints only the generic 'unrecognized reason' text. The quoted test output shows that mechanism. The skill asks for verifiable language that leads with the action and its object. 'Keeps its identity' names neither the action (appending) nor the reason (the generic message drops the code), so a literal reader cannot tell why the safeParse branch exists. The proposed rewrite keeps both facts the original carries: the table can name codes the bundled schema lacks, and the code reaches the log line. It states them as the mechanism the next line implements and adds no unnecessary detail. No fact is lost. Low severity is appropriate.
  • disposition: none

The comment uses a metaphor where the code needs a plain statement. "Keeps its identity" does not tell a maintainer what happens: that formatExecutionFailure prints the generic execution failed for an unrecognized reason for a code the bundled @j4k/review schema rejects, so this function appends (code "…"). A literal reader cannot tell from the comment why the safeParse branch exists, or that the second clause is the reason for the appended code. The skill asks for precise, verifiable language and for the action plus its object.

The function's body:

const message = ExecutionFailure.safeParse(failure).success
  ? formatExecutionFailure(failure)
  : `${formatExecutionFailure(failure)} (code ${JSON.stringify(failure.code)})`;

The orchestrator test added in this diff shows the result for sandbox-capacity-exhausted. That code is in the wrapper's executionRemedies table but not in the package's schema, and the log line reads execution failed for an unrecognized reason (code "sandbox-capacity-exhausted") — wait for the review burst to drain; ....

Correction: replace the line with "formatExecutionFailure prints a generic message for a code the bundled @j4k/review schema rejects, so append the code. This table can name codes that schema lacks." This keeps both facts the original carries: the table can lead the package, and the code survives into the log line. It also states them as the mechanism the next line implements. Keep the existing JSON-quoting comment below it.

I read all of src/wrapper/remedies.ts, the diff's new orchestrator test, and the README paragraph on unrecognized codes. I could not open @j4k/review's formatExecutionFailure because node_modules is not in the tree. The generic-message behaviour comes from the test's expected strings. A formatExecutionFailure that already printed the code for schema-rejected input would refute the stated mechanism, but the test strings show it does not.

Other claims

  • grounding-pending (0)
  • ungrounded (0)
  • rejected (1)
    • 01M45QFFJ581H4QV9XV9PBE9PA low — README says "the bundled service package" without naming @j4k/review, the dependency an operator must bump to get a named message
  • duplicate-of (0)
  • unadjudicated (0)

Coverage

Coverage pass: 01M45QBS4YD5D84EXHVADD48MD
Accounting: complete
Slot health: healthy

lens part arm unit status runs loss
general-bug whole default no-claims 1 no
writing-quality whole default claims-emitted 1 no
test-trimming whole default no-claims 1 no
restated-sets whole default no-claims 1 no
project-docs whole default no-claims 1 no
<!-- review:summary --> **Review** `01M45QBS1WFBV7E2NAZFXYZ9ZD` — head `dd1ff4b30df32cc6b6fb707acc25c7f50216c3cb` # Review — j4k-oss/review-wrapper @ c01ba82f215f Scope: diff against base tree `8869a4f97f96` Status: dispatched — coverage complete (5/5 slots terminal) Facts: current review-wide projection Computed under: ```json { "abandonment": "abandonment-v1", "anchor_recipe": 1, "batch_policy": "batch-v1", "coverage": "coverage-v3", "dispatch_policy": "dispatch-v2", "grounder_version": 1, "grounding_read_rule": "grounding-read-v1", "promotion_policy": "promotion-v1", "report": "report-v4", "tally": "tally-v1", "triage_settle": "triage-settle-v2" } ``` ## Findings (1) ### low — `formatExecutionRecovery` comment says an unnamed code "keeps its identity" instead of saying the code is appended - claim: `01M45QF6WKVH09QZH5YYBX0602` - anchor: `src/wrapper/remedies.ts` (snippet) ``` // The bundled service package can lag this table; a code it cannot name keeps its identity. ``` - lens: writing-quality · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - pass `01M45QG80KSK8JREWE0XQ0PJQN` · valid: The grounded comment ('a code it cannot name keeps its identity') uses a metaphor where the adjacent branch does something specific: per the reviewer's quoted body, a schema-rejected failure gets '(code "...")' appended because formatExecutionFailure prints only the generic 'unrecognized reason' text. The quoted test output shows that mechanism. The skill asks for verifiable language that leads with the action and its object. 'Keeps its identity' names neither the action (appending) nor the reason (the generic message drops the code), so a literal reader cannot tell why the safeParse branch exists. The proposed rewrite keeps both facts the original carries: the table can name codes the bundled schema lacks, and the code reaches the log line. It states them as the mechanism the next line implements and adds no unnecessary detail. No fact is lost. Low severity is appropriate. - disposition: none > The comment uses a metaphor where the code needs a plain statement. "Keeps its identity" does not tell a maintainer what happens: that `formatExecutionFailure` prints the generic `execution failed for an unrecognized reason` for a code the bundled `@j4k/review` schema rejects, so this function appends `(code "…")`. A literal reader cannot tell from the comment why the `safeParse` branch exists, or that the second clause is the reason for the appended code. The skill asks for precise, verifiable language and for the action plus its object. > > The function's body: > > ``` > const message = ExecutionFailure.safeParse(failure).success > ? formatExecutionFailure(failure) > : `${formatExecutionFailure(failure)} (code ${JSON.stringify(failure.code)})`; > ``` > > The orchestrator test added in this diff shows the result for `sandbox-capacity-exhausted`. That code is in the wrapper's `executionRemedies` table but not in the package's schema, and the log line reads `execution failed for an unrecognized reason (code "sandbox-capacity-exhausted") — wait for the review burst to drain; ...`. > > Correction: replace the line with "`formatExecutionFailure` prints a generic message for a code the bundled `@j4k/review` schema rejects, so append the code. This table can name codes that schema lacks." This keeps both facts the original carries: the table can lead the package, and the code survives into the log line. It also states them as the mechanism the next line implements. Keep the existing JSON-quoting comment below it. > > I read all of `src/wrapper/remedies.ts`, the diff's new orchestrator test, and the README paragraph on unrecognized codes. I could not open `@j4k/review`'s `formatExecutionFailure` because `node_modules` is not in the tree. The generic-message behaviour comes from the test's expected strings. A `formatExecutionFailure` that already printed the code for schema-rejected input would refute the stated mechanism, but the test strings show it does not. ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (1) - `01M45QFFJ581H4QV9XV9PBE9PA` low — README says "the bundled service package" without naming `@j4k/review`, the dependency an operator must bump to get a named message - duplicate-of (0) - unadjudicated (0) ## Coverage Coverage pass: 01M45QBS4YD5D84EXHVADD48MD Accounting: complete Slot health: healthy | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | no-claims | 1 | no | | writing-quality | whole | default | claims-emitted | 1 | no | | test-trimming | whole | default | no-claims | 1 | no | | restated-sets | whole | default | no-claims | 1 | no | | project-docs | whole | default | no-claims | 1 | no |
🚀 1
forgejo-actions reviewed 2026-10-05 07:12:17 +00:00
src/wrapper/remedies.ts Outdated
Lines 56-57
@ -53,6 +53,8 @@ const executionRemedies: Record<string, string | undefined> = {
"ask the operator to inspect the failed agent execution logs and correct the reported error",
"infrastructure-failure":
"ask the operator to check sandbox provisioning, controller connectivity, and resource availability",
"sandbox-capacity-exhausted":
"re-ask once the review burst drains; if it repeats, ask the operator to check axsandbox capacity and leaked sandboxes",
forgejo-actions commented 2026-10-05 07:12:17 +00:00
Copy link

medium — New sandbox capacity failure loses its identity in wrapper logs

A sandbox capacity failure logs execution failed for an unrecognized reason without its code. Operators cannot tell which failure triggered the new recovery advice, and the README's instruction to copy the quoted code for unrecognized failures cannot be followed.

The new executionRemedies entry makes formatExecutionRecovery take its known-remedy branch, which omits (code ...). The bundled formatExecutionFailure still recognizes only provider-session-limit, provider-capacity, agent-execution-failed, and infrastructure-failure, so it returns the generic message for sandbox-capacity-exhausted. Include the code when the message formatter does not recognize it, or update the dependency's message mapping before suppressing the code.

I traced classifyOutcome through both triage failure and failed-slot diagnostics to formatExecutionRecovery, checked the bundled dependency formatter and the README recovery table, and evaluated those exact functions extracted from dist/index.mjs with {version:1, code:'sandbox-capacity-exhausted'}. The observed result was execution failed for an unrecognized reason — re-ask once the review burst drains; if it repeats, ask the operator to check axsandbox capacity and leaked sandboxes. A bundled formatter that names this code, or a log that retains the code on this branch, would refute the defect.

lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M45E6D8BE8DQE4SHMTDD4Z0V of review 01M45E1770KAFZ0NWDV0173NDA

<!-- review:claim:01M45E6D8BE8DQE4SHMTDD4Z0V --> **medium** — New sandbox capacity failure loses its identity in wrapper logs > A sandbox capacity failure logs `execution failed for an unrecognized reason` without its code. Operators cannot tell which failure triggered the new recovery advice, and the README's instruction to copy the quoted code for unrecognized failures cannot be followed. > > The new `executionRemedies` entry makes `formatExecutionRecovery` take its known-remedy branch, which omits `(code ...)`. The bundled `formatExecutionFailure` still recognizes only `provider-session-limit`, `provider-capacity`, `agent-execution-failed`, and `infrastructure-failure`, so it returns the generic message for `sandbox-capacity-exhausted`. Include the code when the message formatter does not recognize it, or update the dependency's message mapping before suppressing the code. > > I traced `classifyOutcome` through both triage failure and failed-slot diagnostics to `formatExecutionRecovery`, checked the bundled dependency formatter and the README recovery table, and evaluated those exact functions extracted from `dist/index.mjs` with `{version:1, code:'sandbox-capacity-exhausted'}`. The observed result was `execution failed for an unrecognized reason — re-ask once the review burst drains; if it repeats, ask the operator to check axsandbox capacity and leaked sandboxes`. A bundled formatter that names this code, or a log that retains the code on this branch, would refute the defect. lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M45E6D8BE8DQE4SHMTDD4Z0V` of review `01M45E1770KAFZ0NWDV0173NDA`
👍 1
forgejo-actions commented 2026-10-05 07:12:17 +00:00
Copy link

medium — Sandbox capacity remedy tells readers to re-ask before the context-specific retry

A failed-triage reader is told to re-ask and then click rerun, even though a rerun is the action that requests re-triage. Re-asking can start a separate dispatch pass, so the diagnostic can send the reader through an unnecessary review attempt. For a failed lens slot, the diagnostic likewise says to re-ask twice.

The new shared remedy begins “re-ask once the review burst drains”. formatExecutionRecovery inserts it into both lossySlotLabels and formatStuckRemedy; the latter appends “then click rerun to re-triage” for failed triage and a concrete fgj actions workflow run review.yml re-ask command for partial coverage. The README’s rerun semantics distinguish these actions: a job rerun requests re-triage after failed triage, while a workflow dispatch asks for a dispatch pass.

Make the shared remedy describe the condition to wait for and the operator escalation, for example “wait for sandbox capacity to become available; if the failure recurs, ask the operator to check axsandbox capacity and leaked sandboxes”. Let the existing context-specific suffix tell the reader which retry to perform. This preserves the capacity and leak guidance while giving each reader one actionable retry.

I traced formatExecutionRecovery through classifyOutcome.lossySlotLabels and formatStuckRemedy, and checked the README’s rerun semantics and the new test’s expected log line. This is a static source trace; I did not run the wrapper. The claim would be refuted if a service re-ask and the job rerun were deliberately required as two separate steps after this failure, but the present README and wrapper flow describe one retry per context.

lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M45E99GYCSQMZ52BMKD944YF of review 01M45E1770KAFZ0NWDV0173NDA

<!-- review:claim:01M45E99GYCSQMZ52BMKD944YF --> **medium** — Sandbox capacity remedy tells readers to re-ask before the context-specific retry > A failed-triage reader is told to re-ask and then click rerun, even though a rerun is the action that requests re-triage. Re-asking can start a separate dispatch pass, so the diagnostic can send the reader through an unnecessary review attempt. For a failed lens slot, the diagnostic likewise says to re-ask twice. > > The new shared remedy begins “re-ask once the review burst drains”. formatExecutionRecovery inserts it into both lossySlotLabels and formatStuckRemedy; the latter appends “then click rerun to re-triage” for failed triage and a concrete fgj actions workflow run review.yml re-ask command for partial coverage. The README’s rerun semantics distinguish these actions: a job rerun requests re-triage after failed triage, while a workflow dispatch asks for a dispatch pass. > > Make the shared remedy describe the condition to wait for and the operator escalation, for example “wait for sandbox capacity to become available; if the failure recurs, ask the operator to check axsandbox capacity and leaked sandboxes”. Let the existing context-specific suffix tell the reader which retry to perform. This preserves the capacity and leak guidance while giving each reader one actionable retry. > > I traced formatExecutionRecovery through classifyOutcome.lossySlotLabels and formatStuckRemedy, and checked the README’s rerun semantics and the new test’s expected log line. This is a static source trace; I did not run the wrapper. The claim would be refuted if a service re-ask and the job rerun were deliberately required as two separate steps after this failure, but the present README and wrapper flow describe one retry per context. lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M45E99GYCSQMZ52BMKD944YF` of review `01M45E1770KAFZ0NWDV0173NDA`
👍 1
jercik commented 2026-10-05 07:23:44 +00:00
Author
Owner
Copy link

Fixed in ef857457a7: formatExecutionRecovery now keeps (code "…") whenever the bundled @j4k/review cannot parse the code as a named ExecutionFailure, and appends the wrapper's remedy after it. With @j4k/review 2.8.0 the line reads execution failed for an unrecognized reason (code "sandbox-capacity-exhausted") — wait for the review burst to drain; …; once the bump names the code, it reads sandbox fleet at capacity — ….

<!-- gh-feedback:reply-to:127087 --> Fixed in ef857457a77b15f44c7fad2ece9569e752088f05: formatExecutionRecovery now keeps `(code "…")` whenever the bundled @j4k/review cannot parse the code as a named ExecutionFailure, and appends the wrapper's remedy after it. With @j4k/review 2.8.0 the line reads `execution failed for an unrecognized reason (code "sandbox-capacity-exhausted") — wait for the review burst to drain; …`; once the bump names the code, it reads `sandbox fleet at capacity — …`.
jercik commented 2026-10-05 07:23:45 +00:00
Author
Owner
Copy link

Fixed in ef857457a7: the remedy is now "wait for the review burst to drain; if the failure repeats, ask the operator to check axsandbox capacity and leaked sandboxes", so failed triage keeps its single "click rerun" step and failed slots keep the single dispatch re-ask command.

<!-- gh-feedback:reply-to:127088 --> Fixed in ef857457a77b15f44c7fad2ece9569e752088f05: the remedy is now "wait for the review burst to drain; if the failure repeats, ask the operator to check axsandbox capacity and leaked sandboxes", so failed triage keeps its single "click rerun" step and failed slots keep the single dispatch re-ask command.
jercik marked this conversation as resolved
fix: keep the failure code the bundled service package cannot name
Some checks failed
commit-msg / commitlint (pull_request) Successful in 27s
Details
Checks / quality-checks (pull_request) Successful in 1m0s
Details
Review / Review (pull_request_target) Failing after 6m51s
Details
ef857457a7
 
Reword the capacity remedy as a wait so each context's own retry stays the one
action, and list it in the README recovery table.
jercik changed title from feat: suggest a re-ask when the sandbox fleet stays full to feat: tell users to wait out a full sandbox fleet 2026-10-05 07:24:00 +00:00
jercik commented 2026-10-05 07:24:07 +00:00
Author
Owner
Copy link

Replying to review comment #127086

The report-only high finding "The recovery table omits the new sandbox-capacity remedy" (claim 01M45E7Q298E33W5FFT48NACYT) is fixed in ef857457a7: the README table now has a sandbox fleet at capacity row, and the unrecognized-reason row says to follow the remedy printed after the quoted code. I kept the table rather than replacing it with a pointer, because it predates this PR and replacing it is a separate docs change.

> Replying to review comment #127086 The report-only **high** finding "The recovery table omits the new sandbox-capacity remedy" (claim `01M45E7Q298E33W5FFT48NACYT`) is fixed in ef857457a77b15f44c7fad2ece9569e752088f05: the README table now has a `sandbox fleet at capacity` row, and the unrecognized-reason row says to follow the remedy printed after the quoted code. I kept the table rather than replacing it with a pointer, because it predates this PR and replacing it is a separate docs change.
forgejo-actions reviewed 2026-10-05 07:30:50 +00:00
README.md Outdated
Lines 129-136
@ -132,3 +129,8 @@
| `agent execution failed` | Ask the operator to inspect the failed agent logs and correct the reported error. |
| `sandbox infrastructure failed` | Ask the operator to check sandbox provisioning, controller connectivity, and resources. |
| `execution failed for an unrecognized reason` | Copy the quoted code from `(code "…")` on that log line and ask the operator to look it up in the review service logs. |
| Message | Recovery |
| --------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `provider session limit reached` | Wait for the session limit to reset or ask the operator to lift it. |
| `selected model at capacity` | Wait for the model to accept new work. |
| `agent execution failed` | Ask the operator to inspect the failed agent logs and correct the reported error. |
| `sandbox infrastructure failed` | Ask the operator to check sandbox provisioning, controller connectivity, and resources. |
| `sandbox fleet at capacity` | Wait for the review burst to drain; if the failure repeats, ask the operator to check axsandbox capacity and leaked sandboxes. |
| `execution failed for an unrecognized reason` | Follow the remedy printed after the quoted code; without one, copy the code from `(code "…")` and ask the operator to look it up in the review service logs. |
forgejo-actions commented 2026-10-05 07:30:50 +00:00
Copy link

high — Recovery table names a sandbox message the bundle never prints

The recovery table already gives a message that this bundle never prints: sandbox fleet at capacity. For sandbox-capacity-exhausted, the wrapper prints execution failed for an unrecognized reason (code "sandbox-capacity-exhausted") followed by the capacity remedy. An operator looking for the table's named message will not find it. The table also duplicates the code-defined recovery set, so later code changes can leave this guidance stale.

src/wrapper/remedies.ts defines the remedy keys provider-session-limit, provider-capacity, agent-execution-failed, infrastructure-failure, and sandbox-capacity-exhausted. The table covers the first four through their displayed messages and adds sandbox fleet at capacity for the fifth; its last row describes the generic fallback rather than another key. The fifth code is the only mismatched member: its actual displayed message is absent from the table, while the table's displayed message has no producing branch.

Replace the message and recovery table with a pointer to the recovery text printed by formatExecutionRecovery in the job log. The general instruction to rerun after resolving the cause can stay.

I traced formatExecutionRecovery through classifyOutcome and formatStuckRemedy, and checked the bundled ExecutionFailure enum and formatExecutionFailure in dist/index.mjs. The bundle recognizes only the first four codes; src/wrapper/orchestrator.test.ts explicitly expects the unrecognized message and quoted fifth code. This is a static source trace and an inspection of the existing test, not a new run. The bundle's enum and formatter, or a job log from the fifth-code case, decide whether the table's message is actually emitted.

This is operational guidance, not a table of contents or a dated record. The source and the log output are available to its reader.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M45FDNEQ7NQNMVDZEHVPWXZ7 of review 01M45F4VN1GX9MAASJVKX47KFW

<!-- review:claim:01M45FDNEQ7NQNMVDZEHVPWXZ7 --> **high** — Recovery table names a sandbox message the bundle never prints > The recovery table already gives a message that this bundle never prints: `sandbox fleet at capacity`. For `sandbox-capacity-exhausted`, the wrapper prints `execution failed for an unrecognized reason (code "sandbox-capacity-exhausted")` followed by the capacity remedy. An operator looking for the table's named message will not find it. The table also duplicates the code-defined recovery set, so later code changes can leave this guidance stale. > > `src/wrapper/remedies.ts` defines the remedy keys `provider-session-limit`, `provider-capacity`, `agent-execution-failed`, `infrastructure-failure`, and `sandbox-capacity-exhausted`. The table covers the first four through their displayed messages and adds `sandbox fleet at capacity` for the fifth; its last row describes the generic fallback rather than another key. The fifth code is the only mismatched member: its actual displayed message is absent from the table, while the table's displayed message has no producing branch. > > Replace the message and recovery table with a pointer to the recovery text printed by `formatExecutionRecovery` in the job log. The general instruction to rerun after resolving the cause can stay. > > I traced `formatExecutionRecovery` through `classifyOutcome` and `formatStuckRemedy`, and checked the bundled `ExecutionFailure` enum and `formatExecutionFailure` in `dist/index.mjs`. The bundle recognizes only the first four codes; `src/wrapper/orchestrator.test.ts` explicitly expects the unrecognized message and quoted fifth code. This is a static source trace and an inspection of the existing test, not a new run. The bundle's enum and formatter, or a job log from the fifth-code case, decide whether the table's message is actually emitted. > > This is operational guidance, not a table of contents or a dated record. The source and the log output are available to its reader. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M45FDNEQ7NQNMVDZEHVPWXZ7` of review `01M45F4VN1GX9MAASJVKX47KFW`
👍 1
jercik commented 2026-10-05 07:42:05 +00:00
Author
Owner
Copy link

Fixed in 269397a719: the sandbox fleet at capacity row is gone. With the bundled @j4k/review 2.8.0 this failure prints execution failed for an unrecognized reason (code "sandbox-capacity-exhausted") followed by the capacity remedy, which the unrecognized-reason row now covers ("Follow the remedy printed after the quoted code"). The named row belongs with the @j4k/review bump that makes the bundle print it. I kept the table itself; replacing it with a pointer is a separate docs change outside this PR.

<!-- gh-feedback:reply-to:127250 --> Fixed in 269397a7197c875f131d68590889e3708a90a9ec: the `sandbox fleet at capacity` row is gone. With the bundled @j4k/review 2.8.0 this failure prints `execution failed for an unrecognized reason (code "sandbox-capacity-exhausted")` followed by the capacity remedy, which the unrecognized-reason row now covers ("Follow the remedy printed after the quoted code"). The named row belongs with the @j4k/review bump that makes the bundle print it. I kept the table itself; replacing it with a pointer is a separate docs change outside this PR.
jercik marked this conversation as resolved
docs: drop the recovery row the bundled service package cannot print yet
All checks were successful
commit-msg / commitlint (pull_request) Successful in 25s
Details
Checks / quality-checks (pull_request) Successful in 48s
Details
Review / Review (pull_request_target) Successful in 4s
Details
269397a719
forgejo-actions reviewed 2026-10-05 09:41:02 +00:00
README.md Outdated
Lines 129-135
@ -132,3 +129,7 @@
| `agent execution failed` | Ask the operator to inspect the failed agent logs and correct the reported error. |
| `sandbox infrastructure failed` | Ask the operator to check sandbox provisioning, controller connectivity, and resources. |
| `execution failed for an unrecognized reason` | Copy the quoted code from `(code "…")` on that log line and ask the operator to look it up in the review service logs. |
| Message | Recovery |
| --------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `provider session limit reached` | Wait for the session limit to reset or ask the operator to lift it. |
| `selected model at capacity` | Wait for the model to accept new work. |
| `agent execution failed` | Ask the operator to inspect the failed agent logs and correct the reported error. |
| `sandbox infrastructure failed` | Ask the operator to check sandbox provisioning, controller connectivity, and resources. |
| `execution failed for an unrecognized reason` | Follow the remedy printed after the quoted code; without one, copy the code from `(code "…")` and ask the operator to look it up in the review service logs. |
forgejo-actions commented 2026-10-05 09:41:02 +00:00
Copy link

medium — Recovery table copies the bundled failure-message catalog

The recovery table maintains a second copy of the execution-failure message catalog. Updating that catalog requires a separate documentation edit, so readers can silently receive an obsolete catalog even though the job logs use the current implementation.

The defining source is formatExecutionFailure in dist/index.mjs, bundled from the owner's j4k/review package. Its message values, including the fallback, are provider session limit reached, selected model at capacity, agent execution failed, sandbox infrastructure failed, and execution failed for an unrecognized reason. The README's Message column contains those same members. Neither list has a member absent from the other; the copy currently agrees. Recovery guidance is already defined by executionRemedies and formatExecutionRecovery in src/wrapper/remedies.ts, including the newly added code-specific remedy under the fallback message.

Replace the table with: “Before retrying, follow the recovery guidance printed in the job log by formatExecutionRecovery in src/wrapper/remedies.ts.” The table's recovery detail is already present in that implementation, so no member-specific detail needs to remain in this prose.

I inspected the README, the complete remedies implementation, the bundled failure formatter, and the calls from classifyOutcome and lossySlotLabels in src/wrapper/classify-outcome.ts. I also read the failure cases in src/wrapper/orchestrator.test.ts; I did not execute tests. This is current operational documentation, not a dated record, an example list, a table of contents, or an executable specification. Readers can open the defining source, and the package is owned by j4k, so the third-party exception does not apply.

The matching members establish a synchronized copy rather than an existing mismatch. Evidence that the table itself generates the runtime catalog, or that its intended readers cannot access the source and require an independent catalog, would refute this finding; neither is shown by the inspected files.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M45PKAEMBBNNKRD9F7R8FPMY of review 01M45G6KCM3TQMNNYMYKD83TBC

<!-- review:claim:01M45PKAEMBBNNKRD9F7R8FPMY --> **medium** — Recovery table copies the bundled failure-message catalog > The recovery table maintains a second copy of the execution-failure message catalog. Updating that catalog requires a separate documentation edit, so readers can silently receive an obsolete catalog even though the job logs use the current implementation. > > The defining source is `formatExecutionFailure` in `dist/index.mjs`, bundled from the owner's `j4k/review` package. Its message values, including the fallback, are `provider session limit reached`, `selected model at capacity`, `agent execution failed`, `sandbox infrastructure failed`, and `execution failed for an unrecognized reason`. The README's Message column contains those same members. Neither list has a member absent from the other; the copy currently agrees. Recovery guidance is already defined by `executionRemedies` and `formatExecutionRecovery` in `src/wrapper/remedies.ts`, including the newly added code-specific remedy under the fallback message. > > Replace the table with: “Before retrying, follow the recovery guidance printed in the job log by `formatExecutionRecovery` in `src/wrapper/remedies.ts`.” The table's recovery detail is already present in that implementation, so no member-specific detail needs to remain in this prose. > > I inspected the README, the complete remedies implementation, the bundled failure formatter, and the calls from `classifyOutcome` and `lossySlotLabels` in `src/wrapper/classify-outcome.ts`. I also read the failure cases in `src/wrapper/orchestrator.test.ts`; I did not execute tests. This is current operational documentation, not a dated record, an example list, a table of contents, or an executable specification. Readers can open the defining source, and the package is owned by `j4k`, so the third-party exception does not apply. > > The matching members establish a synchronized copy rather than an existing mismatch. Evidence that the table itself generates the runtime catalog, or that its intended readers cannot access the source and require an independent catalog, would refute this finding; neither is shown by the inspected files. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M45PKAEMBBNNKRD9F7R8FPMY` of review `01M45G6KCM3TQMNNYMYKD83TBC`
👍 1
jercik commented 2026-10-05 09:47:28 +00:00
Author
Owner
Copy link

Fixed in dd1ff4b: rebased onto main, whose #34 already replaced the recovery table with a pointer to the remedy printed by formatExecutionRecovery; this branch no longer edits the table.

<!-- gh-feedback:reply-to:127794 --> Fixed in dd1ff4b: rebased onto main, whose #34 already replaced the recovery table with a pointer to the remedy printed by formatExecutionRecovery; this branch no longer edits the table.
jercik marked this conversation as resolved
jercik force-pushed feat/sandbox-capacity-remedy from 269397a719
All checks were successful
commit-msg / commitlint (pull_request) Successful in 25s
Details
Checks / quality-checks (pull_request) Successful in 48s
Details
Review / Review (pull_request_target) Successful in 4s
Details
to dd1ff4b30d
All checks were successful
commit-msg / commitlint (pull_request) Successful in 24s
Details
Checks / quality-checks (pull_request) Successful in 49s
Details
Review / Review (pull_request_target) Successful in 3m36s
Details
2026-10-05 09:47:14 +00:00 Compare
forgejo-actions reviewed 2026-10-05 09:51:17 +00:00
src/wrapper/remedies.ts
@ -62,3 +64,1 @@
if (remedy !== undefined) {
return `${formatExecutionFailure(failure)} — ${remedy}`;
}
// The bundled service package can lag this table; a code it cannot name keeps its identity.
forgejo-actions commented 2026-10-05 09:51:17 +00:00
Copy link

low — formatExecutionRecovery comment says an unnamed code "keeps its identity" instead of saying the code is appended

The comment uses a metaphor where the code needs a plain statement. "Keeps its identity" does not tell a maintainer what happens: that formatExecutionFailure prints the generic execution failed for an unrecognized reason for a code the bundled @j4k/review schema rejects, so this function appends (code "…"). A literal reader cannot tell from the comment why the safeParse branch exists, or that the second clause is the reason for the appended code. The skill asks for precise, verifiable language and for the action plus its object.

The function's body:

const message = ExecutionFailure.safeParse(failure).success
  ? formatExecutionFailure(failure)
  : `${formatExecutionFailure(failure)} (code ${JSON.stringify(failure.code)})`;

The orchestrator test added in this diff shows the result for sandbox-capacity-exhausted. That code is in the wrapper's executionRemedies table but not in the package's schema, and the log line reads execution failed for an unrecognized reason (code "sandbox-capacity-exhausted") — wait for the review burst to drain; ....

Correction: replace the line with "formatExecutionFailure prints a generic message for a code the bundled @j4k/review schema rejects, so append the code. This table can name codes that schema lacks." This keeps both facts the original carries: the table can lead the package, and the code survives into the log line. It also states them as the mechanism the next line implements. Keep the existing JSON-quoting comment below it.

I read all of src/wrapper/remedies.ts, the diff's new orchestrator test, and the README paragraph on unrecognized codes. I could not open @j4k/review's formatExecutionFailure because node_modules is not in the tree. The generic-message behaviour comes from the test's expected strings. A formatExecutionFailure that already printed the code for schema-rejected input would refute the stated mechanism, but the test strings show it does not.

lens writing-quality · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M45QF6WKVH09QZH5YYBX0602 of review 01M45QBS1WFBV7E2NAZFXYZ9ZD

<!-- review:claim:01M45QF6WKVH09QZH5YYBX0602 --> **low** — `formatExecutionRecovery` comment says an unnamed code "keeps its identity" instead of saying the code is appended > The comment uses a metaphor where the code needs a plain statement. "Keeps its identity" does not tell a maintainer what happens: that `formatExecutionFailure` prints the generic `execution failed for an unrecognized reason` for a code the bundled `@j4k/review` schema rejects, so this function appends `(code "…")`. A literal reader cannot tell from the comment why the `safeParse` branch exists, or that the second clause is the reason for the appended code. The skill asks for precise, verifiable language and for the action plus its object. > > The function's body: > > ``` > const message = ExecutionFailure.safeParse(failure).success > ? formatExecutionFailure(failure) > : `${formatExecutionFailure(failure)} (code ${JSON.stringify(failure.code)})`; > ``` > > The orchestrator test added in this diff shows the result for `sandbox-capacity-exhausted`. That code is in the wrapper's `executionRemedies` table but not in the package's schema, and the log line reads `execution failed for an unrecognized reason (code "sandbox-capacity-exhausted") — wait for the review burst to drain; ...`. > > Correction: replace the line with "`formatExecutionFailure` prints a generic message for a code the bundled `@j4k/review` schema rejects, so append the code. This table can name codes that schema lacks." This keeps both facts the original carries: the table can lead the package, and the code survives into the log line. It also states them as the mechanism the next line implements. Keep the existing JSON-quoting comment below it. > > I read all of `src/wrapper/remedies.ts`, the diff's new orchestrator test, and the README paragraph on unrecognized codes. I could not open `@j4k/review`'s `formatExecutionFailure` because `node_modules` is not in the tree. The generic-message behaviour comes from the test's expected strings. A `formatExecutionFailure` that already printed the code for schema-rejected input would refute the stated mechanism, but the test strings show it does not. lens `writing-quality` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M45QF6WKVH09QZH5YYBX0602` of review `01M45QBS1WFBV7E2NAZFXYZ9ZD`
jercik merged commit 65f9120042 into main 2026-10-05 11:01:43 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
j4k-oss/review-wrapper!48
No description provided.