6 Commits

Author SHA1 Message Date
inliver233 a1ada96cc3 Lint clean: adopt the PR's type/style gate, apply it to the current tree, and drop its path filter
PR #3 proposed adding mypy and flake8 to CI, with configurations for both. The idea is right and the
configurations are kept. Three things about the PR itself needed correcting, and all three were found
by running the gate rather than reading the diff:

  1. It was written against an older tree. Its own edits reference code that no longer exists there
     (`unknown, positions, warnings = ...`, a `struct` import at line 857 that is used 600 lines
     later), so merging it as-is would have removed a live import. The fixes were re-derived against
     the current tree instead, and the PR is superseded.
  2. It added a `paths:` filter to the workflow, watching only `*.py` plus the two lint configs. That
     would stop `check_refs.py` and `check_routing.py` -- documentation gates -- from running on a
     documentation-only commit, which is exactly the change they exist to catch. The workflow keeps
     the PR's `actions/*@v7` bump and gains no filter; the reason is written next to the trigger.
  3. `pip install --upgrade pip pytest` was dropped to just the packages, because upgrading pip in CI
     is not a goal and can fail for reasons unrelated to this repository.

What the gate found on the current tree, and what was done about it -- 70 flake8 findings and 3 mypy
errors, none of which were cosmetic-only:

  F401 (23)  unused imports, removed. These were safe to delete because the import statement itself
             cannot have a side effect that matters here.
  F841 (12)  unused bindings, handled three ways rather than one. Dead code was deleted
             (`mt_mcp_probe.print_waiting_help`'s `host`, `repack.py`'s `signer_jar`); a computed
             value that was then forgotten was **wired up** (`rasc_build.py` now names the linker
             target in its build-failure hint instead of discarding it); and a binding that is
             intentional kept its place with `# noqa: F841` and the reason -- `java2c_probe` reads
             dex header fields one by one against the format spec, and deleting three of them to
             please a linter would have made the parser stop lining up with the format it implements.
  E741 (14)  the ambiguous loop name `l`, renamed with an AST pass that resolves each binding to its
             own scope. A regex rename would have been wrong: several scopes bind their own `l`.
             That pass had a defect of its own -- it treated a comprehension target as living in the
             comprehension scope while leaving references to it in the enclosing one -- which
             produced seven `undefined name 'l'`. Found by running flake8, fixed, and the counting
             bug is documented in `check_routing.py`, whose own `tables()` had the same class of
             error in the other direction.
  style      continuation indents, blank lines between top-level definitions, and lines over length.
             The line limit is 160, not 120, and the number is a **measurement**: this repository's
             secret-scanner rule table holds raw regex source of 148 and 156 characters, which cannot
             be split without changing the pattern.
  mypy (3)   two missing annotations on module-level containers, and one optional-dependency rebind
             (`dexutil = None` after `except ImportError`) that is the repository's own idiom and
             now says so with a `type: ignore[assignment]` and the reason. No code was reordered to
             satisfy the checker.

Verified on a venv containing only `mypy` and `flake8` -- the poorest environment the configuration
must work in, and the one CI uses: **flake8 reports 0, mypy reports "Success: no issues found in 55
source files"**. Behaviour is unchanged: 625 passed / 163 skipped / 25 xfailed, the same five gates
green (check_repo 0 problem, check_refs 0 dangling, check_routing coherent, check_budget 0 failure,
check_commands 0 drift), and every key script still prints help.

  .flake8        new, with the line-length and complexity numbers and their reasons
  mypy.ini       new; optional imports declared, nothing excluded from the analysis
  ci.yml         `actions/*@v7`; new `lint` job on one interpreter; no path filter
  30 files       the findings above
2026-09-22 19:56:34 +08:00
inliver233 dd8057cfcb Make the command gate machine-independent, and calibrate the budget fallback
CI went red on `1dc1d9b` in both jobs while the identical command passed locally, and the reason is
worth keeping: the checks depended on state that only exists on a developer's machine.

check_commands.py -- the gate's verdict depended on a gitignored directory
  The previous commit's fix recognised a workbench script by matching its name against a walk of
  `tools/`. That tree is gitignored, so a clean checkout has none of it: `dcc.py` matched locally and
  skipped, and on CI nothing matched, it fell through to `missing-script`, and the gate reported
  drift for three quoted lines. A check whose verdict changes with a directory CI does not have is
  not a check. Two changes remove the dependency:

    * `qualified_basenames()` collects every `.py` the *document itself* writes with a path, so a
      bare name the document has already located elsewhere is a skip, computed from the text.
    * `./dcc.py` is a declared upstream entry point, checked before the workbench-name test.
    * a bare name after a `cd` that resolves nowhere is `after-cd-unresolved` -- unverifiable, which
      is not the same claim as broken, and is now reported as such instead of as drift.

  Verified by running the whole CI sequence against a tree with `tools/` hidden: five gates exit 0,
  the suite passes. That experiment is the one that should have been run before the first push.

