diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9fdefab..1dbe615 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,9 +28,11 @@ jobs: run: npm run build:types - name: Adopter-visible declarations # Checks a PROPERTY of the whole shipped surface (no bare `object` returns; - # the two platform pages agree on the methods they share), rather than the - # handful of methods a hand-written quickstart happens to call. See the - # header of scripts/check-adopter-types.mjs for what it cannot catch. + # the two platform pages agree on the methods they share; and every page + # member squares with its own source — what it forwards to, and what its + # body does when an argument is missing), rather than the handful of + # methods a hand-written quickstart happens to call. See the header of + # scripts/check-adopter-types.mjs for what it still cannot catch. run: npm run check:types - name: Test run: npm test diff --git a/CHANGELOG.md b/CHANGELOG.md index ef9c543..1904328 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,15 @@ # Changelog -## [Unreleased] +## 0.11.2 — 2026-08-31 ### Added -- **CI now checks a property of the whole adopter-visible surface, not the handful of methods a quickstart happens to call.** The publish workflow's adopter compile gate is necessary but not sufficient: it only exercises what its hand-written quickstart touches, which is precisely how v0.11.1's `page.swipe()` defect reached the registry — the quickstart calls `tap`/`type`/`snapshot`/`screenshot`/`findByText` and never calls `swipe`. Enumerating every method by hand does not scale and rots as methods are added. `scripts/check-adopter-types.mjs` (TypeScript compiler API, no new dependency) instead asserts two properties of the shipped `.d.ts`: no exported function returns a bare `object`, and the Android and iOS pages agree on the required-parameter count and declared return type of every method they both have. A method added tomorrow is covered without anyone remembering to extend a script. Wired into `ci.yml` (every push/PR) and `publish.yml` (before the tarball is packed); exits 1 on a defect and 2 if `types/` was never built, so a misconfigured step cannot look like a pass. Validated against real history rather than a fixture written to pass: it fires on `c006cac` (bare `object` on `connect()`/`translateWda()`, 4 findings), on `c4d60ce` (`page.swipe()`, 5 required vs 4), and on HEAD with the `screenshot()` fix reverted (`Promise` vs `Promise`) — and reports zero on the shipped tree. Known gap, stated in the script header: platform-parity compares the two pages against each other, so a declaration wrong on both platforms in the same way is invisible to it. CI only — no runtime or published-artifact change. +- **CI now checks a property of the whole adopter-visible surface, not the handful of methods a quickstart happens to call.** The publish workflow's adopter compile gate is necessary but not sufficient: it only exercises what its hand-written quickstart touches, which is precisely how v0.11.1's `page.swipe()` defect reached the registry — the quickstart calls `tap`/`type`/`snapshot`/`screenshot`/`findByText` and never calls `swipe`. Enumerating every method by hand does not scale and rots as methods are added. `scripts/check-adopter-types.mjs` (TypeScript compiler API, no new dependency) instead asserts two properties of the shipped `.d.ts`: no exported function returns a bare `object`, and the Android and iOS pages agree on the required-parameter count and declared return type of every method they both have. A method added tomorrow is covered without anyone remembering to extend a script. Wired into `ci.yml` (every push/PR) and `publish.yml` (before the tarball is packed); exits 1 on a defect and 2 if `types/` was never built, so a misconfigured step cannot look like a pass. Validated against real history rather than a fixture written to pass: it fires on `c006cac` (bare `object` on `connect()`/`translateWda()`, 3 findings), on `c4d60ce` (`page.swipe()`, 5 required vs 4), and on HEAD with the `screenshot()` fix reverted (`Promise` vs `Promise`) — and reports zero on the shipped tree. CI only — no runtime or published-artifact change. +- **The same gate now checks every page member against its own source, not against the other platform's copy.** Comparing Android's copy of a method with iOS's can only find *disagreement*, which left two blind spots: a declaration wrong the same way on both platforms had nothing to contradict it, and the 13 of 32 page members that exist on one platform only (`intent`, `grantPermission`, `tapGrid`, … on Android; `unlock`, `lock`, `activate`, `baseUrl`, … on iOS) were never compared at all — 19 of 32 members carried any coverage. Two rules read `src/` instead of the built `.d.ts`, because the evidence lives in the function body a `.d.ts` drops: `forwarded-default` flags a parameter handed straight to another of our functions at a position that function defaults but restated without a default here, and `optional-in-fact` flags a required parameter whose every use sits on the left of `||`/`??`, where the body already handles its absence. Both judge a member against its own implementation, so they give a notion of *wrong* rather than only *these two disagree*, and single-platform members are covered. Validated on real defects, not fixtures: `forwarded-default` fires on `c4d60ce`'s `page.swipe()` from source alone; with that same default removed on *both* pages the original two rules report "OK" while `forwarded-default` still fires — the first blind spot, demonstrated; and `optional-in-fact` fired on shipped `HEAD` against iOS-only `page.unlock()`, a live defect no parity check could see (fixed below). Parameters are resolved by symbol rather than name text, so a nested closure shadowing a parameter cannot misattribute a finding, and shorthand property uses (`post({ pin })`) count as real uses; an argument after a spread is skipped, since its callee position is unknowable. Residual gaps are stated in the script header rather than left implicit: this is absolute for parameter *optionality*, not for return types, and `optional-in-fact` would misread a fallback that itself throws. CI only — no runtime or published-artifact change. + +### Fixed + +- **`page.unlock()` on iOS forced adopters to pass a passcode `connect()` already held.** The wrapper was written `async unlock(pin)` with no default, so the generated declaration was `unlock(pin: any)` — required — even though the body reads `pin || passcode` and falls back to the passcode given to `connect({ passcode })`. A TypeScript adopter calling the idiomatic `page.unlock()` therefore got `TS2554: Expected 1 arguments, but got 0`, and `any` gave them no type either. Now declared `unlock(pin?: string | null)` via JSDoc. Found by the new `optional-in-fact` rule on the shipped tree, not by hand — and invisible to the parity rule, since `unlock` is iOS-only and has no Android counterpart to disagree with. Declaration-only: the change is a JSDoc comment, no runtime code, verified against a packed tarball — a consumer calling `page.unlock()` gets `TS2554` before and compiles clean after. 321 tests unchanged and green. ## 0.11.1 — 2026-08-30 diff --git a/package-lock.json b/package-lock.json index bccc95e..3c784e2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "baremobile", - "version": "0.11.1", + "version": "0.11.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "baremobile", - "version": "0.11.1", + "version": "0.11.2", "license": "Apache-2.0", "bin": { "baremobile": "cli.js" diff --git a/package.json b/package.json index 63473ac..558cc21 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "baremobile", - "version": "0.11.1", + "version": "0.11.2", "type": "module", "description": "ADB-direct Android control for autonomous agents. Accessibility tree in, pruned snapshot out.", "main": "src/index.js", diff --git a/scripts/check-adopter-types.mjs b/scripts/check-adopter-types.mjs index e1cae0e..ec9e61f 100644 --- a/scripts/check-adopter-types.mjs +++ b/scripts/check-adopter-types.mjs @@ -13,16 +13,40 @@ // Exits non-zero with a per-finding explanation. // // Validated against real history, not a fixture built to pass: -// c006cac (pre-v0.11.0) -> bare-object fires 4x on connect()/translateWda() +// c006cac (pre-v0.11.0) -> bare-object fires 3x on connect()/translateWda() // c4d60ce (pre-v0.11.1) -> platform-parity fires on page.swipe(), 5 vs 4 // HEAD with the screenshot fix reverted -> platform-parity fires on the return // HEAD as shipped -> 0 findings // -// KNOWN GAP, stated rather than papered over: platform-parity compares the two -// pages against EACH OTHER, so a declaration that is wrong on BOTH platforms in -// the same way is invisible to it. It catches divergence, not agreed-upon error. +// platform-parity alone was a differential check: it compared the two pages +// against EACH OTHER, so it was blind to (a) a declaration wrong the same way on +// both platforms and (b) the 13 of 32 page members that exist on one platform +// only and therefore have no counterpart to disagree with. Rules 3 and 4 below +// fix that by checking each page member against its own source — the function it +// forwards to, and what its own body does with the argument — which gives a +// notion of "wrong" rather than only "these two disagree". +// +// Rules 3/4 validated the same way: +// c4d60ce (pre-v0.11.1) -> forwarded-default fires on page.swipe(), from source +// alone, with no cross-platform comparison in play +// HEAD with the swipe default removed on BOTH pages -> rules 1+2 report "OK", +// forwarded-default still fires (the exact blind spot above) +// HEAD as shipped -> optional-in-fact fired on iOS-only page.unlock(pin), a real +// defect no parity check could ever see; fixed in the same commit +// +// RESIDUAL GAP: rule 4 reads `x || y` as "the body handles x being absent". It +// would misread a fallback that itself throws (`pin || required()`) as a default. +// No such code exists in either page today; left unguarded deliberately, because +// the failure mode is a visible false positive a human dismisses, not a wrong +// type shipped to an adopter. +// +// RESIDUAL GAP: rules 3/4 give an absolute notion of correct for parameter +// optionality — the defect class that actually shipped twice. A return type that +// is wrong identically on both platforms is still only caught where `tsc` itself +// catches it (a JSDoc @returns that contradicts the returned expression); a lie +// laundered through a cast, as `screenshot()` once was, is not detected here. import ts from 'typescript'; -import { resolve } from 'node:path'; +import { resolve, sep } from 'node:path'; import { existsSync } from 'node:fs'; const typesDir = resolve(process.argv[2] ?? 'types'); @@ -143,6 +167,154 @@ if (android && ios) { } } +// ---- RULES 3 & 4: every page member, checked against its own source ----- +// Rules 1 and 2 read the built .d.ts. These two read `src/` instead, because +// the evidence they need — what a wrapper forwards to, and what its body does +// when an argument is missing — exists only in the body, which a .d.ts drops. +const srcDir = resolve(typesDir, '..', 'src'); +const srcFiles = ['index.js', 'ios.js'].map(f => resolve(srcDir, f)); +const srcProgram = ts.createProgram(srcFiles, { + target: ts.ScriptTarget.ES2022, + module: ts.ModuleKind.NodeNext, + moduleResolution: ts.ModuleResolutionKind.NodeNext, + allowJs: true, checkJs: true, strict: true, skipLibCheck: true, noEmit: true, +}); +const srcChecker = srcProgram.getTypeChecker(); + +// The page object is the literal carrying a `platform` key — the one thing both +// pages declare and nothing else in these files does. +const pageLiterals = []; +for (const file of srcFiles) { + const sf = srcProgram.getSourceFile(file); + if (!sf) { console.error(`missing ${file}`); process.exit(2); } + const visit = (n) => { + if (ts.isObjectLiteralExpression(n) && + n.properties.some(p => p.name && ts.isIdentifier(p.name) && p.name.text === 'platform')) { + pageLiterals.push({ file, node: n }); + } + ts.forEachChild(n, visit); + }; + visit(sf); +} +if (pageLiterals.length !== 2) { + console.error(`expected 2 page literals (one per platform), found ${pageLiterals.length} — ` + + `the anchor for rules 3/4 has moved; fix the checker rather than the count.`); + process.exit(2); +} + +// Optional three ways: `p?`, `p = x`, `...rest` — or, in JS source, a bracketed +// JSDoc tag (`@param {T} [p]`), which is how these files say it. +const optionalDecl = (d) => !!d && ( + !!d.questionToken || !!d.initializer || !!d.dotDotDotToken || + ts.getJSDocParameterTags(d).some(t => t.isBracketed)); +let members = 0, covered = 0; + +for (const { file, node } of pageLiterals) { + const short = file.split('/').pop(); + for (const prop of node.properties) { + if (!prop.name || !ts.isIdentifier(prop.name)) continue; + const name = prop.name.text; + members++; + const fn = ts.isMethodDeclaration(prop) ? prop + : (ts.isPropertyAssignment(prop) && + (ts.isArrowFunction(prop.initializer) || ts.isFunctionExpression(prop.initializer))) + ? prop.initializer : null; + if (!fn?.body || !fn.parameters.length) continue; + covered++; + + // Key parameters by SYMBOL, not by name text. A nested closure is free to + // shadow a parameter name (`list.forEach(duration => ...)`), and a text + // match would blame the outer parameter for what the inner one does — + // verified: it produced a false positive on rule 3 and masked a real + // finding on rule 4 before this was symbol-based. + const paramOf = new Map(); + for (const p of fn.parameters) { + if (!ts.isIdentifier(p.name)) continue; + const sym = srcChecker.getSymbolAtLocation(p.name); + if (sym) paramOf.set(sym, p); + } + + // RULE 3 — forwarded-default: a parameter handed straight to another of our + // functions, at a position that function defaults, but declared required + // here. This is exactly the v0.11.1 `swipe()` defect, caught without + // reference to the other platform. + const walk = (n) => { + if (ts.isCallExpression(n)) { + const sig = srcChecker.getResolvedSignature(n); + if (sig) { + n.arguments.forEach((arg, i) => { + if (!ts.isIdentifier(arg)) return; + // A spread earlier in the call contributes an unknown number of + // arguments, so this argument's syntactic index is no longer the + // callee's parameter index. Nothing can be concluded — say so by + // skipping, rather than comparing against the wrong parameter. + if (n.arguments.some((a, j) => j < i && ts.isSpreadElement(a))) return; + const wp = paramOf.get(srcChecker.getSymbolAtLocation(arg)); + if (!wp || optionalDecl(wp)) return; + const cp = sig.getParameters()[i]; + const cd = cp && (cp.valueDeclaration ?? cp.declarations?.[0]); + if (!cd || !ts.isParameter(cd) || !optionalDecl(cd)) return; + // Only functions we own. Forwarding into a builtin (`Number(ref)`) + // says nothing about our contract — measured: that exclusion is + // what took this rule from 1 false positive to 0 on the real tree. + if (!cd.getSourceFile().fileName.startsWith(srcDir + sep)) return; + findings.push({ + rule: 'forwarded-default', + where: `page.${name}() in ${short}`, + detail: `parameter \`${arg.text}\` is passed to \`${n.expression.getText()}\` at ` + + `position ${i}, which defaults it (\`${cd.getText()}\`) — but the wrapper ` + + `restates it without a default, so adopters are forced to supply an ` + + `argument the implementation would have filled in. Give the wrapper the ` + + `same default.`, + }); + }); + } + } + ts.forEachChild(n, walk); + }; + walk(fn.body); + + // RULE 4 — optional-in-fact: a required parameter whose every use in the + // body sits on the left of `||` / `??`. The body already says what happens + // when it is absent; the declaration should say it may be absent. + for (const [sym, wp] of paramOf) { + if (optionalDecl(wp)) continue; + // Carry the parent down the walk instead of reading `node.parent`. Parent + // pointers exist here only as a side effect of having built the type + // checker; depending on that would break silently if this ever ran before + // the checker was created. + // A shorthand property (`post({ pin })`) is a real use of the parameter, + // but `getSymbolAtLocation` there resolves to the PROPERTY, not the value. + // Missing it would read a genuinely-required parameter as fallback-only — + // verified: this exact probe produced a false positive before the switch. + const symOf = (id, parent) => + parent && ts.isShorthandPropertyAssignment(parent) + ? srcChecker.getShorthandAssignmentValueSymbol(parent) + : srcChecker.getSymbolAtLocation(id); + const uses = []; + const collect = (n, parent) => { + if (ts.isIdentifier(n) && symOf(n, parent) === sym) uses.push({ n, parent }); + ts.forEachChild(n, (c) => collect(c, n)); + }; + collect(fn.body, undefined); + if (!uses.length) continue; + const fallbackOnly = uses.every(({ n, parent }) => + parent && ts.isBinaryExpression(parent) && parent.left === n && + (parent.operatorToken.kind === ts.SyntaxKind.BarBarToken || + parent.operatorToken.kind === ts.SyntaxKind.QuestionQuestionToken)); + if (fallbackOnly) { + findings.push({ + rule: 'optional-in-fact', + where: `page.${name}() in ${short}`, + detail: `parameter \`${wp.name.text}\` is used only as \`${uses[0].parent.getText().split('\n')[0]}\` — ` + + `the body already defines the behaviour when it is absent, yet the declaration ` + + `forces every adopter to pass it. Mark it optional (\`@param {T} [${wp.name.text}]\`).`, + }); + } + } + } +} + for (const f of findings) { console.log(`${f.rule}: ${f.where}\n ${f.detail}\n`); } @@ -151,4 +323,6 @@ if (findings.length) { console.log(`${findings.length} adopter-visible declaration defect(s) — see above.`); process.exit(1); } -console.log('adopter-visible declarations OK (bare-object, platform-parity).'); +console.log(`adopter-visible declarations OK (bare-object, platform-parity, ` + + `forwarded-default, optional-in-fact) — ${covered} of ${members} page members ` + + `carry parameters and were checked against their own source.`); diff --git a/src/ios.js b/src/ios.js index ee76c3b..cd581bd 100644 --- a/src/ios.js +++ b/src/ios.js @@ -573,6 +573,10 @@ export async function connect(opts = {}) { await wdaPost('/wda/homescreen'); }, + /** + * Unlock the device, entering a passcode if iOS asks for one. + * @param {string|null} [pin] - passcode to use; defaults to the one given to connect() + */ async unlock(pin) { const locked = await wdaGet('/wda/locked'); if (!locked.value) return;