Skip to content

fix(ci): walker Js.Exn catch-arm detection + vscode E2E false-positive (closes #426) - #428

Merged
hyperpolymath merged 1 commit into
mainfrom
fix/ci-walker-and-vscode-e2e
May 28, 2026
Merged

fix(ci): walker Js.Exn catch-arm detection + vscode E2E false-positive (closes #426)#428
hyperpolymath merged 1 commit into
mainfrom
fix/ci-walker-and-vscode-e2e

Conversation

@hyperpolymath

Copy link
Copy Markdown
Owner

Summary

Closes #426. Two small surgical fixes that resolve the 3 pre-existing test failures blocking PR auto-merges on main.

Fix 1 — Walker Js.Exn catch-arm detection (resolves 1 test)

tools/res-to-affine/walker.ml — drop the node.children = [] constraint from the fallback in detect_untyped_exception. tree-sitter-rescript parses Js.Exn.Error(_) in a catch-arm as a compound node (constructor pattern with the (_) payload as a child), so the leaf-only check skipped it.

The pre-existing 32-char text length cap is a sufficient guardrail — Js.Exn.Error(_) is 15 chars and the enclosing try/match expressions exceed the cap. Dedupe handles same-line duplicates.

Test: walker-phase2c-parity #2 untyped-exception [OK] (was FAIL).

Fix 2 — vscode E2E false-positive (resolves 2 tests)

packages/affine-vscode/mod.js — rewrite the manual-wiring example in the header doc comment. The previous text contained literal require(\"@hyperpolymath/affine-vscode\") and require(\"vscode-languageclient/node\") strings as illustrative code; once mod.js is embedded at compile time (per #380), the E2E codegen tests' substring-based contains helper false-positived on those comment strings.

New wording uses named placeholders (loadShim() / loadAdapter() / loadVscode() / loadLanguageClient()) plus an explanatory note saying the literal require(...) strings are intentionally omitted so the E2E assertions don't false-positive.

Tests:

  • E2E Node-CJS Codegen #4 --vscode-extension-adapter-override [OK] (was FAIL)
  • E2E Node-CJS Codegen #5 --vscode-extension-no-lc [OK] (was FAIL)

Verification (local)

# walker
dune build tools/res-to-affine/test/test_walker.exe
./_build/default/tools/res-to-affine/test/test_walker.exe
# Test Successful — 7 tests run, 0 failures

# vscode E2E
./_build/default/test/test_main.exe test \"E2E Node-CJS Codegen\"
# All [OK]

Test plan

  • CI `build` job (was failing) → green
  • CI `coverage-visibility` job → green
  • No regression on other vscode/walker tests

🤖 Generated with Claude Code

Closes #426. Fixes the 3 pre-existing test failures that have been
red on main since at least 2026-05-28 10:09Z (commit c871f7f),
blocking PR auto-merges.

Fix 1 — walker.ml detect_untyped_exception (1 test)
====================================================

`Js.Exn.Error(_)` in a catch-arm position is parsed by tree-sitter-rescript
as a compound node (constructor pattern with the `(_)` payload as a
child). The previous fallback constrained matching to leaf nodes
(`node.children = []`), so the compound never matched and the walker
missed line 22 of `tools/res-to-affine/test/fixtures/sample.res`.

Drop the leaf-only constraint. The pre-existing 32-char text length
cap is sufficient guardrail — it allows the constructor-pattern node
(`"Js.Exn.Error(_)"`, 15 chars) but rejects the enclosing try/match
expressions. The master walker's dedupe collapses any same-line
duplicate against the typed `try_expression` hit.

Verified with `tools/res-to-affine/test/test_walker.exe`:
  walker-phase2c-parity #2 untyped-exception  [OK]   (was FAIL)
  all 7 walker tests pass                            (was 6/7)

Fix 2 — packages/affine-vscode/mod.js doc comment (2 tests)
============================================================

The `mod.js` source is embedded at compile time per #380, then the E2E
codegen tests assert that certain `require("…")` strings are NOT present
in the emitted CJS under specific flag combinations:

  - test_vscode_extension_adapter_override (#4): expects
    `require("@hyperpolymath/affine-vscode")` absent when the override
    flag is set.
  - test_vscode_extension_no_lc (#5): expects
    `require("vscode-languageclient/node")` absent when --no-lc is set.

The manual-wiring example in mod.js's header doc-comment contained both
strings literally as illustrative code. The substring-based `contains`
helper in `test_e2e.ml` cannot distinguish doc-comment text from
generated code, so both tests false-positived on the embedded comment.

Rewrite the doc comment to describe the wiring in prose with named
`loadShim()` / `loadAdapter()` / `loadVscode()` / `loadLanguageClient()`
placeholders, plus an explanatory note stating that the literal
`require(...)` strings are intentionally omitted so the E2E substring
assertions don't false-positive.

Verified with `test/test_main.exe`:
  E2E Node-CJS Codegen #4 --vscode-extension-adapter  [OK]   (was FAIL)
  E2E Node-CJS Codegen #5 --vscode-extension-no-lc    [OK]   (was FAIL)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@hyperpolymath
hyperpolymath merged commit fca9419 into main May 28, 2026
22 of 23 checks passed
@hyperpolymath
hyperpolymath deleted the fix/ci-walker-and-vscode-e2e branch May 28, 2026 12:41
@github-actions

Copy link
Copy Markdown

🔍 Hypatia Security Scan

Findings: 81 issues detected

Severity Count
🔴 Critical 4
🟠 High 10
🟡 Medium 67

⚠️ Action Required: Critical security issues found!

View findings
[
  {
    "reason": "Action actions/checkout@v6 needs attention",
    "type": "unpinned_action",
    "file": "publish-jsr.yml",
    "action": "pin_sha",
    "rule_module": "workflow_audit",
    "severity": "medium"
  },
  {
    "reason": "Action denoland/setup-deno@v2 needs attention",
    "type": "unpinned_action",
    "file": "publish-jsr.yml",
    "action": "pin_sha",
    "rule_module": "workflow_audit",
    "severity": "medium"
  },
  {
    "reason": "Issue in affine-vscode-publish.yml",
    "type": "unknown",
    "file": "affine-vscode-publish.yml",
    "action": "flag",
    "rule_module": "workflow_audit",
    "severity": "medium"
  },
  {
    "reason": "Issue in casket-pages.yml",
    "type": "unknown",
    "file": "casket-pages.yml",
    "action": "flag",
    "rule_module": "workflow_audit",
    "severity": "medium"
  },
  {
    "reason": "Issue in casket-pages.yml",
    "type": "unknown",
    "file": "casket-pages.yml",
    "action": "flag",
    "rule_module": "workflow_audit",
    "severity": "medium"
  },
  {
    "reason": "Issue in ci.yml",
    "type": "unknown",
    "file": "ci.yml",
    "action": "flag",
    "rule_module": "workflow_audit",
    "severity": "medium"
  },
  {
    "reason": "Issue in ci.yml",
    "type": "unknown",
    "file": "ci.yml",
    "action": "flag",
    "rule_module": "workflow_audit",
    "severity": "medium"
  },
  {
    "reason": "Issue in ci.yml",
    "type": "unknown",
    "file": "ci.yml",
    "action": "flag",
    "rule_module": "workflow_audit",
    "severity": "medium"
  },
  {
    "reason": "Issue in ci.yml",
    "type": "unknown",
    "file": "ci.yml",
    "action": "flag",
    "rule_module": "workflow_audit",
    "severity": "medium"
  },
  {
    "reason": "Issue in ci.yml",
    "type": "unknown",
    "file": "ci.yml",
    "action": "flag",
    "rule_module": "workflow_audit",
    "severity": "medium"
  }
]

Powered by Hypatia Neurosymbolic CI/CD Intelligence

hyperpolymath added a commit that referenced this pull request Jul 7, 2026
…tale hypatia pin) (#679)

## Problem — every PR carries two red checks

1. **`analyze (actions, none)`** — CodeQL, and per the workflow's own
comment a **required** check:
   ```
##[error]Loaded a configuration file for version '4.36.2', but running
version '4.36.3'
   ```
Root cause: dependabot #670/#673 bumped `analyze`/`upload-sarif` to
v4.36.3 (`54f647b7`) but left **`init`** at v4.36.2 (`8aad20d1`) — a
partial bump (dependabot treats init/analyze as separate actions). init
writes a config the newer analyze rejects. The stale `# v3.28.1`
comments hid the split.

2. **`hypatia / Hypatia Neurosymbolic Analysis`** — `Cache not found for
input keys: hypatia-scanner-v2-… → exit 1`.
Root cause: the wrapper pinned the standards reusable at `d135b05` — the
known estate CI-red master-cause pin. Fixed upstream in standards
**#428** (GITHUB_TOKEN), **#441** (un-stale scanner cache — our exact
symptom), #445, #453.

## Fix (2 files, 3 lines)
- `codeql.yml`: `init` → `54f647b7 # v4.36.3` (same SHA as analyze);
comments corrected to the SHAs' real version.
- `hypatia-scan.yml`: reusable pin `d135b05` → `5fa7a834e` (#453, latest
commit touching the reusable).

YAML validated. This PR's own check run is the live test — `analyze
(actions, none)` and `hypatia` should both go green here.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant