[PR #7033] [CLOSED] test(firmware): exercise the staging ceiling through the emitted refusal #7591

Closed
opened 2026-11-03 13:38:56 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7033
Author: @XiaoBinGan
Created: 10/24/2026
Status: ❌ Closed

Base: main ← Head: fix/firmware-staging-ceiling-wiring


📝 Commits (2)

  • 0f6311e test(firmware): exercise the staging ceiling through the emitted refusal
  • 8fd1cf4 test(firmware): compare the emitted recovery with its owner

📊 Changes

1 file changed (+90 additions, -0 deletions)

View changed files

➕ tests/boundary/firmware/firmwareStagingBudgetWiring.test.ts (+90 -0)

📄 Description

Summary

#6971 established the staging-budget ceiling recovery but covered it through the wording helper
only. Review noted the follow-up: exercise the error inventoryFirmwareOutput actually emits, so the
production wiring is protected rather than just the string function.

This adds that boundary coverage. It is the follow-up requested in the #6971 review.

Problem and expected behavior

A wiring change that passed the schema ceiling where the selected value belongs, or that emitted an
ad-hoc action instead of the owner's, keeps a helper-only test green while the refusal handed to
callers is wrong. The gap is regression protection, not a behaviour defect: the current wiring is
correct.

Expected: the recovery emitted by inventoryFirmwareOutput equals
firmwareStagingBudgetRemediation(<field>, <selected value>) at the schema ceiling and below it.

Change and scope

Adds tests/boundary/firmware/firmwareStagingBudgetWiring.test.ts:

  • The byte ceiling is reached with a sparse file whose declared size is
    MAX_FIRMWARE_STAGING_BYTES + 1. The file reports the size without storing the bytes, so the test
    exercises the real schema ceiling instead of a toy budget. The projected refusal must carry
    reported_limits.max_output_bytes and must equal the owner's output exactly.
  • A one-byte staged tree with a selected budget of 1 reaches the same refusal below the ceiling, and
    the same exact-equality assertion covers the raise advice.
  • Exact equality rather than phrase matching, so an ad-hoc or truncated action that keeps the
    familiar sentences still fails.

Not included: no production change. The entry ceiling cannot be reached without staging more entries
than the schema permits, so its at-ceiling wording stays covered by
src/firmware/FirmwareStagingBudget.test.ts and its below-ceiling entry wiring by
firmwareReportFiles.test.ts. That limit is stated in the test file.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Not applicable — test-only coverage for an existing, already-integrated boundary.

Contract and boundary impact

  • Semantic owner and earliest changed stage: firmware staging-budget enforcement; no production stage
    changes.
  • Contribution scope fit and linked maintainer decision: not applicable; no behaviour change.
  • CLI and MCP/tool-catalog contract: unchanged; the assertions read the existing projection.
  • Provider, bridge, target-format, or platform compatibility: none. The sparse fixture uses
    FileHandle.truncate, which reports the declared length without storing it on the supported hosts;
    a host that allocated it would make the test slower, not wrong.
  • Evidence, artifact, provenance, or reconstruction contract: none.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): none.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/firmware/firmwareStagingBudgetWiring.test.ts (2 cases).
  • Base reproduction or other evidence: verified red in two directions, neither of which the helper
    test catches. Passing the schema ceiling as the selected value makes the below-ceiling case fail.
    Replacing the emitted action with a divergent sentence that keeps the ceiling phrases fails both
    cases under exact equality, where phrase matching passed. Both checks were run and reverted.
  • User-visible CLI/MCP output (if applicable): not applicable; this pins existing output.
  • Remaining proof gaps: the entry ceiling is not reachable in-process without staging more entries
    than the schema permits, so only the byte ceiling is exercised end to end. The test asserts the
    projected refusal, not a full extract_firmware MCP round trip, and no real Unblob extraction ran.

For boundary fixes, identify the applicable boundary regressions
and lifecycle regressions
covered by the evidence above, and explain relevant gaps.

No boundary regression class applies: this changes no production code path, no reader or process
lifetime, and no public contract. It pins an existing refusal's recovery against its owner.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • node node_modules/vitest/vitest.mjs run tests/boundary/firmware/firmwareStagingBudgetWiring.test.ts --maxWorkers=1 — 2 passed.
  • node node_modules/vitest/vitest.mjs run src/firmware tests/boundary/firmware --maxWorkers=1 — 112
    passed, 52 skipped (13 files; the skips are host-gated firmware integration and workspace recovery).
  • npx tsc --noEmit -p . — no errors.
  • npx oxfmt <changed file> — formatted.
  • npm run verify:module-boundaries — 3012 source files checked, no violations.
  • Red → green, three directions: schema-ceiling wiring → 1 failed; single-form remediation → 2
    failed; divergent action keeping the phrases → 2 failed; restored → 2 passed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none; test-only.
  • Real Hopper/Ghidra, browser, or OS coverage: no analysis engine involved.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no target execution and no new data exposure.

Author readiness