check_budget.py -- the measurement was installation-dependent
  The gate measures the always-loaded body in tokens, preferring `tiktoken` and falling back to
  `characters / N`. `N` had been guessed at 3.6 rather than calibrated, so a machine with tiktoken
  saw ~11.1k tokens and passed while a machine without it saw ~14.2k and warned -- and CI's 3.11 job
  is the one without it. `N` is now the measured 4.285 characters per token (47,629 chars -> 11,114
  tokens on SKILL.md), both paths agree within one token, and `tests/unit/test_budget_measurement.py`
  forces both paths and asserts they do not straddle the budget.

  CI now installs `tiktoken` so the comparison exercises the real tokenizer instead of skipping.

Also: two `.py` files are formatted for consistency, and the reconstruction record documents the
CI-only failure, since "it passed on my machine" is the lesson rather than an aside.

Gates: check_repo 0 problem, check_refs 0 dangling / 0 warning, check_routing coherent,
check_budget 0 failure / 0 warning (both measurement paths), check_commands 154 checked / 0 drift,
pytest 617 passed on 3.14 and 609 on 3.11.
2026-09-22 17:09:22 +08:00
inliver233 fe5bdee087 Merge: keep the maintenance pass and the reconstruction pass, both
Two passes met on this branch and neither is discarded. `d0fe0ea` (inliver233) hardened
`build_scripts.py` -- 37 findings that were mostly noise, down to 0, with two defects found in its
own first implementation by an 18-case table -- generalised three absolute paths out of the evidence
record, and added a corpus-size note to `check_budget.py`. `2beade6` (this pass) rewrote
`check_budget.py`'s measurement, added four gates, rewrote `so_constpatch.py`'s zip writer,
replaced 59 out-of-package references, and shrank the always-loaded body.

Resolution, file by file:

  check_budget.py   the only real conflict. Their corpus note and this pass's rewritten measurement
                    are kept as separate functions and separate notes, because they measure different
                    quantities: what a task loads versus what the repository carries. Collapsing them
                    would let a growing corpus hide behind a green always-loaded count -- which is
                    the defect their note exists to prevent.
  build_scripts.py  theirs, taken as-is, including the case table.
  REPO-DECISIONS.md both sections: their maintenance-pass record follows this pass's decision record.
  three extensions  their absolute-path generalisations, on top of this pass's convergence edits.
  everything else   this pass's versions, which supersede (SKILL.md, the routing file, tests/, the
                    skill's evidence/ and references/).

One regression this merge introduced and this commit fixes: `EXTENSION-test-harness.md` carried a
device serial in a quoted `adb devices` transcript. It is `<DEVICE>` now, and `check_repo.py`'s
content scan -- the gate added in the same pass -- is what caught it.

Gates on the merged tree: check_repo 0 problem, check_refs 0 dangling / 0 warning, check_routing
coherent, check_budget 0 failure / 0 warning (6 notes, including both corpus thresholds),
check_commands 146 checked / 0 drift, pytest 608 passed / 158 skipped / 25 xfailed.
2026-09-22 01:43:07 +08:00
inliver233 2beade67f2 Reconstruction pass: make the gates tell the truth, make the package self-contained, keep the one-hop symptom index
An external audit of this repository landed on four structural findings, and this pass is the
response to them. Everything below was re-measured on this tree before being acted on; two of the
audit's claims needed a correction of my own, and both corrections are noted in place.

**The gates were optimistic, and one of them did not read content at all.**

  doctor.py    the capability table was hard-coded (`repack + sign = bool(java) and python`), so a
               machine with a JDK and no zipalign/apksigner was told it could re-sign; the script
               inventory was a hand-written list of 28 against 51 files on disk, omitting the nine
               most important scripts. It now scans the directory (`registered/checked = 52 / 52`,
               unjudgeable entries counted as `unknown`, never skipped) and resolves capability as a
               real requires-closure, reporting BLOCKED with the missing piece and an installable
               next step. Measured: the old doctor reports `[OK] repack + sign` on a JDK-only PATH
               where keytool, jarsigner, zipalign, apksigner and all eight smali jars are absent.
               The report is allowed to look bad; that is the point.
  check_commands.py (new)
               anchor checks prove a section exists, never that a command runs. This parses each
               script's argparse table with `ast` and checks every documented invocation against it.
               146 commands / 41 interfaces, 0 drift. It caught `sig_probe.py --live --pkg`, which
               does not exist -- the reference had been wrong since it was written.
  check_routing.py (new)
               the symptom index now has one home (`SKILL.md`, because symptom -> file must stay one
               hop) and the inventory has another (`references/routing.md`). This gate proves the
               mirror agrees, that every reference file is named, and that every script is named.

