fix: withOAuthValidation reads request from getContext() (closes #33)

[heskew]

Summary

Second of the split from closed PR #43. The Harper v5 migration landed in #46; this PR is the withOAuthValidation fix for Resource API v2.

Closes #33

Old code: const request = args.find((arg) => arg?.session !== undefined); — a v4 / legacy pattern. Resource API v2 method signatures are get(target) / receive(target, data) — the request lives on the resource context via this.getContext(), not in args. The wrapper silently passed through v2 calls without validating.

New code: read the request from this.getContext(). No legacy-args fallback — post-v5 migration, v2 is the only shape supported.

Tests

test/lib/withOAuthValidation.test.js is new (9 tests):

• Valid-session passthrough
• requireAuth: true + no session → 401
• requireAuth: false + no session → passthrough
• Custom onValidationError
• Stale-provider clearing (requireAuth: false)
• Stale-provider rejection (requireAuth: true) — rejection path the fix enables
• Expired token without refresh token (requireAuth: true) — same
• Non-HTTP methods pass through untouched
• No-getContext fallthrough

The two bolded failure-path tests were added after Claude flagged this exact gap on the previous combined attempt: my original test file only covered the happy path, leaving the "this is now broken → 401" branch unverified. That was a legit finding (ai-review-log#1).

Test plan

• bun test — 433 pass, 2 skip, 0 fail
• npm run lint passes
• npm run format:check passes (.claude/settings.local.json warn is gitignored)
• CI — Node matrix + Bun
• Claude review

Closes #33

[claude]

1. Security bypass: requireAuth: true silently passes through when no context is found

File: src/lib/withOAuthValidation.ts:75
What: The if (!request) branch calls originalMethod.apply(this, args) unconditionally — regardless of requireAuth. When getContext() is absent or returns a context without a session, the wrapper bypasses all authentication logic and calls the underlying method.
Why it matters: Any resource decorated with requireAuth: true becomes silently accessible in any call path where the v2 context is missing or malformed. Auth should fail-closed: missing context → deny, not pass.
Suggested fix:

if (!request) {
    if (requireAuth) {
        return { status: 401, body: { error: 'Unauthorized', message: 'No request context available' } };
    }
    return originalMethod.apply(this, args);
}

---

2. Missing test: requireAuth: true + no context must 401

File: test/lib/withOAuthValidation.test.js:249
What: The "fallthrough: no context available" describe block has one test that asserts passthrough, but only under the default requireAuth: false. There is no test verifying that requireAuth: true + missing context produces a 401.
Why it matters: The security-critical fail-closed branch (finding 1) has zero test coverage. Without a dedicated test the fix can regress invisibly.
Suggested fix: Add to the "fallthrough" describe block:

it('returns 401 when requireAuth is true and no context is available', async () => {
    const resource = { async get(target) { return { status: 200 }; } };
    const wrapped = withOAuthValidation(resource, {
        providers: mockProviders, logger: mockLogger, requireAuth: true,
    });
    const result = await wrapped.get({ path: '/protected' });
    assert.equal(result.status, 401);
    assert.equal(result.body.error, 'Unauthorized');
});

[claude]

1. onValidationError called with undefined — breaks the published callback contract

File: src/lib/withOAuthValidation.ts:86
What: When requireAuth: true and no v2 context is available, the code calls onValidationError(undefined as any, error). The published signature is (request: Request, error: string) => any — callers are entitled to access request.session, request.headers, request.ip, etc. Passing undefined as any silently defeats that guarantee at runtime.
Why it matters: Any real onValidationError handler that reads a property of request (e.g. to redirect, log the client IP, or inspect the session) will throw a TypeError at the exact moment auth is failing — replacing a clean 401 with an unhandled exception.
Suggested fix: Either (a) keep the fail-closed 401 path and skip onValidationError entirely when no request is available (document that the hook is not called in this case), or (b) add a onNoContext option for this distinct error case. Either way, remove the undefined as any pass-through.

---

2. onValidationError branch in the no-context path has zero test coverage

File: test/lib/withOAuthValidation.test.js
What: The new fail-closed block (lines 80–94 of withOAuthValidation.ts) has two sub-paths: return a default 401, or delegate to onValidationError. Only the default-401 path is exercised by the "fail-closed" test. The onValidationError branch in that block is never reached by any test. The onValidationError test (line 133) exercises a different path — session exists but has no oauth data — where request is a real object.
Why it matters: The untested branch is the one with the undefined as any bug above. Without a test, the broken behaviour can't be caught by CI even after a fix is applied.
Suggested fix: Add a test: resource with no getContext, requireAuth: true, onValidationError provided — assert the hook is/is not called (per whichever fix is chosen for finding 1) and that the method is not invoked.

---

3. JSDoc example still shows the v4 argument pattern — misleads v2 integrators

File: src/lib/withOAuthValidation.ts:38
What: The @example block shows async get(target, request) where request is a method parameter. That is the v4 pattern this PR explicitly removes. In v2 the second argument is data, not the request; a user copying this example will get request === undefined and a runtime crash on request.session.oauthUser.
Why it matters: The example is the primary integration guide for withOAuthValidation. It now actively teaches the broken pattern.
Suggested fix: Update the example to v2 style — async get(target) with const request = this.getContext() — and remove the stale request parameter.

[claude]

1. Docstring example passes class constructor — Proxy breaks or silently bypasses OAuth

File: src/lib/withOAuthValidation.ts:58
What: The @example was updated to withOAuthValidation(MyResource, {...}), passing the class constructor directly. The Proxy implementation does target[prop] to find methods. On a class constructor, instance methods live on MyResource.prototype, not on the constructor object itself, so target['get'] is undefined. All tests use plain objects (from makeV2Resource), so this is undetected. Following the docstring verbatim either throws TypeError: undefined.apply(...) or — if Harper instantiates the proxied constructor — produces an unwrapped instance where the get trap never fires and OAuth validation is silently skipped.
Why it matters: An integrator following the documented example gets either a hard crash or a complete OAuth bypass, with no test catching the regression.
Suggested fix: Change the example to withOAuthValidation(new MyResource(), {...}), or revert it to a plain-object resource literal consistent with how the tests actually construct the target.

---

2. No test that onValidationError is bypassed in the no-context path

File: src/lib/withOAuthValidation.ts:99
What: Both the JSDoc (lines 34–37) and the inline comment block (lines 90–98) explain that onValidationError is deliberately not called when request is undefined, because its signature requires a valid Request. There is no test that passes onValidationError and triggers the no-context branch; the existing 'fail-closed' test omits the callback entirely.
Why it matters: Without that test, a refactor that accidentally propagates undefined into onValidationError will pass the full test suite and only surface as a TypeError in user code at runtime.
Suggested fix: Add one case to the 'fallthrough: no context available' suite that supplies an onValidationError spy, asserts the result is the hardcoded 401 (not the custom handler's response), and asserts the spy was never called.

[claude]

1. Missing onValidationError tests for the two newly-enabled rejection paths

File: test/lib/withOAuthValidation.test.js
What: Tests 6 and 7 (stale-provider and expired-token, requireAuth: true) verify the default 401 response but have no companion test with onValidationError provided. Test 4 covers onValidationError only for the !hasOAuth path. The stale-provider and expired-token paths invoke onValidationError via the same pattern, but in v2 those paths were unreachable before this fix — so the combination is new, untested behavior.
Why it matters: An integrator who provides onValidationError to handle stale-provider or expired-token rejections (the two explicitly called-out newly-enabled paths) is relying on code that has no test coverage. Any regression in those onValidationError branches would be invisible to the suite.
Suggested fix: Add two tests mirroring tests 6 and 7 but with onValidationError set — one for the stale-provider path (ghost-provider, requireAuth: true), one for the expired-token path — verifying the callback receives a truthy request, the correct error string, and that its return value is used as the response.

[claude]

1. put, patch, delete have zero test coverage

File: test/lib/withOAuthValidation.test.js
What: Three of the five new HTTP method overrides in Wrapped are never exercised by any test — not for validation failure, not for delegation to the parent, not even the "undefined super method" path. post has one test (valid session, method not implemented on parent). put, patch, and delete have none.
Why it matters: The method-name string passed to delegate ('put', 'patch', 'delete') is the only connection between the override and the lookup in parentProto[method]. A typo there silently bypasses OAuth validation for every request on those verbs — the test suite would not catch it. With three verbs untested, the new subclass-based wrapper is only half-verified.
Suggested fix: Add at minimum one requireAuth: true + invalid session → 401 test and one valid session → delegation test for each of put, patch, and delete. The "undefined super method" test using post is a reasonable template; replicate it for the missing verbs.

---

2. request.session.oauth is deleted before onValidationError is called in two branches

File: src/lib/withOAuthValidation.ts:77-98
What: In the "no provider name" branch (line 78–79) and the "unknown provider" branch (lines 93–94), delete request.session.oauth and delete request.session.oauthUser execute before onValidationError(request, error) is invoked. The request received by the custom handler has its OAuth fields already cleared. Only the !hasOAuth branch — the only branch covered by the onValidationError test — has nothing to delete, so this ordering is never observed under test.
Why it matters: An integrator writing a custom error handler to log which provider caused the failure (e.g., for audit trails) will find request.session.oauth is undefined and request.session.oauthUser is undefined. In the "unknown provider" branch the provider name is in the error string, but oauthUser is unrecoverable. Nothing in the JSDoc or the OAuthValidationOptions declaration documents that the callback receives a partially-mutated request. This can cause silent data loss in audit handlers or a TypeError if the handler accesses nested fields assuming they're present.
Suggested fix: Either (a) document explicitly that request.session.oauth and request.session.oauthUser will be undefined when onValidationError is called from these two branches, and add tests that verify a custom handler receives the expected (cleared) state, or (b) defer the deletion until after onValidationError returns, so the handler can read the original session data before the caller decides whether to persist the cleared state.

[claude]

Two blockers found.

1. withOAuthValidation is never re-exported from the package entry point

File: src/index.ts (missing line)
What: src/lib/withOAuthValidation.ts exports withOAuthValidation, getOAuthProviders, and OAuthValidationOptions, but none are re-exported from src/index.ts. package.json maps the package root to ./dist/index.js only -- no sub-path export for ./lib/withOAuthValidation exists.
Why it matters: The JSDoc @example says import { withOAuthValidation } from '@harperfast/oauth', but that import fails at runtime because the symbol is absent from the package public surface. Every integrator following the documented pattern gets an import error. The test imports directly from dist/lib/withOAuthValidation.js, bypassing the package entry, so CI stays green while the shipped API is broken.
Suggested fix: Add to src/index.ts:

export { withOAuthValidation, getOAuthProviders } from './lib/withOAuthValidation.ts';
export type { OAuthValidationOptions } from './lib/withOAuthValidation.ts';

---

2. Test demonstrates spread on session.oauth -- pattern silently fails in production

File: test/lib/withOAuthValidation.test.js:432 (and :461)
What: The onValidationError callbacks in the callback-sees-full-session tests capture OAuth data using { ...request.session.oauth } and { ...request.session.oauthUser }. In the test these are plain objects so spread works. In production, request.session.oauth is a GenericTrackedObject and { ...obj } copies nothing (CLAUDE.md Non-Obvious Gotchas).
Why it matters: This test is the de-facto code sample for onValidationError. Integrators who copy this callback pattern for audit logging will silently get empty objects and lose the OAuth provenance data the test is designed to illustrate.
Suggested fix: Replace spread captures with explicit property access, e.g. { provider: request.session.oauth.provider, accessToken: request.session.oauth.accessToken }.

[claude]

1. getOAuthProviders is new public API with no tests

File: src/lib/withOAuthValidation.ts:236 (exported via src/index.ts:39)
What: getOAuthProviders is a new function added to this PR and newly exported from the package entry point, making it public API for the first time. No test in test/lib/withOAuthValidation.test.js (or anywhere else) exercises it.
Why it matters: The function silently swallows all errors and returns null on failure. A missing test means the scope-traversal logic (scope.parent?.resources?.get?.('oauth') → scope.resources?.get?.('oauth')) is unverified, and a breaking change to the traversal order or fallback behavior would go undetected. It is now a public API commitment.
Suggested fix: Add at least two test cases: (a) provider found via scope.parent.resources, (b) provider not found returns null. The existing MockResource pattern in the test file is sufficient infrastructure.

---

2. Missing test: requireAuth: false + validateAndRefreshSession returns { valid: false }

File: src/lib/withOAuthValidation.ts:128-130
What: When a session has valid OAuth data, the provider is registered, but the token is expired and no refresh token is available — and requireAuth is false — the code falls through to return undefined (passthrough). This path calls validateAndRefreshSession, which has the side-effect of calling clearOAuthSession before returning { valid: false }. There is no test covering this combination.
Why it matters: The whole point of this PR is that the v2 request is now correctly obtained via getContext(), which means validateAndRefreshSession is now actually reachable for the first time in v2 apps. The happy path (requireAuth: true + expired token → 401) is tested, but the requireAuth: false variant — where the session is cleared and the method runs without OAuth data — is new reachable behavior with no coverage.
Suggested fix: Add a test: session has { provider: 'github', accessToken: 'expired', expiresAt: Date.now() - 60_000, refreshToken: undefined }, requireAuth: false. Assert (a) the wrapped method is called, and (b) context.session.oauth is undefined after the call (cleared by clearOAuthSession inside validateAndRefreshSession).

[claude]

1. onValidationError returning undefined silently bypasses authentication

File: src/lib/withOAuthValidation.ts:69,89,107,119
What: validateOAuthForRequest uses return undefined as the sentinel meaning "no error — delegate should continue." But onValidationError can also return undefined (e.g., a void async logging callback). When it does, validateOAuthForRequest returns undefined, delegate sees deny === undefined, and falls through to calling the underlying method — even though validation failed and requireAuth is true. All four error paths are affected (!hasOAuth, !providerName, !providerData, !validation.valid).
Why it matters: Any integrator who writes an async error handler that logs and returns nothing bypasses auth silently. No test or type constraint prevents this.
Suggested fix: After calling onValidationError, if it returns undefined, fall back to the default 401 rather than propagating undefined:

if (onValidationError) {
  const result = await onValidationError(request, error);
  return result ?? { status: 401, body: { error: 'Unauthorized', message: error } };
}

Apply the same ?? defaultDeny pattern to every call site, including the clearStaleOAuth paths where response is stored before returning.

---

2. Missing test: onValidationError + expired/invalid token (!validation.valid path)

File: test/lib/withOAuthValidation.test.js
What: The onValidationError describe block covers the !hasOAuth, !providerName, and !providerData paths but not the !validation.valid path (token expired, refresh failed). That branch calls return onValidationError(request, error) at line 119 and is the most likely real-world scenario for auth errors in production sessions.
Why it matters: Without a test, the undefined-bypass bug in finding #1 on this specific path would not be caught even after the fix is applied; it also leaves the most common real-world failure mode (expired token + custom error handler) without behavioral coverage.
Suggested fix: Add a test with expiresAt: Date.now() - 60_000, no refreshToken, requireAuth: true, and an onValidationError that returns undefined — assert the underlying method does NOT run and the response is a 401.

[claude]

1. Expired-token test mocks don't exercise the production clearOAuthSession path

File: test/lib/withOAuthValidation.test.js:459 (makeSession), 639-669, 1029-1077
What: makeSession() never defines a delete() method on the session object, so every call to clearOAuthSession inside validateAndRefreshSession takes the in-memory fallback (delete session.oauth; delete session.oauthUser). In production, Harper sessions have session.delete(id), which causes clearOAuthSession (handlers.ts:316-317) to destroy the entire DB record — not just the two oauth fields. Both expired-token test suites (requireAuth true and false) exercise only the fallback branch and give false confidence about the production code path.
Why it matters: The test comment on line 1023 ("stale oauth metadata must be cleared") implies only the oauth fields are cleared, but in production the user's entire session is destroyed from the database. Any code reading session state after clearOAuthSession — including the resource method that still runs in the requireAuth: false path — is operating on a deleted record.
Suggested fix: Add a variant of makeSession that includes delete: async (id) => { delete session[id]; } (or a spy) and run both expired-token test cases through it, asserting the delete was called and the post-deletion behaviour is correct.

---

2. requireAuth: false + expired token: full session deletion while resource returns 200

File: src/lib/withOAuthValidation.ts:131-145
What: When requireAuth: false and validateAndRefreshSession returns {valid: false} due to an expired token with no refresh token, validateOAuthForRequest returns undefined (passthrough) and the delegate calls the parent method. But validateAndRefreshSession has already called clearOAuthSession(session, logger) as a side effect (sessionValidator.ts:115), which in production calls session.delete(session.id) and destroys the DB session record. The resource method then runs and returns 200 with a dead session. Compare the stale-provider path (lines 127–128): it calls the local clearStaleOAuth(), which only deletes in-memory fields and does not touch the DB.
Why it matters: A hybrid-auth resource (requireAuth: false) will silently and permanently log users out whenever their OAuth token expires — the user gets a 200 but their next request has no session at all. This is new reachable behaviour for v2 (the old Proxy code bypassed validateAndRefreshSession entirely when args-based detection failed). The inconsistency between the stale-provider path (in-memory clear only) and the expired-token path (full DB destruction) makes the wrapper's contract unpredictable.
Suggested fix: Either (a) call clearStaleOAuth() explicitly in the !validation.valid + !requireAuth branch instead of relying on sessionValidator's side effect, keeping behaviour consistent with the stale-provider path; or (b) document and test that expired tokens in requireAuth: false resources cause full session termination — including a mock that has session.delete() — so integrators can make an informed choice.

[claude]

No blockers found.

[claude]

1. WeakSet dedup relies on context-object identity across static→instance dispatch — unverified against real Harper Resource

File: src/lib/withOAuthValidation.ts:270–278
What: The validated WeakSet prevents double-validation by checking whether the context object was already validated. The static override adds request to the WeakSet; the instance override checks this.getContext(). Dedup only works if this.getContext() returns the exact same object reference as the request argument passed to the static. The tests guarantee this because MockResource.getContext() returns this._context, which is whatever the test passed to the constructor. There is no guarantee Harper's real Resource base class does the same — if its get(target, request) static wraps or enriches the request before constructing a Resource instance, this.getContext() returns the wrapper and validated.has(wrapper) is false. Validation then runs twice, calling validateAndRefreshSession (and potentially a network token-refresh) twice per request.
Why it matters: Not a security regression — double-validation returns the same result — but it silently defeats the stated dedup invariant that justifies using a class-based wrapper over a simpler per-call approach. If token refresh is a network call, doubling it is a real cost at scale, and the WeakSet overhead adds complexity for no benefit.
Suggested fix: Either document the object-identity assumption explicitly (e.g., "requires Harper to construct instances with the unmodified context object") and verify it against Harper v5's Resource constructor, or move the dedup logic to a request-scoped signal that doesn't depend on object identity — e.g., a property stamped onto the request/session object rather than a closure-level WeakSet.

---

2. 405-semantics claim is unverified against real Harper's Resource base class statics

File: src/lib/withOAuthValidation.ts:309–319 and test/lib/withOAuthValidation.test.js:1021–1057
What: The static override loop installs Wrapped[method] for every verb where (ResourceClass as any)[method] is a function. The test for "unimplemented verbs preserve 405 semantics" uses MockResource, which has no static HTTP methods at all. In that environment, typeof ResourceClass.post === 'undefined', so no static override is installed for post and the test passes. In production, user classes extend Harper's Resource base, which almost certainly defines static handlers for all HTTP verbs (to implement transactional dispatch). If Resource.post is defined, Wrapped.post becomes our auth-checking override — meaning an unauthenticated request to a resource that only implements get would receive a 401 instead of the native Harper 405. The PR explicitly claims 405 is preserved; the test does not verify this against the production base class shape.
Why it matters: Behavior change that integrators writing access-control logic or API clients that distinguish 401 from 405 will observe incorrectly. If authentication status leaks through 405 vs 401 signals, this also has a minor security-information implication.
Suggested fix: Add a test using a StaticCapableResource-shaped base (which already exists in the test file at line 1070) where ALL five verbs have static handlers inherited by the user class, but the user only defines async get(). Assert that calling a non-get verb on the Wrapped class still produces 405 (or at minimum, does not return 200/204 from the user's missing method).

---

3. Missing test: onValidationError returning undefined is untested on stale-provider failure paths

File: test/lib/withOAuthValidation.test.js:919–1018
What: The expired-token failure path has an explicit test (line 1232) confirming that a void onValidationError handler (one that returns undefined) falls back to the default 401 — the security-critical "fail-closed on undefined handler return" property. The !providerName and !providerData stale-provider paths use the same callCallbackOrDeny helper with the same ?? defaultDeny semantics, but both paths are only tested with handlers that return explicit responses. Neither has a test for the void-return case.
Why it matters: If these paths are refactored separately from the expired-token path — e.g., inlining the callback invocation without the defaultDeny argument — authentication could be silently bypassed for sessions carrying stale provider data. The expired-token test would still pass, giving false confidence. The PR description explicitly calls out the "no silent bypass" property as a key correctness guarantee.
Suggested fix: For each of the !providerName and !providerData branches, add a test mirroring "handler returns undefined — wrapper falls back to default 401": pass a no-return onValidationError, assert result.status === 401, and assert the underlying method was not called.

[claude]

Review: fix/withOAuthValidation-resource-api-v2

The subclass approach is architecturally correct — covers both the instance-method and static-method dispatch surfaces, deduplication via WeakSet is sound, 405 preservation is correct, and the `callCallbackOrDeny ?? defaultDeny` guard closes the silent-bypass hole from a void-returning `onValidationError`. Overall implementation is solid.

One blocker on test coverage.

---

1. Static POST/PUT/PATCH dispatch — no test for the 3-arg (target, data, request) call shape

File: src/lib/withOAuthValidation.ts:361

What: The static override extracts the request as args[args.length - 1]. Harper's REST dispatcher uses two distinct arg shapes:

• GET/DELETE → (target, request) — 2 args, request is args[1]
• POST/PUT/PATCH → (target, data, request) — 3 args, request is args[2]

args[args.length - 1] is correct for both shapes. But every static-dispatch test in the suite calls Wrapped.get(target, request) — the 2-arg shape. No test sends Wrapped.post(target, data, request) or the equivalent for PUT/PATCH with a 3-arg call to a user-defined static.

Why it matters: If a future refactor changes the extraction to args[1] ("always the second arg"), it would correctly handle GET/DELETE and silently extract data instead of request for POST/PUT/PATCH statics. The session check (context as any)?.session would then test the request body for a session field — almost certainly undefined — and the no-context fail-closed path would either return 401 unconditionally (breaking requireAuth: false resources) or pass through without validation (if requireAuth is false). All existing tests would still pass. The for (method of HTTP_METHODS) all-verbs tests only exercise instance methods and would not catch this regression.

Suggested fix: Add one test calling a user-defined static async post at the static level with a 3-arg signature and verifying auth is enforced:

it('static post (3-arg shape: target, data, request): enforces OAuth', async () => {
  class PostResource extends StaticCapableResource {
    static async post(_target, _data, request) {
      return { status: 201, body: { user: request.session.oauthUser.email } };
    }
  }
  const Wrapped = withOAuthValidation(PostResource, {
    providers: mockProviders,
    logger: mockLogger,
    requireAuth: true,
  });

  // No OAuth on session — must be rejected before the user static runs
  const result = await Wrapped.post({ path: '/items' }, { name: 'new-item' }, { session: { id: 'no-oauth' } });
  assert.equal(result.status, 401);

  // Valid session — user static must run and see oauthUser
  const ok = await Wrapped.post({ path: '/items' }, { name: 'new-item' }, { session: makeSession() });
  assert.equal(ok.status, 201);
});

---

Non-blocker note: withOAuthValidation, getOAuthProviders, and OAuthValidationOptions are new public exports from src/index.ts. The project is at 2.0.0-alpha.1 so alpha semver discipline applies, but worth tagging the minor surface expansion in the PR description for release-notes tooling.

[claude]

Review — PR #48: withOAuthValidation Resource API v2 fix

No blockers found.

Verified against the full layered scope (universal + Harper v5 + plugin + OAuth-specific). Key things I traced:

Dispatch surface coverage ✓

Both dispatch paths are intercepted for every user-implemented verb:

• Instance method (async get(target)) — Wrapped.prototype[method] override reads the context via this.getContext().
• Own static (static async get(target, request)) — Wrapped[method] override reads args[args.length - 1] (correct for both 2-arg GET/DELETE and 3-arg POST/PUT/PATCH shapes). The 3-arg shape is explicitly pinned in a test.

Unimplemented verbs are shadowed with undefined on Wrapped (not just omitted), so Harper's resource.method ? … : missingMethod check takes the 405 branch as expected.

Fail-closed on every auth path ✓

• No context / no session → 401 (not passthrough).
• onValidationError returning undefined → result ?? defaultDeny → falls back to the default 401. Tested on all three branches (expired token, unknown provider, no provider name).
• onValidationError is intentionally not called on the no-context path (callback signature requires a Request; passing undefined would produce a TypeError in user code). Tested and documented.

WeakSet dedup ✓

markValidated stamps both ctx and ctx.getContext?.() so the dedup fires correctly when Harper's transactional normalises request → request.getContext() before constructing the per-request instance. markValidated is only called on validation success, so a denied request is never stamped as validated.

405 semantics ✓

The parentInstanceMethod detection uses parentProto[method] (full prototype chain), not hasOwnProperty, correctly picking up methods inherited from user-defined base classes. The assumption that Resource.prototype does not define get/post/etc. as functions (only statics do) is consistent with Harper v5's transactional-static pattern and is exercised by the StaticCapableResource test fixture which mirrors that shape.

Newly public API ✓

withOAuthValidation, getOAuthProviders, and OAuthValidationOptions are freshly exported from src/index.ts (not previously public), so the signature change from the old Proxy-based (resource: any) to the new (ResourceClass: T): T is not a breaking change to a published API.

[claude]

Review: fix/withOAuthValidation-resource-api-v2

No blockers found.

Surfaces verified

Static-method dispatch bypass (the core fix)

• Object.prototype.hasOwnProperty.call(ResourceClass, method) correctly detects user-owned statics vs. Resource base's inherited transactionals. A static override is installed on Wrapped for every verb the user actually implements — so Harper's direct class-level dispatch (Wrapped.get(target, request)) is intercepted before the user's static runs. Tests cover both 2-arg (GET/DELETE) and 3-arg (POST/PUT/PATCH) shapes (args[args.length - 1] extracts the request correctly).

405 preservation

• Unimplemented verbs are explicitly shadowed with undefined on Wrapped (not just omitted), which blots out the inherited Resource base transactional so REST dispatch's truthy-check takes the 405 branch rather than running validation on a verb the user never exposed. Tested against both MockResource (no base statics) and StaticCapableResource (all base statics — the production-shaped case).

onValidationError → undefined silent-bypass guard

• callCallbackOrDeny uses result ?? defaultDeny, so a void/undefined-returning callback is indistinguishable from no callback. Tested for all three requireAuth: true failure paths (no OAuth, stale provider, expired token).

requireAuth: false doesn't invoke onValidationError

• The callback is only reached inside if (requireAuth) blocks. Tests confirm the callback is NOT fired for stale-provider and expired-token paths when requireAuth: false.

Deduplication (WeakSet)

• markValidated stamps both the incoming request arg and request.getContext?.() (the normalized context Harper's transactional may produce). hasValidated checks both. The dedup test verifies the provider lookup happens exactly once across the static → instance chain. One note: the test's MockResource.getContext() returns the same object reference (so the inner !== ctx branch inside markValidated is never exercised), but this branch path is a performance guard (no double token refresh), not a security invariant — a miss just means two refresh calls, not an auth bypass.

Fail-closed, no context

• When context has no .session, requireAuth: true returns 401 without invoking onValidationError (the callback signature requires a Request; passing undefined would TypeError in user code). Tested with an explicit assertion that handlerCalled === false.

New public symbols

• withOAuthValidation, getOAuthProviders, OAuthValidationOptions are net-new exports from src/index.ts. All three have test coverage. getOAuthProviders tests cover parent-scope lookup, same-scope fallback, both-missing → null, and throw-in-traversal → null.

Production-path session (session.delete)

• makeProductionLikeSession correctly exercises the session.delete(id) branch inside clearOAuthSession. The onValidationError callback-sees-session-before-delete test confirms the callback observes full oauth/oauthUser data even after clearOAuthSession runs the DB-destruction path.