feat: behavior-vs-drift guard + anchor-aware heals + signed releases (v0.1.3) - #6
Conversation
…(v0.1.3) Moat features: 1. Behavior-vs-drift guard — a failing assertion means the app's behavior changed, not that a selector moved. 9lives no longer routes assertion failures to a Tier 2 rewrite (which would force the test green and hide a real bug); it flags them as a possible real bug and hands off to a human. Opt back in with NINELIVES_HEAL_ASSERTIONS=1. 2. Anchor-aware Tier 1 — try re-identifying anchors in stability order (testid > id > aria-label > text > class) and record WHICH anchor re-found the element (metadata.anchor + 'Re-found via <anchor>' in the report). 3. Footer growth CTA — sharper self-healing pitch in the PR-comment footer (still no client telemetry; install analytics come from the 9lives.run edge). 4. Trust primitives in release.yml — PyPI PEP 740 attestations, Sigstore-signed wheel+sdist attached to the GitHub release, and a best-effort CycloneDX SBOM. Bump to 0.1.3. 4 new tests (guard default + opt-in, anchor recording, stable- anchor preference); suite 32 green, ruff clean. Verified end-to-end: an assertion failure is flagged 'possible real bug' and the spec is left untouched; selector drift still heals via Tier 1.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Sigilix OverviewEffort: 4/5 (large) Quality gates
Summary — latest pushIntroduces a behavior-vs-drift guard that routes assertion failures to a human by default instead of auto-rewriting them, adds anchor-aware Tier 1 healing that re-identifies elements using a stability-ordered priority (testid > id > aria-label > text > class), and hardens the release pipeline with PEP 740 attestations, Sigstore signing, and CycloneDX SBOM generation. The specialist review flagged two P1 logic defects in the new guard implementation and a testing gap that must be resolved before merge. Important files
Sequence diagramsequenceDiagram
participant Test as Test Runner
participant Strategy as HealingStrategy
participant CLI as CLI Heal Loop
participant Tier1 as Tier1 Healer
participant Human as Human Reviewer
Test->>Strategy: select_strategy(failure)
alt Failure is ASSERTION_FAILED
Strategy->>CLI: TIER3_HUMAN (default) or TIER2_AI_SUGGEST (opt-in)
CLI->>Human: Flag 'possible real bug'
else Failure is LOCATOR_NOT_FOUND
Strategy->>Tier1: TIER1_AUTO
Tier1->>Tier1: Re-find via anchor priority
Tier1-->>CLI: HealingResult with anchor metadata
end
Confidence: 2/5Two P1 logic defects in the new assertion guard implementation cause a mismatch between the strategy selection and the CLI's conditional messaging, risking silent misrouting of failures.
Suggested labels:
|
![sigilix[bot]](https://avatars.githubusercontent.com/in/3558711?s=60&v=4){kind=small}
| - name: GitHub release (artifacts + signatures + SBOM) | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: gh release create "$GITHUB_REF_NAME" dist/* --generate-notes |
There was a problem hiding this comment.
SBOM failure blocks release despite 'best-effort' comment
The GitHub release step references the literal path 9lives.cdx.json, but that file is only created if the preceding SBOM step succeeds. Because shopt -s nullglob only affects glob expansions, a missing SBOM file causes gh release create to fail, contradicting the SBOM step's continue-on-error: true and the 'best-effort; never blocks a release' comment. Change the literal path to a glob so nullglob can skip it when the SBOM step fails.
Example:
SBOM step fails (e.g., cyclonedx-bom install error).
Release step runs: gh release create v0.1.3 --generate-notes dist/*.whl dist/*.tar.gz dist/*.sigstore* 9lives.cdx.json
Result: gh: 9lives.cdx.json: no such file or directory
The release is not created despite the 'best-effort; never blocks a release' intent.
Current:
attestations: trueProposed:
dist/*.whl dist/*.tar.gz dist/*.sigstore* *.cdx.json| run: gh release create "$GITHUB_REF_NAME" dist/* --generate-notes | |
| dist/*.whl dist/*.tar.gz dist/*.sigstore* *.cdx.json |
More Info
- Threat model: A transient failure in the CycloneDX SBOM generator (network, tool install, etc.) prevents the release from being published, even though the build and signatures are ready.
- Specific code citations: The SBOM step at
.github/workflows/release.ymlusescontinue-on-error: trueand writes9lives.cdx.json; the release step on the next block passes that literal filename togh release create. - Existing protections:
continue-on-error: trueon the SBOM step allows the job to continue, but the release step has no conditional guard for the literal file path. - Proposed mitigation: Change the literal
9lives.cdx.jsonto a glob like*.cdx.jsonsoshopt -s nullglobskips it when the SBOM step fails, or build the file list conditionally before callinggh release create. - Alternative mitigations considered: Moving the SBOM generation into a separate job would also isolate failures, but is broader than needed for this one-line fix.
- Severity calibration: Score 4 because it breaks the release pipeline on a non-essential step, which is a real operational failure under plausible conditions (SBOM tool flakiness), but it does not affect runtime correctness or security.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release.yml
Line: 27
Comment:
**SBOM failure blocks release despite 'best-effort' comment**
The GitHub release step references the literal path `9lives.cdx.json`, but that file is only created if the preceding SBOM step succeeds. Because `shopt -s nullglob` only affects glob expansions, a missing SBOM file causes `gh release create` to fail, contradicting the SBOM step's `continue-on-error: true` and the 'best-effort; never blocks a release' comment. Change the literal path to a glob so nullglob can skip it when the SBOM step fails.
Example:
SBOM step fails (e.g., cyclonedx-bom install error).
Release step runs: gh release create v0.1.3 --generate-notes dist/*.whl dist/*.tar.gz dist/*.sigstore* 9lives.cdx.json
Result: gh: 9lives.cdx.json: no such file or directory
The release is not created despite the 'best-effort; never blocks a release' intent.
Threat model:
A transient failure in the CycloneDX SBOM generator (network, tool install, etc.) prevents the release from being published, even though the build and signatures are ready.
Specific code citations:
The SBOM step at `.github/workflows/release.yml` uses `continue-on-error: true` and writes `9lives.cdx.json`; the release step on the next block passes that literal filename to `gh release create`.
Existing protections:
`continue-on-error: true` on the SBOM step allows the job to continue, but the release step has no conditional guard for the literal file path.
Proposed mitigation:
Change the literal `9lives.cdx.json` to a glob like `*.cdx.json` so `shopt -s nullglob` skips it when the SBOM step fails, or build the file list conditionally before calling `gh release create`.
Alternative mitigations considered:
Moving the SBOM generation into a separate job would also isolate failures, but is broader than needed for this one-line fix.
Severity calibration:
Score 4 because it breaks the release pipeline on a non-essential step, which is a real operational failure under plausible conditions (SBOM tool flakiness), but it does not affect runtime correctness or security.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
✅ Confirmed real (P1) — fixed in #7. The literal 9lives.cdx.json is now *.cdx.json so nullglob drops it when the best-effort SBOM step fails. v0.1.3 only shipped because that step happened to succeed. Good catch.
There was a problem hiding this comment.
Confirmed — the P1 is resolved. The fix in #7 changed the literal 9lives.cdx.json path in .github/workflows/release.yml:27 to *.cdx.json, so shopt -s nullglob (already set earlier in the step) correctly drops the argument when the best-effort SBOM step fails and produces no file. v0.1.3 only shipped cleanly because the SBOM step happened to succeed; without the glob, any SBOM failure would have crashed gh release create despite continue-on-error: true. No further action needed on this finding.
| ) | ||
| result = asyncio.run(tier1_healer.heal(failure)) | ||
| assert result.success | ||
| assert result.metadata.get("anchor") == "testid" |
There was a problem hiding this comment.
test_tier1_records_winning_anchor does not exercise anchor tracking because the selector is not parsed
The test test_tier1_records_winning_anchor uses #login-btn as the failed_selector, but _extract_identifiers only matches attribute selectors like id='login-btn' — it does not parse CSS id selectors (#id). Therefore _find_alternative_selector will return None, the anchor tracking path is never executed, and the test will either fail (because result.success is False) or exercise a different code path that does not record an anchor. The test does not verify the intended anchor-recording behavior.
Example:
failed_selector = '#login-btn'
page_html = '<button id="login-btn-v2">Sign in</button>'
Extracted identifiers: [] (no attribute match)
→ _find_alternative_selector returns None
→ Tier1 falls through to _try_transformations or _no_heal_result
→ anchor metadata never set, test fails
Suggested fix:
use 'id="login-btn"' as the failed_selector, or update _extract_identifiers to parse '#id' selectorsDetailed reasoning
The fix: either change the test's selector to id='login-btn' so the extraction logic recognizes it, or extend _extract_identifiers to handle CSS #id selectors. The test should still assert that the anchor is 'id' and the changes message includes re-found via id.
More Info
- Threat model: A future change to the anchor tracking code could break the feature without this test catching it, because the test does not actually exercise the anchor path.
- Specific code citations:
_extract_identifiersinsrc/ninelives/healing/tier1.pyonly looks forid='...'attribute selectors; the test uses#login-btn. - Existing protections: No other test covers anchor tracking for CSS id selectors.
- Proposed mitigation: Change the test's
failed_selectortoid='login-btn'so the extraction logic recognizes it and the anchor tracking path is exercised. - Alternative mitigations considered: Alternatively, the extraction logic could be extended to handle
#idselectors. The test should then be updated to reflect that the anchor is still'id'. - Severity calibration: The feature's anchor tracking is not properly tested for a common selector form, leaving a gap in regression coverage. Score 4 because the test as written is misleading and would fail.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_healing.py
Line: 419
Comment:
**test_tier1_records_winning_anchor does not exercise anchor tracking because the selector is not parsed**
The test `test_tier1_records_winning_anchor` uses `#login-btn` as the `failed_selector`, but `_extract_identifiers` only matches attribute selectors like `id='login-btn'` — it does not parse CSS id selectors (`#id`). Therefore `_find_alternative_selector` will return `None`, the anchor tracking path is never executed, and the test will either fail (because `result.success` is `False`) or exercise a different code path that does not record an anchor. The test does not verify the intended anchor-recording behavior.
The fix: either change the test's selector to `id='login-btn'` so the extraction logic recognizes it, or extend `_extract_identifiers` to handle CSS `#id` selectors. The test should still assert that the anchor is `'id'` and the changes message includes `re-found via id`.
Example:
failed_selector = '#login-btn'
page_html = '<button id="login-btn-v2">Sign in</button>'
Extracted identifiers: [] (no attribute match)
→ _find_alternative_selector returns None
→ Tier1 falls through to _try_transformations or _no_heal_result
→ anchor metadata never set, test fails
Threat model:
A future change to the anchor tracking code could break the feature without this test catching it, because the test does not actually exercise the anchor path.
Specific code citations:
`_extract_identifiers` in `src/ninelives/healing/tier1.py` only looks for `id='...'` attribute selectors; the test uses `#login-btn`.
Existing protections:
No other test covers anchor tracking for CSS id selectors.
Proposed mitigation:
Change the test's `failed_selector` to `id='login-btn'` so the extraction logic recognizes it and the anchor tracking path is exercised.
Alternative mitigations considered:
Alternatively, the extraction logic could be extended to handle `#id` selectors. The test should then be updated to reflect that the anchor is still `'id'`.
Severity calibration:
The feature's anchor tracking is not properly tested for a common selector form, leaving a gap in regression coverage. Score 4 because the test as written is misleading and would fail.
Suggested fix shape:
use 'id="login-btn"' as the failed_selector, or update _extract_identifiers to parse '#id' selectors
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Dismissing as a false positive. _extract_identifiers does parse CSS #id selectors — id_match = re.search(r"#([a-zA-Z][\w-]*)", selector) (tier1.py:98) matches #login-btn. The path is exercised: this test passes asserting result.metadata['anchor'] == 'id' and 're-found via id', and the pre-existing test_tier1_heals_moved_id_from_page_snapshot also uses #login-btn and passes. The #(\w+) you may have seen is a separate TRANSFORMATIONS entry, not the extractor.
There was a problem hiding this comment.
Retracting — the finding was a false positive. _extract_identifiers at tier1.py:98 uses re.search(r"#([a-zA-Z][\w-]*)", selector), which correctly parses CSS #id selectors like #login-btn. The #(\w+) pattern is a separate TRANSFORMATIONS entry (tier1.py:24), not the extractor. test_tier1_records_winning_anchor exercises the anchor-tracking path as intended: _extract_identifiers yields ("id", "login-btn"), _find_in_html resolves login-btn-v2, and the test's assertions on metadata["anchor"] and changes_made are valid. The pre-existing test_tier1_heals_moved_id_from_page_snapshot also uses #login-btn and passes, confirming the path works.
Learned something new
"For detecting CSS id selectors, use the extractor
_extract_identifierswhich handles#idselectors viar"#([a-zA-Z][\w-]*)"pattern, not the TRANSFORMATIONS entry which uses#(\w+)"
Undo: reply@sigilix forgetto this comment.
| @@ -194,7 +194,14 @@ def _heal_loop(spec: Path, working_spec: Path, *, auto_apply: bool, max_iteratio | |||
| elif tier == HealingTier.TIER2_AI_SUGGEST: | |||
There was a problem hiding this comment.
Tier 3 handler silently treats all failure types the same when NINELIVES_HEAL_ASSERTIONS is not set
The _heal_loop function now branches on failure.failure_type == FailureType.ASSERTION_FAILED to print a special message, but the else branch prints a generic message for all other failure types. If a new FailureType is added in the future, it will silently fall into the generic message without any explicit handling, which could mask a missing branch. The fix is to add an explicit elif for each known failure type or a final else that raises an error for unknown types.
Example:
A new FailureType.TIMEOUT is added to the enum. The select_strategy method returns HealingTier.TIER3_HUMAN for it. The _heal_loop handler prints the generic message instead of a timeout-specific message, and no one notices the missing branch.
Suggested fix:
if failure.failure_type == FailureType.ASSERTION_FAILED:
...
elif failure.failure_type == FailureType.LOCATOR_NOT_FOUND:
print(f"{PAW} this failure needs a human ({tier.value}) — no automatic fix attempted.")
else:
raise ValueError(f"Unknown failure type: {failure.failure_type}")More Info
- Threat model: A future developer adds a new
FailureTypeenum value and forgets to update this handler. The new failure type will be silently treated as a generic human-needed failure, potentially hiding a bug or missing a special handling path. - Specific code citations:
src/ninelives/cli.pyline 194-214: theelsebranch in_heal_loopthat handlesHealingTier.TIER3_HUMANand other tiers.src/ninelives/healing/strategy.pylines 199-212: theselect_strategymethod that returnsHealingTier.TIER3_HUMANforFailureType.ASSERTION_FAILEDwhen the opt-in is not set. - Existing protections: The
select_strategymethod explicitly handlesFailureType.ASSERTION_FAILEDandFailureType.LOCATOR_NOT_FOUND, but the consumer in_heal_loopdoes not mirror this exhaustiveness. - Proposed mitigation: Add an explicit
elif failure.failure_type == FailureType.LOCATOR_NOT_FOUND:branch before theelse, or use a match statement to ensure all arms are covered. - Alternative mitigations considered: A
matchstatement would provide exhaustiveness checking at the language level, but a simpleelifchain is sufficient for the current two-arm enum. - Severity calibration: This is a maintainability issue, not a runtime bug. The current code works correctly for the two existing failure types, but the lack of exhaustiveness makes future changes error-prone. Score 3 reflects the moderate risk of silent misbehavior when the enum grows.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ninelives/cli.py
Line: 194
Comment:
**Tier 3 handler silently treats all failure types the same when NINELIVES_HEAL_ASSERTIONS is not set**
The `_heal_loop` function now branches on `failure.failure_type == FailureType.ASSERTION_FAILED` to print a special message, but the `else` branch prints a generic message for all other failure types. If a new `FailureType` is added in the future, it will silently fall into the generic message without any explicit handling, which could mask a missing branch. The fix is to add an explicit `elif` for each known failure type or a final `else` that raises an error for unknown types.
Example:
A new FailureType.TIMEOUT is added to the enum. The select_strategy method returns HealingTier.TIER3_HUMAN for it. The _heal_loop handler prints the generic message instead of a timeout-specific message, and no one notices the missing branch.
Threat model:
A future developer adds a new `FailureType` enum value and forgets to update this handler. The new failure type will be silently treated as a generic human-needed failure, potentially hiding a bug or missing a special handling path.
Specific code citations:
`src/ninelives/cli.py` line 194-214: the `else` branch in `_heal_loop` that handles `HealingTier.TIER3_HUMAN` and other tiers. `src/ninelives/healing/strategy.py` lines 199-212: the `select_strategy` method that returns `HealingTier.TIER3_HUMAN` for `FailureType.ASSERTION_FAILED` when the opt-in is not set.
Existing protections:
The `select_strategy` method explicitly handles `FailureType.ASSERTION_FAILED` and `FailureType.LOCATOR_NOT_FOUND`, but the consumer in `_heal_loop` does not mirror this exhaustiveness.
Proposed mitigation:
Add an explicit `elif failure.failure_type == FailureType.LOCATOR_NOT_FOUND:` branch before the `else`, or use a match statement to ensure all arms are covered.
Alternative mitigations considered:
A `match` statement would provide exhaustiveness checking at the language level, but a simple `elif` chain is sufficient for the current two-arm enum.
Severity calibration:
This is a maintainability issue, not a runtime bug. The current code works correctly for the two existing failure types, but the lack of exhaustiveness makes future changes error-prone. Score 3 reflects the moderate risk of silent misbehavior when the enum grows.
Suggested fix shape:
if failure.failure_type == FailureType.ASSERTION_FAILED:
...
elif failure.failure_type == FailureType.LOCATOR_NOT_FOUND:
print(f"{PAW} this failure needs a human ({tier.value}) — no automatic fix attempted.")
else:
raise ValueError(f"Unknown failure type: {failure.failure_type}")
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Dismissing. The generic "needs a human" message for all non-assertion Tier-3 types (navigation, flow-changed) is the intended graceful fallback — only the assertion case warrants the special "possible real bug" wording. The suggested fix (raise on an unknown FailureType) would make the CLI crash on a failure it could otherwise report cleanly — strictly worse UX for an end-user tool. If we grow the enum and want per-type copy, that's a deliberate follow-up, not an exhaustiveness guard that aborts.
There was a problem hiding this comment.
You're right, the original finding was wrong. The code is intentionally designed this way.
Looking at the full picture:
-
The strategy selector already gates exhaustiveness.
select_strategyinsrc/ninelives/healing/strategy.py:191-208explicitly handles everyFailureTypethat should route to Tier 1 or Tier 2. The remaining types (NAVIGATION_FAILED,FLOW_CHANGED,NETWORK_ERROR,SYNTAX_ERROR,UNKNOWN) all fall through toTIER3_HUMAN— that's the correct, deliberate default. -
The CLI's
elsebranch is the Tier 3 handler. Atsrc/ninelives/cli.py:194-205, theif failure.failure_type == FailureType.ASSERTION_FAILEDcheck carves out the one special message ("possible real bug — behavior changed, not a selector"), and theelseprints the generic "needs a human" message for all other Tier 3 types. That's the right UX: only assertions get the extra warning about hidden bugs; everything else that escalates to a human is reported cleanly without crashing. -
raiseon an unknown FailureType would be strictly worse. The CLI is an end-user tool. Crashing on a failure it could report is bad UX. The graceful fallback is the feature, not a bug. -
If the enum grows, the strategy selector is the right place to add per-type routing, and the CLI message can get per-type copy as a deliberate follow-up. An exhaustiveness guard that aborts is not appropriate here.
Retracting the finding. The else branch at cli.py:197 is the intended graceful fallback for all non-assertion Tier 3 failures.
…he release (#7) Sigilix (PR #6, P1): gh release create referenced the literal 9lives.cdx.json, but shopt -s nullglob only skips *globs* — a missing SBOM (its step is continue-on-error) would make gh release create fail with 'no such file', contradicting the best-effort intent. Use *.cdx.json so nullglob drops it when absent. The v0.1.3 run only survived because the SBOM step happened to succeed.
1 participant
Four moat features — the ones that make 9lives hard to copy and safe to trust.
1. 🛡️ Behavior-vs-drift guard (the trust feature)
A failing assertion means the app behaved differently — not that a selector moved. The old code routed assertion failures with an
expected/receivedmessage to a Tier 2 rewrite, which would edit the assertion to make the test pass — silently masking a real regression. Now:NINELIVES_HEAL_ASSERTIONS=1if you really want an assertion update proposed.Live proof:
2. ⚓ Anchor-aware Tier 1
Re-identify the element by anchors in stability order (
testid > id > aria-label > text > class) and record which anchor won (metadata.anchor, and "Re-found viatestid" in the report) — so a heal is explainable and the durable identity is preferred over fragile copy/CSS.3. 📣 Footer growth CTA
Sharper self-healing pitch in the PR-comment footer. No client telemetry (the README promise stands); install attribution comes from the
9lives.runedge redirect, server-side.4. 🔏 Signed, provable releases (
release.yml)attestations: true) — signed build provenance on PyPI, OIDC only..sigstorebundles attached to the GitHub release.Bump to 0.1.3. 4 new tests (guard default + opt-in, anchor recording, stable-anchor preference); suite 32 green, ruff clean. Ship: merge → tag
v0.1.3→git tag -f v1 && git push -f origin v1.