Skip to content

Commit 8bc8fef

Browse files
feat(parser): add provider facade core
Introduce the Provider interface, ProviderBase/ProviderFactory, and source-set helpers; own provider discovery and lookup at the root; and add the legacy-call shim scan that gates provider files. fix(parser): cover Aider, OMP, Reasonix in migration manifest These agents live in the registry but were absent from the provider migration manifest, so ValidateProviderMigrationModes failed once the registry began enforcing that every agent has a mode. They remain on the legacy path here; later stack commits migrate them to concrete providers and flip these entries to provider-authoritative. fix(sync): keep shadow provider discovery observational Shadow provider mode must not add provider-only work or satisfy source lookups that the legacy runtime would miss. Otherwise a migration comparison can change live sync behavior before the provider becomes authoritative.\n\nProvider-authoritative discovery now reports discovery failures as sync failures and suppresses the provider completion watermark for that run, preserving the next incremental pass. The shim scan also keeps pending exemptions honest by failing stale entries while ignoring provider-owned selector methods.\n\nValidation: go test -tags "fts5" ./internal/parser -run 'TestProviderFilesDoNotReferenceLegacyEntrypoints' -count=1; go test -tags "fts5" ./internal/sync -run 'Test(DiscoverProviderSourcesOnlyRunsAuthoritativeProviders|SyncAllProviderDiscoveryFailureSkipsFinishedWatermark|FindSourceFileFallsBackToAuthoritativeNonFileProvider|ClassifyProviderChangedPath|ProcessFileShadow|ProcessFileProviderAuthoritative|ProviderVirtualSourceBackedByEvent)' -count=1; go test -tags "fts5" ./internal/parser ./internal/sync -count=1; go vet ./...; git diff --check docs(parser): clarify provider freshness contract The facade spec still described successful parses as eligible for a clean skip-cache entry, which conflicts with the no-schema-change data-version model and can leave unchanged sessions stale after parser upgrades.\n\nDocument stored changed-path hints explicitly and keep successful unchanged-source freshness tied to DB metadata plus parser data version, reserving skipped_files for retry, failure, and explicit skip cases.\n\nValidation: go test -tags "fts5" ./internal/parser -run 'TestProviderFilesDoNotReferenceLegacyEntrypoints' -count=1; git diff --check. mdformat ran via commit hook. docs(parser): pin provider source identity semantics The facade contract needs to say exactly which provider source key is persisted because the migration intentionally avoids a schema change. Without that rule, providers could diverge between SourceRef, SourceFingerprint, and sessions.file_path identities.\n\nAlso define capability conformance by meaningful field presence so unsupported zero-value fields are treated consistently in provider tests.\n\nValidation: git diff --check. mdformat is unavailable on PATH, but the commit hook ran. style(docs): mdformat provider dual-run harness plan
1 parent 833ccf4 commit 8bc8fef

22 files changed

Lines changed: 5389 additions & 127 deletions

Makefile

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,15 @@ nilaway-golangci-build:
332332

333333
# Run NilAway through the custom golangci-lint module plugin.
334334
nilaway: pricing-snapshot ensure-embed-dir nilaway-golangci-build
335-
$(CUSTOM_GCL) run --config .golangci.nilaway.yml ./...
335+
@set -e; \
336+
root=$$(pwd); \
337+
dirs=$$(go list -f '{{.Dir}}' ./...); \
338+
for dir in $$dirs; do \
339+
pkg="./$${dir#$$root/}"; \
340+
echo "$(CUSTOM_GCL) run --config .golangci.nilaway.yml $$pkg"; \
341+
GOMAXPROCS=$${GOMAXPROCS:-1} GOGC=$${GOGC:-10} GOMEMLIMIT=$${GOMEMLIMIT:-512MiB} \
342+
$(CUSTOM_GCL) run --config .golangci.nilaway.yml "$$pkg"; \
343+
done
336344

