diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 4eeb080..7555732 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -144,8 +144,11 @@ func Run(ctx context.Context, options Options) (Summary, error) { return nil }) var v8Overrides map[int]json.RawMessage + // The same lastAction PushSnapshot hands the goja state below: the two + // engines evaluate this step against one action, not two. + stepAction := lastAction g.Go(func() error { - overrides, err := extractorSource.ExtractorOverrides(gctx) + overrides, err := extractorSource.ExtractorOverrides(gctx, stepAction) if err != nil { logger.Warn("v8 extractor evaluation failed", "step", si, "err", err) return nil diff --git a/internal/runner/source.go b/internal/runner/source.go index 486904c..513a8ab 100644 --- a/internal/runner/source.go +++ b/internal/runner/source.go @@ -23,8 +23,23 @@ type ActionSource interface { // ExtractorSource yields per-step extractor overrides the runner applies after // PushSnapshot. The mobile path has none (returns nil); the web path returns the // values its extractors computed in V8 against the real DOM. +// +// lastAction is the action the previous step actually applied, the same value +// PushSnapshot hands the goja state. The web path has to install it in the page +// before its extractors run: a spec extractor reading state.lastAction runs in +// V8 there, and V8 has no way to know what the runner dispatched. type ExtractorSource interface { - ExtractorOverrides(ctx context.Context) (map[int]json.RawMessage, error) + ExtractorOverrides( + ctx context.Context, + lastAction *verifier.Action, + ) (map[int]json.RawMessage, error) +} + +// lastActionInstaller is the web driver's channel for the previous step's +// action. It is declared here rather than folded into driver.WebDriver so the +// mobile drivers stay untouched; every web driver must implement it. +type lastActionInstaller interface { + SetLastAction(ctx context.Context, encoded json.RawMessage) error } // gojaSource drives both action selection and (trivially) extractor overrides @@ -38,7 +53,10 @@ func (s gojaSource) NextAction(context.Context) (verifier.Action, error) { return s.verifier.NextAction() } -func (gojaSource) ExtractorOverrides(context.Context) (map[int]json.RawMessage, error) { +func (gojaSource) ExtractorOverrides( + context.Context, + *verifier.Action, +) (map[int]json.RawMessage, error) { return nil, nil } @@ -59,7 +77,24 @@ func (s webSource) NextAction(ctx context.Context) (verifier.Action, error) { return verifier.DecodeAction(raw) } -func (s webSource) ExtractorOverrides(ctx context.Context) (map[int]json.RawMessage, error) { +// ExtractorOverrides installs the previous step's action in the page, then +// reads back what the spec's extractors computed against the live DOM. The +// install is not best-effort: a web driver that cannot take it leaves +// state.lastAction null in V8, which silently turns every action-gated +// property vacuously true, so it is reported as an error instead. +func (s webSource) ExtractorOverrides( + ctx context.Context, + lastAction *verifier.Action, +) (map[int]json.RawMessage, error) { + installer, ok := s.web.(lastActionInstaller) + if !ok { + return nil, fmt.Errorf( + "web driver %T cannot install state.lastAction; every property gated "+ + "on the last action would be vacuously true", s.web) + } + if err := installer.SetLastAction(ctx, verifier.EncodeLastAction(lastAction)); err != nil { + return nil, fmt.Errorf("install last action: %w", err) + } return s.web.EvaluateExtractors(ctx) } diff --git a/internal/runner/web_extractor_trace_test.go b/internal/runner/web_extractor_trace_test.go index ae9a33e..0858d02 100644 --- a/internal/runner/web_extractor_trace_test.go +++ b/internal/runner/web_extractor_trace_test.go @@ -44,6 +44,8 @@ func (d *webMockDriver) NextActionFromV8(context.Context) (json.RawMessage, erro return nil, nil } +func (d *webMockDriver) SetLastAction(context.Context, json.RawMessage) error { return nil } + // TestRunner_TraceRecordsTheValueTheVerdictUsed fails if the trace and the // verdict disagree about an extractor. A witness is only an explanation of a // violation if it holds the state the violated property was evaluated against. diff --git a/internal/runner/web_last_action_test.go b/internal/runner/web_last_action_test.go new file mode 100644 index 0000000..f15a877 --- /dev/null +++ b/internal/runner/web_last_action_test.go @@ -0,0 +1,78 @@ +package runner + +import ( + "context" + "encoding/json" + "testing" + "time" + + mockdriver "github.com/priyanshujain/sanderling/internal/driver/mock" +) + +// On web the spec's extractors run in the page, so state.lastAction has to be +// installed there by the runner. It used to be hardcoded null in +// pkg/spec/src/web-runtime.ts, which made every property gated on the last +// action (folio's submitMovesBalanceByTypedAmount, for one) vacuously true on +// web: no failure, no warning, just a green run that proved nothing. + +const lastActionSpec = ` +import { actions } from "@sanderling/spec"; +globalThis.actions = actions(() => []); +globalThis.properties = {}; +` + +// tappingWebDriver is a web target whose V8 picker always taps one named +// control, so the runner has a real applied action to report on the next step. +type tappingWebDriver struct { + *mockdriver.Driver + installed []string +} + +func (d *tappingWebDriver) InstallBundle(context.Context, []byte) error { return nil } + +func (d *tappingWebDriver) EvaluateExtractors(context.Context) (map[int]json.RawMessage, error) { + return nil, nil +} + +func (d *tappingWebDriver) NextActionFromV8(context.Context) (json.RawMessage, error) { + return json.RawMessage(`{"kind":"Tap","x":12,"y":34,"selector":"id:TxnSubmit"}`), nil +} + +func (d *tappingWebDriver) SetLastAction(_ context.Context, encoded json.RawMessage) error { + d.installed = append(d.installed, string(encoded)) + return nil +} + +func TestRunner_WebInstallsLastActionInThePage(t *testing.T) { + state := newHarnessWithSpec(t, lastActionSpec) + web := &tappingWebDriver{Driver: state.mock} + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if _, err := Run(ctx, Options{ + Duration: 100 * time.Millisecond, + IdleTimeout: 20 * time.Millisecond, + MaxSteps: 3, + Driver: web, + Verifier: state.verifier, + TraceWriter: state.writer, + }); err != nil { + t.Fatalf("Run: %v", err) + } + + if len(web.installed) < 2 { + t.Fatalf("the page was handed lastAction %d time(s); the web path never installed it", + len(web.installed)) + } + // Step 1 has no previous action, exactly as the goja host reports it. + if web.installed[0] != "null" { + t.Errorf("step 1 installed %s, want null", web.installed[0]) + } + // Every later step carries what the runner actually applied. The shape is + // the goja host's (internal/verifier/marshal.go lastActionFields), pinned + // against it by TestLastAction_WebJSONMatchesTheGojaObject. + const want = `{"kind":"Tap","on":"id:TxnSubmit"}` + if web.installed[1] != want { + t.Errorf("step 2 installed %s, want %s", web.installed[1], want) + } +}