**so_constpatch.py was corrupting APKs it rebuilt.** The writer copied filename and timestamp onto a
fresh ZipInfo and recompressed the whole archive: 435-entry real package, `extra` lost on 205 entries,
`external_attr` rewritten on all 435, 132 untouched entries recompressed, and `resources.arsc` pushed
to an unaligned offset. Android's own `zipalign -c -v 4` fails the old output and passes the new one;
a 26-case fixture suite pins it, and `extractNativeLibs="false"` is now refused by default with a
pointer at `repack.py`. Fixing it exposed the same defect in `repack.py`, where `^lib/[^/]+\.so$`
cannot match `lib/arm64-v8a/*.so` -- so its alignment handling had never once fired on a real APK.

**The package was not self-contained.** 59 references pointed at the repository root, which `npx
skills add` does not install. They now converge on `references/evidence-summary.md` (capability,
conclusion, strength, evidence that ships) plus `evidence/` with the machine-readable companions: 8
references remain and each is explicitly labelled "repository root, not shipped". A drill in which
the new files were copied to an isolated directory and asked three questions answered all three.

**SKILL.md: 572 lines / 15,733 tokens -> 398 lines / ~10,900 tokens**, without losing the two things
that had to stay. The symptom index remains inline; the reference and script indexes moved to the
on-demand routing file. The freed budget went into making the rules *stronger*, not shorter: the
constraints section is now a table of thirteen non-negotiable constraints each paired with the check
that catches a violation, and the tooling obligations are instructions whose violation is a defect
(hedging language in SKILL.md: 1 line, down from a paragraph of hedged advice). Long references gained
a navigation head so a reader can decide from the top whether to load one.

**New: an executable test layer.** `tests/` (unit, CLI contract, no-device integration) plus
`pytest.ini`, `tests/run_tests.py`, a minimal CI job, and `evals/evals.json` written but never run --
stated as such in the file. 608 passed / 158 skipped / 25 xfailed. The 25 xfails are a work order, not
a pass: thirteen scripts report a missing input by exiting 1 instead of 2, seven let a traceback
escape, four treat `--help` as an error, and `preflight --json` lacks a `status` field. A regression
row for `dex_patch_bytes.py` reproduces a previous artifact byte-for-byte
(`67ae1e58e7a44777e5ac8f9f823ab21e…`) and additionally pins the four bytes that may move.

**Two real defects the new tests found in the tooling itself.** `check_repo.py` decoded subprocess
output with the locale encoding, so on a GBK console a script printing an em dash in its usage line
was reported as crashed; it now decodes UTF-8 explicitly. And a CLI contract test drove a *real
attached device* through `coldstart.py` -- logcat, screenshots, five orphan `adb logcat` processes --
which is fixed twice over: child processes run in a private temporary cwd, and the fifteen
device-bound scripts are never executed beyond `--help`/argument parsing.

**Quoting on the device side is now one implementation.** `devsh.su` escaped only double quotes, so
`;`, `$( )` and backticks in a caller-supplied value still reached the device shell while
`coldstart.Device.shell` quoted correctly. Both use `scripts/device_shell.py` now: POSIX single-quote
escaping, argv arrays by default, and identifier validation that refuses a malformed package, path or
component with exit 2 instead of a traceback. The integration test that used to pin the *defect* (it
asserted `su -c "…"` with `\"` escapes) now asserts the property, across five metacharacter payloads.

Gates: check_repo 0 problems (including the content-reading identity scan over git ls-files),
check_refs 0 dangling / 0 warning, check_routing coherent (53 symptom / 45 reference / 54 script rows),
check_budget 0 failure / 0 warning with the always-loaded body now measured in tokens rather than in
"narrative lines with the index excluded", check_commands 146 checked / 0 drift, pytest 608 passed.

Not done, and stated rather than implied: CI has never run on a runner; the evals have never been
executed; the 25 xfails are open; no device-side install/launch was performed in this pass.
2026-09-22 01:37:51 +08:00
inliver233 d0fe0ea337 Harden the path audit, generalise three absolute paths, and gate the corpus size
An external review found the evidence record honest and the gates real, but raised three
concrete defects. Each is fixed at its cause rather than at its symptom, and the reasoning
is recorded in docs/tool-verification/REPO-DECISIONS.md.

