Skip to content

feat(heal): scripts/heal.py becomes a heal-service client - #22

Merged
philmcneely merged 8 commits into
mainfrom
feat/heal-service-client
Nov 7, 2026
Merged

philmcneely merged 8 commits into
mainfrom
feat/heal-service-client

Conversation

@philmcneely

Copy link
Copy Markdown
Owner

scripts/heal.py becomes a client of the heal service (spec: docs/HEAL_SERVICE_SPEC.md, in a separate PR). The framework no longer holds a model URL, a model name, a prompt or any parsing of model replies; a unit test checks that none of them come back.

Do not merge before the heal service's phase 1 is merged. The service is not public yet, so the integration test skips in CI with a stated reason.

What changes

  • scripts/heal.py: same flags (-k, --max, --open-pr). It keeps the JUnit parse, the exact-test re-run, revert and the PR flow.
    • It now prints decisions with token usage and a run total, and writes heal-summary.json.
    • A service error or a failure past --max becomes a recorded needs_human decision instead of a crash.
  • utils/heal_client.py: builds the request (stack files under the source roots, capped, with sha256), probes the reopened failing route, sends the language signals, and runs the try → result loop.
  • utils/heal_fingerprints.py: a pytest plugin, off unless HEAL_FINGERPRINTS=on.
    • It records what each locator hit on a clean green run into .heal/fingerprints.json and writes .heal/.gitignore itself.
    • HEAL_FINGERPRINT_COPY=off stores hashes of page copy instead of the copy.
  • utils/heal_locale.py: by_language, for healed copy-based locators on a page in another language.
  • utils/heal_probe.js: the in-page probe, vendored and byte-identical to the service's copy.

Treating the service as untrusted

Every patch the service returns is checked before it touches a file. The client applies it only if all of these hold:

  • the file's hash matches the base hash;
  • the file sits under the source roots by real path, with no symlinks;
  • the span holds exactly the old expression, and that expression is one locator chain in the file on disk;
  • the new expression is one validated locator expression, with no comment and no control or bidi characters;
  • it equals the code the client just checked on the live page (count 1, the expected element), and every other file's patch is checked the same way.

Other safeguards:

  • Writes: they are reverted on any failure, and file modes are kept.
  • Probe: the client runs its vendored copy and blocks other origins while it runs.
  • No input values: they are never sent. Aria snapshots are redacted, and inventory keys go through an allowlist.
  • Token over plain http: a bearer token over http:// to a non-localhost host is refused unless HEAL_ALLOW_INSECURE=1.
  • PR body: text is markdown-escaped.

A security review of this branch found five HIGH issues. All are fixed, each with a test written first (tests/unit/test_heal_client_security.py).

Tests

  • pytest tests/unit: 292 passed.
  • ruff check .: clean.
  • The integration test runs the real service in process against Chromium, including a French page, when heal-service is installed.

Returns the locator of the page's language, and the first entry (the original copy)
for any other language. The page fixture records the language on async pages.
…an green run

Off unless HEAL_FINGERPRINTS=on. Writes .heal/fingerprints.json atomically with an
automatic .heal/.gitignore (*); committing it is an opt-in. Only clean green tests
(passed once, not flaky, not during a heal) are recorded; never input values.
Adds utils/heal_client.py (request building, live count/element check, patch safety,
try->result loop) and removes the direct model call, its prompt and the HEAL_BASE_URL /
HEAL_MODEL / HEAL_API_KEY settings. A service error becomes a needs_human decision,
failures past --max are recorded, decisions print with token usage, and
heal-summary.json is written to the test artifacts directory.
Redact typed values and allowlist inventory keys before anything is sent; reject comments and control characters in
replacements; require the original span to be a locator chain that is exactly one AST node; write only the locator that was
checked live (also-patches included); vendor the probe and block foreign origins while it runs; revert partial writes, keep
file modes, never write through symlinks; refuse a bearer token over http to a remote host; optional hashed fingerprint
copy; escape markdown in PR bodies; validate HEAL_DIR and HEAL_SOURCE_ROOTS; detect the helper import by AST.
Copilot AI balanced review requested due to automatic review settings November 6, 2026 02:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

✅ All smoke tests passed! View report

@philmcneely

Copy link
Copy Markdown
Owner Author

No changes needed: the only feedback on this PR is the github-actions smoke-pass notice (lint, unit, smoke, setup and semgrep all green) and a Copilot review that didn't run because of quota. There's no review comment to act on, so I made no commit. The full suite is still pending.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

✅ All chromium login tests passed! View report

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

✅ All firefox login tests passed! View report

@philmcneely
philmcneely requested a balanced review from Copilot November 7, 2026 04:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@philmcneely
philmcneely merged commit 2b63256 into main Nov 7, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants