fix(adb): share host configuration refusals with apktool
The ADB provider, which the apktool provider mirrors, had the same two defects: a relative REA_ADB_PATH escaped as UNKNOWN because selection ran before the try that maps configuration failures, and a missing adb binary projected generic target advice instead of its reason. Move the configuration-failure-to-refusal mapping into one providerConfigurationRefusal helper used by both providers, resolve the ADB selection inside the mapped try, and share the CLI refusal assertion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
9655845bbc
commit
92fddf3176
6 files changed
+85
-36
No files matched your search
@@ -10,12 +10,12 @@ import type {
|
||||
} from "../application/AnalysisProvider.js";
|
||||
import {
|
||||
createAnalysisExecution,
|
||||
providerConfigurationRefusal,
|
||||
type AnalysisExecution,
|
||||
} from "../application/AnalysisProvider.js";
|
||||
import type { AdbRequest } from "../domain/adb/adbDeviceAnalysis.js";
|
||||
import {
|
||||
AnalysisArtifactChangedError,
|
||||
AnalysisCapabilityUnavailableError,
|
||||
AnalysisCancelledError,
|
||||
AnalysisInputError,
|
||||
AnalysisOutputError,
|
||||
@@ -417,8 +417,8 @@ export class AdbProvider {
|
||||
async inspectAvailability(
|
||||
signal?: AbortSignal,
|
||||
): Promise<ProviderAvailability> {
|
||||
const selection = this.selection();
|
||||
try {
|
||||
const selection = this.selection();
|
||||
await inspectAdbClient(selection, signal);
|
||||
return {
|
||||
status: "available",
|
||||
@@ -555,21 +555,19 @@ export class AdbProvider {
|
||||
const signal = options?.signal;
|
||||
if (signal?.aborted === true)
|
||||
return err(new AnalysisCancelledError(request.operation));
|
||||
const selection = this.selection();
|
||||
let selection: AdbBinarySelection;
|
||||
let identity: AdbClientIdentity;
|
||||
try {
|
||||
selection = this.selection();
|
||||
identity = await inspectAdbClient(selection, signal);
|
||||
} catch (cause) {
|
||||
if (cause instanceof AnalysisCancelledError) return err(cause);
|
||||
if (cause instanceof AdbConfigurationFailure)
|
||||
return err(
|
||||
new AnalysisCapabilityUnavailableError(
|
||||
providerConfigurationRefusal(
|
||||
ADB_PROVIDER_IDENTITY.id,
|
||||
request.operation,
|
||||
typeof cause.diagnostics.remediation === "string"
|
||||
? `${cause.message} ${cause.diagnostics.remediation}`
|
||||
: cause.message,
|
||||
{ cause },
|
||||
cause,
|
||||
),
|
||||
);
|
||||
return err(
|
||||
|
||||
@@ -22,6 +22,7 @@ import type {
|
||||
} from "../application/AnalysisProvider.js";
|
||||
import {
|
||||
createAnalysisExecution,
|
||||
providerConfigurationRefusal,
|
||||
type AnalysisExecution,
|
||||
} from "../application/AnalysisProvider.js";
|
||||
import type { ApktoolRequest } from "../domain/apktool/apktoolResourceAnalysis.js";
|
||||
@@ -409,21 +410,14 @@ export class ApktoolProvider {
|
||||
);
|
||||
} catch (cause) {
|
||||
if (cause instanceof AnalysisCancelledError) return err(cause);
|
||||
if (cause instanceof ApktoolConfigurationFailure) {
|
||||
// A host configuration refusal; the target is not the problem.
|
||||
const reason =
|
||||
typeof cause.diagnostics.remediation === "string"
|
||||
? `${cause.message}. ${cause.diagnostics.remediation}.`
|
||||
: `${cause.message}.`;
|
||||
if (cause instanceof ApktoolConfigurationFailure)
|
||||
return err(
|
||||
new AnalysisCapabilityUnavailableError(
|
||||
providerConfigurationRefusal(
|
||||
APKTOOL_PROVIDER_IDENTITY.id,
|
||||
request.operation,
|
||||
reason,
|
||||
{ cause, userMessage: reason },
|
||||
cause,
|
||||
),
|
||||
);
|
||||
}
|
||||
return err(
|
||||
new AnalysisProtocolError(
|
||||
`apktool version probe failed for ${request.operation}`,
|
||||
|
||||
@@ -5,6 +5,7 @@ import type { NativeToolName } from "../contracts/native/nativeToolContracts.js"
|
||||
import type { ArtifactAnalysisOperation } from "../contracts/artifactToolContracts.js";
|
||||
import type { ManagedToolName } from "../contracts/managed/managedToolContracts.js";
|
||||
import type { AnalysisError } from "../domain/analysisErrorBase.js";
|
||||
import { AnalysisCapabilityUnavailableError } from "../domain/analysisErrorCore.js";
|
||||
import type { AnalysisProfileCommitment } from "../domain/analysisProfile.js";
|
||||
import type { JsonValue } from "../domain/jsonValue.js";
|
||||
import type { Result } from "../domain/result.js";
|
||||
@@ -210,6 +211,31 @@ export type ProviderAvailability =
|
||||
readonly diagnostics: Readonly<Record<string, JsonValue>>;
|
||||
};
|
||||
|
||||
/** A host tool configuration failure observed before any target work. */
|
||||
export interface ProviderConfigurationFailure extends Error {
|
||||
readonly diagnostics: Readonly<Record<string, JsonValue>>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Refuse an operation because the host tool is misconfigured. The reason,
|
||||
* not the generic target advice, tells the caller what to change.
|
||||
*/
|
||||
export const providerConfigurationRefusal = (
|
||||
providerId: string,
|
||||
operation: string,
|
||||
failure: ProviderConfigurationFailure,
|
||||
): AnalysisCapabilityUnavailableError => {
|
||||
const remediation = failure.diagnostics.remediation;
|
||||
const reason =
|
||||
typeof remediation === "string"
|
||||
? `${failure.message}. ${remediation}.`
|
||||
: `${failure.message}.`;
|
||||
return new AnalysisCapabilityUnavailableError(providerId, operation, reason, {
|
||||
cause: failure,
|
||||
userMessage: reason,
|
||||
});
|
||||
};
|
||||
|
||||
/** Provider-owned target support observation before profile resolution. */
|
||||
export type ProviderTargetSupport =
|
||||
| {
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { expect, it } from "vitest";
|
||||
|
||||
import { serveCli as serve } from "./cliServeHarness.js";
|
||||
import {
|
||||
expectConfigurationRefusal,
|
||||
serveCli as serve,
|
||||
} from "./cliServeHarness.js";
|
||||
|
||||
it.each([
|
||||
["inspect-adb-client"],
|
||||
@@ -34,13 +37,29 @@ it.each([
|
||||
expect(schema).toBeTruthy();
|
||||
});
|
||||
|
||||
it("reports a missing adb binary with recovery guidance", async () => {
|
||||
const result = await serve(["inspect-adb-client", "--json"], {
|
||||
REA_ADB_PATH: "/nonexistent/adb",
|
||||
});
|
||||
expect(result.stdout).toContain("capability_unavailable");
|
||||
expect(result.stdout).toContain("REA_ADB_PATH");
|
||||
});
|
||||
it.each([
|
||||
[
|
||||
["inspect-adb-client", "--json"],
|
||||
"/nonexistent/adb",
|
||||
"The selected adb binary was not found: /nonexistent/adb. Set REA_ADB_PATH",
|
||||
],
|
||||
[
|
||||
["list-adb-devices", "--json"],
|
||||
"/nonexistent/adb",
|
||||
"The selected adb binary was not found: /nonexistent/adb. Set REA_ADB_PATH",
|
||||
],
|
||||
[
|
||||
["list-adb-devices", "--json"],
|
||||
"relative/adb",
|
||||
"REA_ADB_PATH must be an absolute path to the adb binary.",
|
||||
],
|
||||
])(
|
||||
"%j with REA_ADB_PATH=%s names the host requirement",
|
||||
async (argv, binary, reason) => {
|
||||
const result = await serve(argv, { REA_ADB_PATH: binary });
|
||||
expectConfigurationRefusal(result, reason);
|
||||
},
|
||||
);
|
||||
|
||||
it("validates device serials before contacting any device", async () => {
|
||||
const result = await serve(["inspect-adb-device", "bad serial", "--json"]);
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { expect, it } from "vitest";
|
||||
|
||||
import { serveCli as serve } from "./cliServeHarness.js";
|
||||
import {
|
||||
expectConfigurationRefusal,
|
||||
serveCli as serve,
|
||||
} from "./cliServeHarness.js";
|
||||
|
||||
it.each(["inspect-apktool-client", "decode-android-resources"])(
|
||||
"%s advertises a schema through the public CLI",
|
||||
@@ -32,15 +35,7 @@ it.each([
|
||||
"%j with REA_APKTOOL_COMMAND=%s names the host requirement",
|
||||
async (argv, command, reason) => {
|
||||
const result = await serve(argv, { REA_APKTOOL_COMMAND: command });
|
||||
const output = JSON.parse(result.stdout) as {
|
||||
code?: unknown;
|
||||
message?: unknown;
|
||||
remediation?: { action?: unknown };
|
||||
};
|
||||
expect(output.code).toBe("capability_unavailable");
|
||||
// The target is not the problem; the refusal must say what to configure.
|
||||
expect(output.message).toEqual(expect.stringContaining(reason));
|
||||
expect(output.remediation?.action).toEqual(expect.stringContaining(reason));
|
||||
expectConfigurationRefusal(result, reason);
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import { expect } from "vitest";
|
||||
|
||||
import { createCli } from "../../../src/cli.js";
|
||||
|
||||
/** Captured CLI invocation for boundary tests. */
|
||||
@@ -25,3 +27,18 @@ export const serveCli = async (
|
||||
});
|
||||
return { stdout, exitCode };
|
||||
};
|
||||
|
||||
/** A host tool configuration refusal names what to configure, not the target. */
|
||||
export const expectConfigurationRefusal = (
|
||||
result: ServedCli,
|
||||
reason: string,
|
||||
): void => {
|
||||
const output = JSON.parse(result.stdout) as {
|
||||
code?: unknown;
|
||||
message?: unknown;
|
||||
remediation?: { action?: unknown };
|
||||
};
|
||||
expect(output.code).toBe("capability_unavailable");
|
||||
expect(output.message).toEqual(expect.stringContaining(reason));
|
||||
expect(output.remediation?.action).toEqual(expect.stringContaining(reason));
|
||||
};
|
||||
Reference in new issue
Block a user