build_scripts.py -- the audit reported 37 findings, most of them noise
  It matched any `X:\...` literal, so `C:\Program Files\...`, `C:\Python314\...` and literal
  placeholders all counted as machine-specific leftovers. A gate whose output is mostly noise
  stops being read -- the same failure scan_leaks.py names when it refuses to exempt the
  RFC 5737 ranges. Two explicit exemption families are added instead of a looser regex:
  GENERIC_PATH_PREFIXES (install locations that exist on every host of that platform) and
  `_leaks_nothing()` (also exempts an already-redacted path, since a placeholder or an
  ellipsis means a normalisation pass already ran).

  Two traps surfaced only because the change was tested rather than merely run. BARE_ABS stops
  at whitespace, so `C:\Program Files\...` arrives truncated as `C:\Program` and a one-way
  `startswith` misses the exemption; the comparison is now bidirectional under a minimum
  length. And with a lookahead of only `[a-z0-9]` the optional `\\?` backtracks to empty, the
  next character is the separator, it passes, and a real personal path is swallowed -- the
  lookahead has to exclude the separator as well. Both were caught by the case table, not by
  the audit run.

  Result: 37 -> 0 findings, exit 2 -> exit 0. The real hits (`E:\apk-reverse\...`,
  `E:\agb_apktool.jar`) were fixed in the documents, not by widening the list.

Three absolute paths on the published surface
  EXTENSION-desensitization.md, EXTENSION-extraction-shell-bench.md and EXTENSION-vmp-diff.md
  quoted a workbench root, a tool jar and an SDK location absolutely.
  docs/tool-verification/ already states the rule -- a record may say a run happened at a
  `tools/...` path, written relatively -- so the three files were made to match the rule they
  were already under. The tool-jar and SDK cases are rephrased rather than placeholder-ised,
  so no reader has to learn a new token.

check_budget.py -- blind to the half of the problem it was not measuring
  The gate budgets the always-loaded part and could not see the corpus, where a new reference
  costs exactly one index line: the corpus can double while the narrative count stays green.
  Two note-level thresholds now report it. Notes, not failures -- a large corpus is a choice,
  a growth nobody measured is the defect. The first run reports 47 references + 55 scripts =
  102 files, 31,423 lines.

A cross-tool inconsistency this pass ran into, recorded rather than papered over
  build_scripts.py now exempts any `<...>` placeholder; scan_leaks.py recognises a fixed token
  list. An invented placeholder in a source comment passed the first and was read by the
  second as the very personal name its `user_path_windows` rule exists to catch, turning
  check_repo.py red. That is by design -- a strict list is what stops a real name hiding
  behind a plausible-looking placeholder -- but the two tools disagree about what "already
  redacted" means, and the disagreement stays invisible until someone writes a new token.
  Rephrased here; a future pass may want one shared list.

Verification
  - build_scripts.py: 37 -> 0 findings on a clean clone, exit 0.
  - An 18-case table pins both directions, 0 failures: generic and already-redacted paths are
    exempt, while a real workbench path, a real personal name after `C:\Users\`, a real POSIX
    home and a real project path still report. It found two defects in the first
    implementation.
  - All four gates pass: check_repo 0 problem, check_refs 0 dangling / 0 warning,
    check_budget 0 failure / 0 warning / 3 notes, build_scripts 0 findings.
2026-09-21 23:18:17 +08:00
inliver233 fd2a1150be Structure: sink the evidence record out of SKILL.md, and gate the budget
SKILL.md was 658 lines against the Anthropic Agent Skills guidance of keeping the body under 500, and the growth sat where it costs most: the Coverage section had accumulated the evidence for every claim plus a per-pass verification history (153 lines / 11.8 KB), and Hand-off had grown two knowledge tables. Both now load on demand, with SKILL.md keeping the decision-level statement and a pointer.

  Coverage   153 -> 68 lines   new references/coverage-and-limits.md
  Hand-off    51 -> 19 lines   new references/handoff-boundaries.md
  SKILL.md   658 -> 543 lines  (404 narrative + 139 index)
  index rows 24.0 KB -> ~19 KB  descriptions now say when to load, not what is inside

check_budget.py is the substance of this pass. check_repo.py proves consistency, and consistency has a natural counter-pressure: a broken path fails loudly and someone fixes it. Bloat has none -- every pass adds a reference, an index row and a coverage claim, and nothing in the repository notices. The new gate measures narrative lines separately from index lines, because 139 of SKILL.md's lines are the discoverability tax for 86 bundled files and counting them against a prose budget would push the skill to drop files it needs. Conclusions restated beyond the benchmark matrix, the evidence record and the coverage statement are reported as notes.

No measured content changed in this pass: every claim, strength label and negative result is where it was, and the two new references carry the moved text verbatim plus the corrections the benchmark pass produced.
2026-09-21 22:12:58 +08:00