[PR #2868] fix(adb): drop the remaining exclusivity claim from push guidance #5452

Open
opened 2026-10-31 01:06:45 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2868
Author: @CodesWithRobi
Created: 10/13/2026
Status: 🔄 Open

Base: main ← Head: fix/adb-push-not-only-mutator


📝 Commits (1)

  • 56bdd57 fix(adb): drop the remaining exclusivity claim from push guidance

📊 Changes

3 files changed (+17 additions, -4 deletions)

View changed files

📝 src/adb/AdbProvider.test.ts (+4 -0)
📝 src/adb/AdbProvider.ts (+5 -4)
📝 src/contracts/toolEffects.test.ts (+8 -0)

📄 Description

Summary

#2990 fixed the false exclusivity in the push_adb_file contract text and closed #2602, but the change stopped at the contract surface: AdbProvider's class comment still described the caller-requested file push as the only device mutation, and nothing guards that claim from returning.

This rebases onto current main and follows up on that fix (Refs #2602):

  • Correct the AdbProvider class comment; install, uninstall, start-app, start-activity, and stop-app also mutate device state.
  • Add a contract-wide regression test that no tool description claims to be the only device-mutating operation.
  • Assert the emitted device-mutation limitation for every push_adb_file digest verdict: device-side sha256sum verified, unavailable, and unrecognized output.

The contract description and limitation strings stay as main defines them, so this no longer duplicates the merged fix.

Problem and expected behavior

Smallest trigger: read AdbProvider's class comment on main after #2990 — it still says the file push is the only device mutation, although several ADB tools declare mutatesDevice: true in src/contracts/toolEffects.ts. Expected: no surface claims exclusivity, and a test keeps it that way.

Change and scope

Documentation and tests only; no behavior change. The contract description and limitation strings are unchanged from main, so the diff is limited to the missed comment and the new coverage. Not included: any change to ADB tool effects, schemas, or descriptions.

Contract and boundary impact

  • Semantic owner and earliest changed stage: AdbProvider doc comment (src/adb/AdbProvider.ts); tests in src/adb/AdbProvider.test.ts and src/contracts/toolEffects.test.ts
  • Contribution scope fit and linked maintainer decision: standalone correction within existing ADB boundaries; not applicable
  • CLI and MCP/tool-catalog contract: none (description text unchanged from main)
  • Provider, bridge, target-format, or platform compatibility: none
  • Evidence, artifact, provenance, or reconstruction contract: none
  • Target selection, authority, upstream behavior, and cleanup details: none
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: none

Expected effects (check known effects; use documentation-only on its own).

  • Read existing files or captured data.
  • Documentation or usability only; no target interaction.

Evidence and regression coverage

  • Tests added or updated: toolEffects.test.ts rejects any contract claiming to be the only device-mutating tool; AdbProvider.test.ts asserts the device-mutation limitation for every push digest verdict (verified, unavailable, unrecognized output).
  • Base reproduction or other evidence: before this change the AdbProvider comment claimed exclusivity while toolEffects.ts marks several ADB tools as device-mutating.
  • User-visible CLI/MCP output: none.
  • Remaining proof gaps: none for a documentation/test-only change.

For evidence-bearing changes: not applicable.

Validation performed

On main f4b2ba131 after rebasing:

  • npm run test:local -- src/adb/AdbProvider.test.ts src/contracts/toolEffects.test.ts — 99 passed
  • npm run check:fast — pass
  • npm run docs:check — pass

Main's #6010 renamed the push limitation to "Push writes a private snapshot of the caller's file to the device" and added a third digest verdict. The assertion uses toContain on that limitation, so it covers all verdicts without breaking on further wording changes.

Compatibility, safety, and release

  • Breaking changes or migration steps: none
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable
  • Package or release metadata impact: none
  • Security, privacy, process, or containment review: none; documentation and tests only

Author readiness

Human owner: @CodesWithRobi

  • 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 (#2602 is already completed by #2990; this is a follow-up).
  • 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. (not applicable)
  • 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/2868 **Author:** [@CodesWithRobi](https://github.com/CodesWithRobi) **Created:** 10/13/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/adb-push-not-only-mutator` --- ### 📝 Commits (1) - [`56bdd57`](https://github.com/morluto/rea/commit/56bdd577281478994680ad1c128a993e093256e9) fix(adb): drop the remaining exclusivity claim from push guidance ### 📊 Changes **3 files changed** (+17 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/adb/AdbProvider.test.ts` (+4 -0) 📝 `src/adb/AdbProvider.ts` (+5 -4) 📝 `src/contracts/toolEffects.test.ts` (+8 -0) </details> ### 📄 Description ## Summary #2990 fixed the false exclusivity in the `push_adb_file` contract text and closed #2602, but the change stopped at the contract surface: `AdbProvider`'s class comment still described the caller-requested file push as *the only device mutation*, and nothing guards that claim from returning. This rebases onto current `main` and follows up on that fix (`Refs #2602`): - Correct the `AdbProvider` class comment; `install`, `uninstall`, `start-app`, `start-activity`, and `stop-app` also mutate device state. - Add a contract-wide regression test that no tool description claims to be the only device-mutating operation. - Assert the emitted device-mutation limitation for every `push_adb_file` digest verdict: device-side `sha256sum` verified, unavailable, and unrecognized output. The contract description and limitation strings stay as `main` defines them, so this no longer duplicates the merged fix. ## Problem and expected behavior Smallest trigger: read `AdbProvider`'s class comment on `main` after #2990 — it still says the file push is the only device mutation, although several ADB tools declare `mutatesDevice: true` in `src/contracts/toolEffects.ts`. Expected: no surface claims exclusivity, and a test keeps it that way. ## Change and scope Documentation and tests only; no behavior change. The contract description and limitation strings are unchanged from `main`, so the diff is limited to the missed comment and the new coverage. Not included: any change to ADB tool effects, schemas, or descriptions. ## Contract and boundary impact - Semantic owner and earliest changed stage: `AdbProvider` doc comment (`src/adb/AdbProvider.ts`); tests in `src/adb/AdbProvider.test.ts` and `src/contracts/toolEffects.test.ts` - Contribution scope fit and linked maintainer decision: standalone correction within existing ADB boundaries; not applicable - CLI and MCP/tool-catalog contract: none (description text unchanged from `main`) - Provider, bridge, target-format, or platform compatibility: none - Evidence, artifact, provenance, or reconstruction contract: none - Target selection, authority, upstream behavior, and cleanup details: none - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none Expected effects (check known effects; use documentation-only on its own). - [x] Read existing files or captured data. - [x] Documentation or usability only; no target interaction. ## Evidence and regression coverage - Tests added or updated: `toolEffects.test.ts` rejects any contract claiming to be the only device-mutating tool; `AdbProvider.test.ts` asserts the device-mutation limitation for every push digest verdict (verified, unavailable, unrecognized output). - Base reproduction or other evidence: before this change the `AdbProvider` comment claimed exclusivity while `toolEffects.ts` marks several ADB tools as device-mutating. - User-visible CLI/MCP output: none. - Remaining proof gaps: none for a documentation/test-only change. For evidence-bearing changes: not applicable. ## Validation performed On `main` `f4b2ba131` after rebasing: - `npm run test:local -- src/adb/AdbProvider.test.ts src/contracts/toolEffects.test.ts` — 99 passed - `npm run check:fast` — pass - `npm run docs:check` — pass Main's #6010 renamed the push limitation to *"Push writes a private snapshot of the caller's file to the device"* and added a third digest verdict. The assertion uses `toContain` on that limitation, so it covers all verdicts without breaking on further wording changes. ## Compatibility, safety, and release - Breaking changes or migration steps: none - Real Hopper/Ghidra, browser, or OS coverage: not applicable - Package or release metadata impact: none - Security, privacy, process, or containment review: none; documentation and tests only ## Author readiness Human owner: @CodesWithRobi - [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 - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above (#2602 is already completed by #2990; this is a follow-up). - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. (not applicable) - [x] 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>
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#5452
No description provided.