337345
# Install pinned local lint tools.
338346
lint-tools:
Lines changed: 308 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,308 @@
1+
# Provider Dual-Run Harness Implementation Plan
2+
3+
> **For agentic workers:** REQUIRED SUB-SKILL: Use
4+
> superpowers:subagent-driven-development (recommended) or
5+
> superpowers:executing-plans to implement this plan task-by-task. Steps use
6+
> checkbox (`- [ ]`) syntax for tracking.
7+
8+
**Goal:** Add the root-level provider migration harness so provider branches
9+
must opt into shadow comparison instead of only adding parallel provider
10+
implementations.
11+
12+
**Architecture:** The parser package owns the per-`AgentType` migration manifest
13+
because provider branches already change parser factories. The sync package owns
14+
the source-level observation helper because it converts provider `Fingerprint`
15+
and `Parse` calls into engine-shaped planned effects without touching the live
16+
database.
17+
18+
**Tech Stack:** Go 1.26, `testing`, `github.com/stretchr/testify`, git-spice
19+
stacked branches.
20+
21+
**Migration mode semantics:**
22+
23+
- `legacy-only`: only the legacy parser/sync path runs and writes. This is the
24+
default for legacy adapter providers and is allowed for concrete providers
25+
only with an explicit rollback note and open follow-up task.
26+
- `shadow-compare`: the legacy path remains authoritative for DB writes,
27+
skip-cache persistence, data-version rows, source metadata, diagnostics,
28+
SSE, and return values. The provider path runs through the shared provider
29+
runner and produces normalized in-memory planned effects. Tests compare
30+
those planned effects against the legacy outcome; runtime mismatches are
31+
developer diagnostics only and must not create user-visible parse
32+
diagnostics.
33+
- `provider-authoritative`: the provider path owns writes and return values and
34+
the old provider-specific legacy branch is gone. This mode is reserved for
35+
the stack tip after every parse-capable provider has passed shadow
36+
comparison.
37+
- `import-only`: the provider is intentionally excluded from filesystem parse
38+
comparison because it represents non-filesystem import/export metadata
39+
rather than a parser replacement.
40+
41+
Promotion requires fixture evidence for parsed sessions, exclusions, skip-cache
42+
keys, data-version state, source metadata, diagnostics, retry state, and
43+
source-key/session-ID compatibility. Rollback means moving the manifest entry
44+
back to `legacy-only`, recording the reason in kata/review notes, and leaving
45+
the legacy path authoritative until the mismatch is fixed.
46+
47+
Provider observations must reject cross-provider output before planning effects
48+
and before any remote machine prefix is applied. `ParseResult.Session.Agent`
49+
must equal the provider `AgentType`. Persisted session IDs in the result graph
50+
must use the provider's ID prefix when one exists; this includes result IDs,
51+
parent IDs, usage-event session IDs, subagent links, exclusions, and diagnostic
52+
session IDs. Diagnostic `SourceError.SourceKey` values are required and must be
53+
the provider fingerprint key, `SourceRef.FingerprintKey`, `SourceRef.Key`, or a
54+
virtual key derived from one of those candidates by appending `#`, `::`, or `|`.
55+
56+
`ProviderPlannedEffects` is an engine-shaped comparison model, not a second
57+
writer. Its source key is the fingerprint key when available, then
58+
`SourceRef.FingerprintKey`, then `SourceRef.Key`. Its skip-cache key follows the
59+
same engine order used for persisted skip decisions. Its data-version entries
60+
match the rows the legacy engine would stamp after successful writes, including
61+
retry state from `DataVersionNeedsRetry`. Its diagnostics mirror parse
62+
diagnostics without inserting them into the live store. Provider retry-reason
63+
text and SSE scopes are outside the root process-result comparison until a later
64+
caller task exposes equivalent legacy data.
65+
66+
Performance rule: shadow comparison may double-parse a source only while that
67+
provider is actively migrating. Large roots and shared database providers need
68+
fixture or benchmark coverage before promotion, and caller-level shadow wiring
69+
must keep provider failures from blocking legacy writes unless a test is
70+
explicitly asserting the mismatch.
71+
72+
______________________________________________________________________
73+
74+
### Task 1: Provider Migration Manifest
75+
76+
**Files:**
77+
78+
- Create: `internal/parser/provider_migration.go`
79+
80+
- Modify: `internal/parser/provider_test.go`
81+
82+
- [ ] **Step 1: Write the failing manifest tests**
83+
84+
Add tests that prove the manifest covers the registry and rejects a concrete
85+
provider left in `legacy-only` mode:
86+
87+
```go
88+
func TestProviderMigrationModesCoverRegistry(t *testing.T) {
89+
err := ValidateProviderMigrationModes(ProviderFactories(), ProviderMigrationModes())
90+
require.NoError(t, err)
91+
}
92+
93+
func TestProviderMigrationModesRejectConcreteProviderLeftLegacyOnly(t *testing.T) {
94+
factory := testProviderFactory{def: AgentDef{Type: AgentCodex, DisplayName: "Codex"}}
95+
modes := map[AgentType]ProviderMigrationMode{
96+
AgentCodex: ProviderMigrationLegacyOnly,
97+
}
98+
99+
err := ValidateProviderMigrationModes([]ProviderFactory{factory}, modes)
100+
require.Error(t, err)
101+
assert.Contains(t, err.Error(), "codex")
102+
assert.Contains(t, err.Error(), "shadow-compare")
103+
}
104+
```
105+
106+
- [ ] **Step 2: Run the parser tests and verify RED**
107+
108+
Run:
109+
110+
```bash
111+
go test -tags "fts5" ./internal/parser -run TestProviderMigrationModes -count=1
112+
```
113+
114+
Expected: FAIL because `ProviderMigrationMode`, `ProviderMigrationModes`, and
115+
`ValidateProviderMigrationModes` do not exist yet.
116+
117+
- [ ] **Step 3: Implement the manifest types and validation**
118+
119+
Create `internal/parser/provider_migration.go` with:
120+
121+
```go
122+
type ProviderMigrationMode string
123+
124+
const (
125+
ProviderMigrationLegacyOnly ProviderMigrationMode = "legacy-only"
126+
ProviderMigrationShadowCompare ProviderMigrationMode = "shadow-compare"
127+
ProviderMigrationProviderAuthoritative ProviderMigrationMode = "provider-authoritative"
128+
ProviderMigrationImportOnly ProviderMigrationMode = "import-only"
129+
)
130+
```
131+
132+
Add a registry-covering manifest initialized to `legacy-only`, return copies to
133+
callers, and validate:
134+
135+
- every provider factory has one mode;
136+
137+
- no extra manifest entry points at an unknown agent;
138+
139+
- concrete non-legacy factories cannot remain `legacy-only`;
140+
141+
- `shadow-compare`, `provider-authoritative`, and `import-only` require a
142+
concrete factory;
143+
144+
- `import-only` is allowed only for Claude.ai and ChatGPT.
145+
146+
- [ ] **Step 4: Run the parser tests and verify GREEN**
147+
148+
Run:
149+
150+
```bash
151+
go test -tags "fts5" ./internal/parser -run TestProviderMigrationModes -count=1
152+
```
153+
154+
Expected: PASS.
155+
156+
### Task 2: Source-Level Provider Observation
157+
158+
**Files:**
159+
160+
- Create: `internal/sync/provider_shadow.go`
161+
162+
- Create: `internal/sync/provider_shadow_test.go`
163+
164+
- [ ] **Step 1: Write failing observation tests**
165+
166+
Add tests that use a fake provider to prove the helper:
167+
168+
- calls `Fingerprint` before `Parse`;
169+
- converts `ParseOutcome` into an observation;
170+
- records planned data-version/source/diagnostic effects in memory;
171+
- never accepts a mismatched `SourceRef.Provider`;
172+
- rejects provider results, exclusions, and diagnostics whose agent or persisted
173+
session-ID namespace belongs to another provider.
174+
175+
The main test should assert:
176+
177+
```go
178+
assert.Equal(t, []string{"fingerprint", "parse"}, provider.calls)
179+
assert.Equal(t, []string{"codex:one"}, observation.Planned.DataVersionSessionIDs())
180+
assert.Equal(t, []string{"codex:two"}, observation.Planned.RetrySessionIDs())
181+
assert.Equal(t, []string{"source-key"}, observation.Planned.SourceKeys)
182+
assert.Len(t, observation.Planned.Diagnostics, 1)
183+
```
184+
185+
- [ ] **Step 2: Run the sync tests and verify RED**
186+
187+
Run:
188+
189+
```bash
190+
go test -tags "fts5" ./internal/sync -run TestObserveProviderSource -count=1
191+
```
192+
193+
Expected: FAIL because `ObserveProviderSource` and observation types do not
194+
exist.
195+
196+
- [ ] **Step 3: Implement the minimal observation helper**
197+
198+
Create `internal/sync/provider_shadow.go` with:
199+
200+
```go
201+
type ProviderObserveRequest struct {
202+
Source parser.SourceRef
203+
Machine string
204+
ForceParse bool
205+
}
206+
207+
type ProviderObservation struct {
208+
Results []parser.ParseResult
209+
ExcludedSessionIDs []string
210+
SourceErrors []parser.SourceError
211+
SkipReason parser.SkipReason
212+
ForceReplace bool
213+
Planned ProviderPlannedEffects
214+
}
215+
```
216+
217+
`ObserveProviderSource` checks the source/provider type match, calls
218+
`Fingerprint`, calls `Parse`, validates provider output invariants, and builds
219+
in-memory planned effects. It must not accept a `db.DB`, `Engine`, writer
220+
callback, or mutable skip-cache reference.
221+
222+
- [ ] **Step 4: Run the sync tests and verify GREEN**
223+
224+
Run:
225+
226+
```bash
227+
go test -tags "fts5" ./internal/sync -run TestObserveProviderSource -count=1
228+
```
229+
230+
Expected: PASS.
231+
232+
### Follow-Up: Caller-Level Wiring
233+
234+
**Files:**
235+
236+
The root harness branch wires the shared `processFile` shadow comparison. The
237+
remaining caller families below stay as later sync migration work so provider
238+
branches can add caller-specific source selection, hint lookup, and acceptance
239+
coverage one behavior group at a time.
240+
241+
**Step 1: Wire remaining source-processing callers into shadow comparison**
242+
243+
Move changed-path sync and `SyncSingleSession` semantics into the caller-level
244+
dual-run wrapper without adding a duplicate `processFile` hook. These callers
245+
reuse the shared `processFile` observation for parse comparison, then add
246+
caller-specific source selection, stored-source hints, and acceptance assertions
247+
around that observation. They must leave live DB/diagnostic/SSE state driven
248+
only by the legacy result.
249+
250+
**Step 2: Add lookup/watch/diagnostic caller coverage**
251+
252+
Move session watch flows, export/source lookup, source mtime, token-usage raw
253+
source probing, parse-diff, and parse diagnostics through the same provider
254+
runner. Tests must cover source lookup freshness, virtual paths, source mtime,
255+
raw probing behavior, report shape, and source-error behavior.
256+
257+
**Step 3: Define runtime mismatch reporting**
258+
259+
Mismatches are test failures in shared parity tests. Runtime mismatch reporting
260+
is developer-only logging or debug diagnostics and must include provider, source
261+
key, fingerprint key, mode, field path, legacy value summary, provider value
262+
summary, and whether fingerprinting or parsing failed. It must not persist
263+
user-visible parse diagnostics while `shadow-compare` is active.
264+
265+
### Task 3: Validation And Commit
266+
267+
**Files:**
268+
269+
- Modify as needed from Tasks 1-2.
270+
271+
- [ ] **Step 1: Format and verify**
272+
273+
Run:
274+
275+
```bash
276+
go fmt ./...
277+
go test -tags "fts5" ./internal/parser -run TestProviderMigrationModes -count=1
278+
go test -tags "fts5" ./internal/sync -run TestObserveProviderSource -count=1
279+
go test -tags "fts5" ./internal/parser -count=1
280+
go test -tags "fts5" ./internal/sync -count=1
281+
go vet ./...
282+
git diff --check
283+
```
284+
285+
Expected: all commands pass. If `go fmt ./...` rewrites unrelated comments,
286+
restore only unrelated user-owned changes before committing.
287+
288+
- [ ] **Step 2: Commit on `provider-facade-core`**
289+
290+
Commit the root harness slice with a conventional message:
291+
292+
```bash
293+
git add docs/superpowers/plans/2026-06-20-provider-dual-run-harness.md internal/parser/provider_migration.go internal/parser/provider_test.go internal/sync/provider_shadow.go internal/sync/provider_shadow_test.go
294+
git commit -m "feat(parser): add provider migration harness"
295+
```
296+
297+
- [ ] **Step 3: Restack locally when explicitly authorized**
298+
299+
If the user has explicitly authorized branch changes and restacking for this
300+
session, run:
301+
302+
```bash
303+
git-spice upstack restack
304+
```
305+
306+
Expected: dependent provider branches are replayed on the harness branch and
307+
conflicts are resolved provider by provider. Do not push, submit, or update PRs
308+
unless the user has separately authorized that network operation.

0 commit comments

Comments
 (0)