Add v4 path-segment routing repro script#103
Merged
Merged
Conversation
…uting test Lightweight repro harness for the Studio/CM Okta SSO symptom (Dawson, 2026-05-22) on the v1.x stack: spawns harperdb v4 against a temp fixture with two providers (decoy named "oauth", tenant named "oac-tenant-acme") and asserts that /oauth/oac-tenant-acme/login redirects to the tenant — not the decoy. NOT YET WORKING. harperdb v4's CLI reads its root path from ~/.harperdb/hdb_boot_properties.file (written by `harperdb install`) and doesn't appear to honor a ROOTPATH/HDB_ROOT env override. Two remaining paths: script a `harperdb install` against a temp dir with atomic save/restore of the boot-properties file, or identify the correct HARPER_<section>_<key> env vars v4 accepts (analogous to v5's HARPER_SET_CONFIG). See file header for current state. The counterpart test on main (Harper 5.0.7) is integrationTests/path-segment-routing.test.ts and currently passes — i.e. parseRoute on v5 correctly extracts the URL path segment and Dawson's diagnosis is not reproducible on that stack. parseRoute on v1.x (v1.4.0) is byte-identical to main; if the symptom is real on Dawson's stack, it would have to come from a Harper v4 behavior difference (target.id shape), not the plugin code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Working set of CLI args mirrors @harperfast/integration-testing@0.3.0's spawn pattern (--ROOTPATH, --HDB_ADMIN_*, --HTTP_PORT, --DEFAULTS_MODE, --TC_AGREEMENT, --REPLICATION_HOSTNAME), so harperdb installs into a temp dir and binds on a non-default port without prompting. Result: /oauth/oac-tenant-acme/login redirects to tenant.test/authorize with the tenant client_id (NOT the decoy provider literally named "oauth"). parseRoute on the v1.4.0 plugin source + harperdb 4.7.19 correctly extracts the URL path segment — same as Harper v5. Dawson's "providerName is 'oauth'" diagnosis does not reproduce on either stack. Atomic save/restore of ~/.harperdb/hdb_boot_properties.file: harperdb's installer rewrites that file to point at the temp ROOTPATH. The script now backs up the original before launch and restores it in finally (plus SIGINT/SIGTERM handlers for unclean exits), so an existing local harperdb install survives repeated runs intact. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
heskew
changed the title
Lint failure on CI: stale import from an earlier shape of the script. Also wrapped the long FAIL log string per prettier. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bring the v1.x .github/workflows/ tree in line with main's so the Claude review action's "workflow file must be identical to default branch" validation gate passes for PRs targeting v1.x. The chronic mismatch was silently failing every v1.x PR's review job at the OIDC-token-exchange step. What changes for v1.x as a side effect of the harmonization: - pr-checks.yml: Node 20 dropped from the test matrix (was [20, 22, 24], now [22, 24]); test:coverage is the single coverage path (the v1.x-specific test:node-twenty fork is no longer wired into CI but the script remains in package.json). Action pins updated to current SHAs. - release.yml: setup-node SHA bumped to v6.4.0 to match main. - claude-review.yml: rewritten as a thin caller of the reusable in HarperFast/ai-review-prompts (this is what unblocks the review check). - New: claude-issue-to-pr.yml, claude-mention.yml, gemini-review.yml, validate-caller-workflows.yml — additive, parallel to main's review infrastructure. NOT synced: integration-tests.yml. That workflow runs `npm run install:fixtures && npm run test:integration`, neither of which exists on v1.x. If we want a similar CI check on this branch later it should wrap scripts/v4-routing-repro.mjs (added in this PR), which is a different shape than main's harness. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Reviewed; no blockers found. |
Contributor
|
Reviewed; no blockers found. Prior findings resolved by 19761ef. |
…uild Acts on the agy + codex review pass on this PR. Fixes (blocker + significant): 1. **Backup-overwrite race (blocker, agy)** — relocate the boot-props backup from a hardcoded /tmp path into the per-run tempRoot. On an unclean rerun, the previous design copied the now-dirty BOOT_PROPS_PATH over the still-original backup at the hardcoded path, permanently losing the user's real ~/.harperdb config. With the backup under tempRoot (mkdtemp'd per run), prior leftover backups can't be overwritten — and an existsSync check inside backupBootProps provides a belt-and-suspenders guard if tempRoot were ever reused. 2. **Cleanup path unification (significant, codex)** — replace the process.exit() short-circuit in the signal handlers with a single synchronous `cleanup()` function that the finally block AND every signal handler call. cleanup() now: kills harperdb (with async SIGKILL fallback), restores boot props via the same code path used by normal exits, and removes tempRoot. waitForReady also subscribes to the child process's `error` event so ENOENT / EACCES route through the same cleanup. Adds SIGHUP / SIGQUIT in addition to SIGINT / SIGTERM. 3. **/tmp multi-user collision (significant, agy)** — covered by #1 (backup is now inside the run-scoped tempRoot, no shared-tmp races). 4. **node_modules cp waste (significant, agy)** — eliminate the fixtureDir → components/app recursive copy. Write config.yaml / package.json directly into tempRoot/components/app and run npm install there. Saves seconds of recursive-copy per run. 5. **Silent stale dist/ (significant, codex)** — replace `npm run build` (which is `tsc || true`) with: rm dist/, invoke the local tsc binary directly, then assert dist/index.js was emitted. A TS emission failure now stops the script instead of letting it pass against the previous build. 6. **Peer-dep registry drift (significant, codex)** — pin the fixture's harperdb peer to the exact version (4.7.19) being spawned by the script. Removes the risk that `npm install` resolves a different registry version than what the script's harperBin is. Also adopts agy's `oac-oauth-tenant` rename: the tenant provider name now contains the substring "oauth" so this script (and its v5 counterpart in PR #102) also catches an over-aggressive stripping regression in addition to the prefix-extraction case. Out of scope: the v5 layer applied to v1.x AI reviews (both reviewers flagged this; needs a branch-aware layer input in HarperFast/ai-review-prompts, not changes here). Verified locally: run produces PASS, ~/.harperdb/hdb_boot_properties.file restored to its pre-run state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Adds a lightweight repro / regression-guard script for the symptom Dawson reported on 2026-05-22 (CM/Studio Okta SSO), targeted at the v1.x stack that CM actually consumes (`harperdb ^4.7.28` + `@harperfast/oauth ^1.4.0`).
Counterpart to PR #102 (`main` branch / Harper v5).
What it does
`scripts/v4-routing-repro.mjs` spawns `harperdb` v4 against a temp fixture configured with two static providers:
Then fires `GET /oauth/oac-tenant-acme/login` and inspects the resulting redirect. If `parseRoute` returned the literal `"oauth"` prefix as `providerName` (Dawson's described symptom), the decoy would handle the request and we'd see `decoy.test/authorize` with the decoy's `client_id`. The expected/passing path is the tenant routing.
Result
```
status: 302
location: http://tenant.test/authorize?client_id=tenant-acme-client-id&...
PASS: routed to tenant provider — parseRoute extracted the URL path segment
```
`parseRoute` on `harperdb 4.7.19` + `@harperfast/oauth` v1.4.0 source correctly extracts the URL path segment. Dawson's diagnosis does not reproduce on this stack either (companion PR #102 already established this on the v5 stack). The bug, whatever it actually is, is not in the plugin's URL routing on either branch.
Side-effect protection
`harperdb`'s installer rewrites `~/.harperdb/hdb_boot_properties.file` to point at the temp `ROOTPATH`, which would orphan an existing local `harperdb` install. The script backs up that file before launch and restores it in a `finally` block, with `SIGINT` / `SIGTERM` handlers for unclean exits.
Why not wire it into CI
For now, run manually:
```bash
npm install && node scripts/v4-routing-repro.mjs
```
Set `KEEP_TEMP=1` to inspect the temp dir afterward.
Test plan
🤖 Generated with Claude Code