Human owner: @XiaoBinGan

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/7033 **Author:** [@XiaoBinGan](https://github.com/XiaoBinGan) **Created:** 10/24/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/firmware-staging-ceiling-wiring` --- ### 📝 Commits (2) - [`0f6311e`](https://github.com/morluto/rea/commit/0f6311e1db44d9eb6145934c7a603a84c3984dd1) test(firmware): exercise the staging ceiling through the emitted refusal - [`8fd1cf4`](https://github.com/morluto/rea/commit/8fd1cf446ac98f5f6ac5e94368f97bb2171813ce) test(firmware): compare the emitted recovery with its owner ### 📊 Changes **1 file changed** (+90 additions, -0 deletions) <details> <summary>View changed files</summary> ➕ `tests/boundary/firmware/firmwareStagingBudgetWiring.test.ts` (+90 -0) </details> ### 📄 Description ## Summary #6971 established the staging-budget ceiling recovery but covered it through the wording helper only. Review noted the follow-up: exercise the error `inventoryFirmwareOutput` actually emits, so the production wiring is protected rather than just the string function. This adds that boundary coverage. It is the follow-up requested in the #6971 review. ## Problem and expected behavior A wiring change that passed the schema ceiling where the selected value belongs, or that emitted an ad-hoc action instead of the owner's, keeps a helper-only test green while the refusal handed to callers is wrong. The gap is regression protection, not a behaviour defect: the current wiring is correct. Expected: the recovery emitted by `inventoryFirmwareOutput` equals `firmwareStagingBudgetRemediation(<field>, <selected value>)` at the schema ceiling and below it. ## Change and scope Adds `tests/boundary/firmware/firmwareStagingBudgetWiring.test.ts`: - The byte ceiling is reached with a sparse file whose declared size is `MAX_FIRMWARE_STAGING_BYTES + 1`. The file reports the size without storing the bytes, so the test exercises the real schema ceiling instead of a toy budget. The projected refusal must carry `reported_limits.max_output_bytes` and must equal the owner's output exactly. - A one-byte staged tree with a selected budget of 1 reaches the same refusal below the ceiling, and the same exact-equality assertion covers the raise advice. - Exact equality rather than phrase matching, so an ad-hoc or truncated action that keeps the familiar sentences still fails. Not included: no production change. The entry ceiling cannot be reached without staging more entries than the schema permits, so its at-ceiling wording stays covered by `src/firmware/FirmwareStagingBudget.test.ts` and its below-ceiling entry wiring by `firmwareReportFiles.test.ts`. That limit is stated in the test file. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. Not applicable — test-only coverage for an existing, already-integrated boundary. ## Contract and boundary impact - Semantic owner and earliest changed stage: firmware staging-budget enforcement; no production stage changes. - Contribution scope fit and linked maintainer decision: not applicable; no behaviour change. - CLI and MCP/tool-catalog contract: unchanged; the assertions read the existing projection. - Provider, bridge, target-format, or platform compatibility: none. The sparse fixture uses `FileHandle.truncate`, which reports the declared length without storing it on the supported hosts; a host that allocated it would make the test slower, not wrong. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): none. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/firmware/firmwareStagingBudgetWiring.test.ts` (2 cases). - Base reproduction or other evidence: verified red in two directions, neither of which the helper test catches. Passing the schema ceiling as the selected value makes the below-ceiling case fail. Replacing the emitted action with a divergent sentence that keeps the ceiling phrases fails both cases under exact equality, where phrase matching passed. Both checks were run and reverted. - User-visible CLI/MCP output (if applicable): not applicable; this pins existing output. - Remaining proof gaps: the entry ceiling is not reachable in-process without staging more entries than the schema permits, so only the byte ceiling is exercised end to end. The test asserts the projected refusal, not a full `extract_firmware` MCP round trip, and no real Unblob extraction ran. For boundary fixes, identify the applicable [boundary regressions](https://github.com/morluto/rea/blob/main/docs/testing.md#boundary-regressions) and [lifecycle regressions](https://github.com/morluto/rea/blob/main/docs/testing.md#lifecycle-regressions) covered by the evidence above, and explain relevant gaps. No boundary regression class applies: this changes no production code path, no reader or process lifetime, and no public contract. It pins an existing refusal's recovery against its owner. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `node node_modules/vitest/vitest.mjs run tests/boundary/firmware/firmwareStagingBudgetWiring.test.ts --maxWorkers=1` — 2 passed. - `node node_modules/vitest/vitest.mjs run src/firmware tests/boundary/firmware --maxWorkers=1` — 112 passed, 52 skipped (13 files; the skips are host-gated firmware integration and workspace recovery). - `npx tsc --noEmit -p .` — no errors. - `npx oxfmt <changed file>` — formatted. - `npm run verify:module-boundaries` — 3012 source files checked, no violations. - Red → green, three directions: schema-ceiling wiring → 1 failed; single-form remediation → 2 failed; divergent action keeping the phrases → 2 failed; restored → 2 passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none; test-only. - Real Hopper/Ghidra, browser, or OS coverage: no analysis engine involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no target execution and no new data exposure. ## Author readiness Human owner: @XiaoBinGan - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-03 13:38:56 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
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
morluto/rea#7591
No description provided.