466 lines
13 KiB
Markdown
466 lines
13 KiB
Markdown
# N15 Selector Report Hardening Implementation Plan
|
|
|
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
|
|
|
**Goal:** Harden the N14 offline selector report tests and verifier so ambiguous strict-mode behavior and report-output safety regressions are caught before merge.
|
|
|
|
**Architecture:** Keep N15 as a test/verifier-only change. Add an in-memory ambiguous-match test to `internal/uiaselector/report_test.go`, then strengthen `scripts/verify-n14-uia-selector-report.ps1` with stale-output cleanup, file/non-empty checks, and JSON match-field allowlist validation.
|
|
|
|
**Tech Stack:** Go 1.23.x tests, existing `internal/uiaselector` package, Windows PowerShell 5.1-compatible verifier script, standard `go test` and repository Git workflow.
|
|
|
|
---
|
|
|
|
## Scope and boundary
|
|
|
|
N15 changes only test and verifier coverage.
|
|
|
|
Modify only:
|
|
|
|
```text
|
|
internal/uiaselector/report_test.go
|
|
scripts/verify-n14-uia-selector-report.ps1
|
|
```
|
|
|
|
Do not modify:
|
|
|
|
```text
|
|
native/ISphereWinHelper/Program.cs
|
|
internal/tools/winhelper.go
|
|
cmd/isphere-mcp/main.go
|
|
cmd/uia-selector-report/main.go
|
|
internal/uiaselector/report.go
|
|
internal/uiaselector/catalog.go
|
|
internal/uiaselector/matcher.go
|
|
```
|
|
|
|
Do not add selectors, helper ops, MCP tools, CLI arguments, live hwnd/process handling, UI automation calls, clicks, keyboard input, clipboard operations, conversation actions, message sending, file transfer, or visible text export.
|
|
|
|
## File structure
|
|
|
|
```text
|
|
internal/uiaselector/report_test.go
|
|
Add one explicit ambiguous strict-mode regression test using an in-memory UIA tree.
|
|
|
|
scripts/verify-n14-uia-selector-report.ps1
|
|
Clean stale outputs before CLI execution, require fresh non-empty report files, and validate JSON match fields with an allowlist.
|
|
```
|
|
|
|
## Task 1: Cover ambiguous strict-mode classification
|
|
|
|
**Files:**
|
|
- Modify: `internal/uiaselector/report_test.go`
|
|
|
|
- [ ] **Step 1: Add the ambiguous strict-mode regression test**
|
|
|
|
Append this test to `internal/uiaselector/report_test.go`:
|
|
|
|
```go
|
|
func TestStrictModeClassifiesAmbiguousMatches(t *testing.T) {
|
|
root := &Node{
|
|
ControlType: "Window",
|
|
AutomationID: "frmMain",
|
|
FrameworkID: "WinForm",
|
|
Children: []Node{
|
|
{ControlType: "Pane", AutomationID: "duplicate", FrameworkID: "WinForm"},
|
|
{ControlType: "Pane", AutomationID: "duplicate", FrameworkID: "WinForm"},
|
|
},
|
|
}
|
|
catalog := []Selector{
|
|
{
|
|
ID: "duplicate_panel",
|
|
Description: "duplicate structural selector",
|
|
AllowedUse: AllowedUseReadOnlyLocate,
|
|
Required: Criteria{AutomationID: "duplicate", ControlType: "Pane", FrameworkID: "WinForm"},
|
|
StabilityScore: 80,
|
|
Evidence: Evidence{CaptureID: "unit-test", UIAPath: "root/duplicate"},
|
|
},
|
|
}
|
|
|
|
report := RunCatalogReport("synthetic-ambiguous.json", root, catalog, fixedReportTime())
|
|
if !report.OK {
|
|
t.Fatalf("report OK=false: %#v", report.Error)
|
|
}
|
|
if report.CatalogSize != 1 {
|
|
t.Fatalf("catalog_size = %d, want 1", report.CatalogSize)
|
|
}
|
|
if report.MatchedCount != 1 {
|
|
t.Fatalf("matched_count = %d, want 1", report.MatchedCount)
|
|
}
|
|
if report.UnmatchedCount != 0 {
|
|
t.Fatalf("unmatched_count = %d, want 0", report.UnmatchedCount)
|
|
}
|
|
if report.AmbiguousCount != 1 {
|
|
t.Fatalf("ambiguous_count = %d, want 1", report.AmbiguousCount)
|
|
}
|
|
if len(report.Selectors) != 1 {
|
|
t.Fatalf("len(selectors) = %d, want 1", len(report.Selectors))
|
|
}
|
|
if got := report.Selectors[0].MatchCount; got != 2 {
|
|
t.Fatalf("selector match_count = %d, want 2", got)
|
|
}
|
|
if got := StrictExitCode(report); got != StrictExitAmbiguous {
|
|
t.Fatalf("StrictExitCode = %d, want %d", got, StrictExitAmbiguous)
|
|
}
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 2: Prove the test catches a broken ambiguous branch**
|
|
|
|
Temporarily change `internal/uiaselector/report.go` in `StrictExitCode` from:
|
|
|
|
```go
|
|
if report.AmbiguousCount > 0 {
|
|
return StrictExitAmbiguous
|
|
}
|
|
```
|
|
|
|
to:
|
|
|
|
```go
|
|
if report.AmbiguousCount > 0 {
|
|
return StrictExitOK
|
|
}
|
|
```
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
gofmt -w internal\uiaselector\report_test.go
|
|
go test ./internal/uiaselector -run "TestStrictModeClassifiesAmbiguousMatches" -count=1
|
|
```
|
|
|
|
Expected: FAIL with:
|
|
|
|
```text
|
|
StrictExitCode = 0, want 3
|
|
```
|
|
|
|
Restore `internal/uiaselector/report.go` immediately after this command so production code again returns `StrictExitAmbiguous` for ambiguous reports. Do not commit the temporary mutation.
|
|
|
|
- [ ] **Step 3: Run the ambiguous test with production code restored**
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
gofmt -w internal\uiaselector\report_test.go internal\uiaselector\report.go
|
|
go test ./internal/uiaselector -run "TestStrictModeClassifiesAmbiguousMatches" -count=1
|
|
```
|
|
|
|
Expected: PASS.
|
|
|
|
- [ ] **Step 4: Run package tests**
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
go test ./internal/uiaselector -count=1
|
|
```
|
|
|
|
Expected: PASS.
|
|
|
|
- [ ] **Step 5: Commit ambiguous strict-mode test**
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
git add internal\uiaselector\report_test.go
|
|
git diff --cached --name-only
|
|
```
|
|
|
|
Expected staged file list:
|
|
|
|
```text
|
|
internal/uiaselector/report_test.go
|
|
```
|
|
|
|
Then commit:
|
|
|
|
```powershell
|
|
git commit -m "test: cover N15 ambiguous strict exit"
|
|
```
|
|
|
|
## Task 2: Harden N14 selector report verifier
|
|
|
|
**Files:**
|
|
- Modify: `scripts/verify-n14-uia-selector-report.ps1`
|
|
|
|
- [ ] **Step 1: Run a failing pre-change verifier hardening check**
|
|
|
|
Run this PowerShell check before editing the verifier:
|
|
|
|
```powershell
|
|
$scriptPath = "scripts\verify-n14-uia-selector-report.ps1"
|
|
$scriptText = Get-Content -LiteralPath $scriptPath -Raw -Encoding UTF8
|
|
$requiredSnippets = @(
|
|
'Remove-Item -LiteralPath $jsonPath, $mdPath -Force -ErrorAction SilentlyContinue',
|
|
'$jsonItem = Get-Item -LiteralPath $jsonPath -ErrorAction Stop',
|
|
'if ($jsonItem.PSIsContainer)',
|
|
'if ($jsonItem.Length -le 0)',
|
|
'$mdItem = Get-Item -LiteralPath $mdPath -ErrorAction Stop',
|
|
'$allowedMatchFields = @(',
|
|
'$match.PSObject.Properties',
|
|
'JSON match leaks visible text field'
|
|
)
|
|
foreach ($snippet in $requiredSnippets) {
|
|
if (-not $scriptText.Contains($snippet)) {
|
|
throw "missing verifier hardening snippet: $snippet"
|
|
}
|
|
}
|
|
```
|
|
|
|
Expected: FAIL, with the first missing snippet indicating stale-output cleanup is not implemented yet.
|
|
|
|
- [ ] **Step 2: Insert stale-output cleanup before CLI execution**
|
|
|
|
In `scripts/verify-n14-uia-selector-report.ps1`, after:
|
|
|
|
```powershell
|
|
$jsonPath = Join-Path $outDir "n12r-selector-report.json"
|
|
$mdPath = Join-Path $outDir "n12r-selector-report.md"
|
|
```
|
|
|
|
insert:
|
|
|
|
```powershell
|
|
Remove-Item -LiteralPath $jsonPath, $mdPath -Force -ErrorAction SilentlyContinue
|
|
```
|
|
|
|
- [ ] **Step 3: Replace output file existence checks with file and non-empty checks**
|
|
|
|
Replace this block:
|
|
|
|
```powershell
|
|
if (-not (Test-Path -LiteralPath $jsonPath)) {
|
|
throw "JSON report was not created: $jsonPath"
|
|
}
|
|
if (-not (Test-Path -LiteralPath $mdPath)) {
|
|
throw "Markdown report was not created: $mdPath"
|
|
}
|
|
```
|
|
|
|
with:
|
|
|
|
```powershell
|
|
$jsonItem = Get-Item -LiteralPath $jsonPath -ErrorAction Stop
|
|
if ($jsonItem.PSIsContainer) {
|
|
throw "JSON report path is not a file: $jsonPath"
|
|
}
|
|
if ($jsonItem.Length -le 0) {
|
|
throw "JSON report is empty: $jsonPath"
|
|
}
|
|
$mdItem = Get-Item -LiteralPath $mdPath -ErrorAction Stop
|
|
if ($mdItem.PSIsContainer) {
|
|
throw "Markdown report path is not a file: $mdPath"
|
|
}
|
|
if ($mdItem.Length -le 0) {
|
|
throw "Markdown report is empty: $mdPath"
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 4: Add JSON match-field allowlist validation after report summary checks**
|
|
|
|
After this block:
|
|
|
|
```powershell
|
|
if ($report.ambiguous_count -ne 0) {
|
|
throw "ambiguous_count=$($report.ambiguous_count), want 0"
|
|
}
|
|
```
|
|
|
|
insert:
|
|
|
|
```powershell
|
|
Write-Host "## JSON match field allowlist"
|
|
$allowedMatchFields = @(
|
|
"path",
|
|
"control_type",
|
|
"automation_id",
|
|
"class_name",
|
|
"framework_id",
|
|
"has_name",
|
|
"name_length"
|
|
)
|
|
$allowedMatchFieldLookup = @{}
|
|
foreach ($field in $allowedMatchFields) {
|
|
$allowedMatchFieldLookup[$field] = $true
|
|
}
|
|
foreach ($selector in @($report.selectors)) {
|
|
foreach ($match in @($selector.matches)) {
|
|
if ($null -eq $match) {
|
|
continue
|
|
}
|
|
foreach ($property in @($match.PSObject.Properties)) {
|
|
$fieldName = $property.Name
|
|
if ($fieldName -eq "name" -or $fieldName -eq "value") {
|
|
throw "JSON match leaks visible text field: $fieldName"
|
|
}
|
|
if (-not $allowedMatchFieldLookup.ContainsKey($fieldName)) {
|
|
throw "JSON match contains disallowed field: $fieldName"
|
|
}
|
|
}
|
|
}
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 5: Run the hardening check again**
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
$scriptPath = "scripts\verify-n14-uia-selector-report.ps1"
|
|
$scriptText = Get-Content -LiteralPath $scriptPath -Raw -Encoding UTF8
|
|
$requiredSnippets = @(
|
|
'Remove-Item -LiteralPath $jsonPath, $mdPath -Force -ErrorAction SilentlyContinue',
|
|
'$jsonItem = Get-Item -LiteralPath $jsonPath -ErrorAction Stop',
|
|
'if ($jsonItem.PSIsContainer)',
|
|
'if ($jsonItem.Length -le 0)',
|
|
'$mdItem = Get-Item -LiteralPath $mdPath -ErrorAction Stop',
|
|
'$allowedMatchFields = @(',
|
|
'$match.PSObject.Properties',
|
|
'JSON match leaks visible text field'
|
|
)
|
|
foreach ($snippet in $requiredSnippets) {
|
|
if (-not $scriptText.Contains($snippet)) {
|
|
throw "missing verifier hardening snippet: $snippet"
|
|
}
|
|
}
|
|
Write-Host "VERIFIER_HARDENING_SNIPPETS_OK"
|
|
```
|
|
|
|
Expected:
|
|
|
|
```text
|
|
VERIFIER_HARDENING_SNIPPETS_OK
|
|
```
|
|
|
|
- [ ] **Step 6: Run the hardened verifier**
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
powershell -NoProfile -ExecutionPolicy Bypass -File scripts\verify-n14-uia-selector-report.ps1
|
|
```
|
|
|
|
Expected output includes:
|
|
|
|
```text
|
|
## go test ./internal/uiaselector ./cmd/uia-selector-report
|
|
## go run ./cmd/uia-selector-report
|
|
N14 selector report generated: catalog=10 matched=10 unmatched=0 ambiguous=0
|
|
## JSON match field allowlist
|
|
## output safety scan
|
|
N14 UIA selector report verification passed.
|
|
```
|
|
|
|
- [ ] **Step 7: Commit verifier hardening**
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
git add scripts\verify-n14-uia-selector-report.ps1
|
|
git diff --cached --name-only
|
|
```
|
|
|
|
Expected staged file list:
|
|
|
|
```text
|
|
scripts/verify-n14-uia-selector-report.ps1
|
|
```
|
|
|
|
Then commit:
|
|
|
|
```powershell
|
|
git commit -m "test: harden N15 selector report verifier"
|
|
```
|
|
|
|
## Task 3: Final verification and boundary check
|
|
|
|
**Files:**
|
|
- No new files.
|
|
- Verify: `internal/uiaselector/report_test.go`, `scripts/verify-n14-uia-selector-report.ps1`, and unchanged helper/MCP action surface.
|
|
|
|
- [ ] **Step 1: Run targeted tests**
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
go test ./internal/uiaselector -run "TestStrictModeClassifiesAmbiguousMatches" -count=1
|
|
go test ./internal/uiaselector ./cmd/uia-selector-report -count=1
|
|
```
|
|
|
|
Expected: both commands PASS.
|
|
|
|
- [ ] **Step 2: Run repository tests**
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
go test ./...
|
|
```
|
|
|
|
Expected: all packages PASS.
|
|
|
|
If dependency download fails with a transient proxy error, retry once with:
|
|
|
|
```powershell
|
|
$env:GOPROXY = "https://goproxy.cn,direct"
|
|
go test ./...
|
|
```
|
|
|
|
- [ ] **Step 3: Run the hardened verifier**
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
powershell -NoProfile -ExecutionPolicy Bypass -File scripts\verify-n14-uia-selector-report.ps1
|
|
```
|
|
|
|
Expected: verifier PASS and output includes `## JSON match field allowlist`.
|
|
|
|
- [ ] **Step 4: Check helper/MCP/live action boundary**
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
git diff --name-only main..HEAD -- native/ISphereWinHelper/Program.cs internal/tools/winhelper.go cmd/isphere-mcp/main.go cmd/uia-selector-report/main.go internal/uiaselector/report.go internal/uiaselector/catalog.go internal/uiaselector/matcher.go
|
|
```
|
|
|
|
Expected: no output.
|
|
|
|
If any file appears, stop and inspect because N15 is test/verifier-only.
|
|
|
|
- [ ] **Step 5: Check changed files**
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
git diff --name-only main..HEAD
|
|
```
|
|
|
|
Expected changed files are limited to:
|
|
|
|
```text
|
|
docs/superpowers/specs/2026-07-09-n15-selector-report-hardening-design.md
|
|
docs/superpowers/plans/2026-07-09-n15-selector-report-hardening-implementation.md
|
|
internal/uiaselector/report_test.go
|
|
scripts/verify-n14-uia-selector-report.ps1
|
|
```
|
|
|
|
- [ ] **Step 6: Final status**
|
|
|
|
Run:
|
|
|
|
```powershell
|
|
git status --short --branch
|
|
git log --oneline --max-count=6
|
|
```
|
|
|
|
Expected: working tree clean on `codex/n15-report-hardening`. Do not push unless the user explicitly requests pushing this branch or merging to main.
|
|
|
|
## Self-review checklist
|
|
|
|
- Spec coverage: ambiguous strict-mode test, stale-output cleanup, file/non-empty checks, JSON match-field allowlist, retained forbidden string scan, and helper/MCP boundary check are all covered.
|
|
- Scope: only `internal/uiaselector/report_test.go` and `scripts/verify-n14-uia-selector-report.ps1` are implementation targets; design and plan docs are documentation targets.
|
|
- Boundary: no helper op, no MCP tool, no live hwnd/process behavior, no UI action, no report format change, no CLI argument change.
|
|
- Verification: final commands include targeted tests, all repository tests, hardened verifier, boundary diff check, and changed-file check.
|
|
- Commit cadence: test and verifier hardening are separate commits. |