Repository navigation
Migrate conductor.json to .conductor/settings.toml - #42
Merged
Merged
Conversation
scott-lowe-vapi
added a commit
that referenced
this pull request
Oct 1, 2026
…CI (#56) ## Problem `npm test` isn't wired into any workflow (`.github/workflows/` only held `promotion.yml`), so nothing blocks a merge that breaks tests. Since the hash-store migration in #41, **20 of 357 tests have failed on `main`**, unnoticed. Bisecting first-parent `main`: 0 failures at #42, 20 failures from #41's merge onward, and no new failures since then (#55 included). ## Diagnosis: stale tests, not engine bugs #41 moved drift baselines out of the state file into `.vapi-state-hash/<org>/<uuid>`, made `upsertState` / `asResourceState` strip legacy fields, and made pull/push/apply refuse legacy-shaped state. The tests were never moved over. | Tests | Failures | Cause | |---|---|---| | `audit.test.ts` | 4 | Fixtures put `lastPulledHash` in state; `audit.ts` correctly reads the hash store now | | `state-migration.test.ts`, 2 in `drift.test.ts` | 4 | Asserted legacy fields survive state writes, which is exactly what the migration removed | | `reconcile-state-key.test.ts` | 4 | Asserted `lastPushedHash` lands in state; the shared push path records the baseline in the hash store instead | | `drift.test.ts` (`checkDriftForUpdate`) | 3 | Missing the new required `env` argument, so the hash-store path was `undefined` | | `drift.test.ts` (converged edge) | 1 | Pinned `both-diverged` for local == platform ≠ baseline, which `classifyDrift` now deliberately returns as `clean` (the documented invariant behind the phantom-drift fix, improvements.md #23) | | Pull/push spawn tests | 4 | Legacy-format state fixtures, refused by the migration gate | The last row matters most. Three of those were the regression guards #41 added for #22 (rename keeps the local filename; same-name clobber) and #23 (a stale baseline must not block a push). **They have never passed**, so those fixes had no working coverage. ## Changes - **CI**: `.github/workflows/ci.yml` runs `npm run build` + `npm test` on every PR and on pushes to `main`, on Node 20 and 22 (the `engines` range). It sets a job timeout and read-only permissions. - **Fixtures moved to the hash store**: - The spawn tests copy `src/` into a temp dir, so the engine's store resolves there; they seed `<tmp>/.vapi-state-hash/<env>/<uuid>`. - `drift.test.ts` seeds through `writeBaseline` under a throwaway `drift-test-<pid>` org and removes it afterwards. - `audit.ts` gains an optional `baselineReader` DI seam next to `stateLoader` / `listLocalIds`. That is the only production-code change, and the default is the existing `readBaseline(VAPI_ENV, uuid)`. - **`push-stale-baseline-noop` now tests what it says.** With empty `credentials`, `maybeBootstrapState` treats state as uninitialized and its bootstrap pull rewrote the stale baseline before the drift check ever ran. Migrating the fixture alone would have produced a green test that exercises nothing. It now seeds a dummy credential and asserts no bootstrap ran. - **Converged-edge test** rewritten to pin `clean`, with the rationale from `drift.ts`. - **Section J** (classifier short-circuit) rewritten around the hash store. The original bug (a rebuilt state section dropped the baseline) can't happen by construction now. The new test pins that separation, so moving the baseline back into the state entry fails it. - **`tool-assistant-cycle.test.ts`** deleted nothing it wrote: `updateToolAssistantRefs` records a baseline into the developer's *real* `.vapi-state-hash/test-fixture-org/` on every run. It now removes it. - `improvements.md` #32. ## Evidence - `npm test`: **355 / 355 pass** (was 337 / 357; two fewer tests because Section J's four tests became one and one `state-migration` test was split in two). `npm run build` is clean. - **Mutation check:** removing the agree-gate (`localHash === platformHash` in both `classifyDrift` and `checkDriftForUpdate`) fails `push-stale-baseline-noop` and the converged-edge test. Restoring it passes both. - **Isolation check:** after a full run, the real `.vapi-state-hash/` holds no test-written baseline. The `drift-test-<pid>` folder is removed, and the `tool-assistant-cycle` baseline file is deleted, leaving only an empty gitignored `test-fixture-org/` folder. ## Not in this PR `tsconfig.json` still includes only `src/**/*`, so the typecheck never sees `tests/`. That's why stale fixture shapes compiled silently. Including `tests/` surfaces **37 existing type errors**; that's its own change. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR migrates the legacy
conductor.jsonconfiguration to the new.conductor/settings.tomlformat.Generated by Conductor.