Skip to content

Phase 0: auth-scoped test infrastructure#78

Merged
kennethphough merged 5 commits into
masterfrom
feature/phase-0-auth-tests
Apr 24, 2026
Merged

Phase 0: auth-scoped test infrastructure#78
kennethphough merged 5 commits into
masterfrom
feature/phase-0-auth-tests

Conversation

@kennethphough

@kennethphough kennethphough commented Apr 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Phase 0 of the Kyte MCP / auth migration (see docs/design/kyte-mcp-and-auth-migration.md). Establishes the auth-scoped test coverage required before Phase 1 refactors the middleware into a strategy dispatcher.

  • Test infrastructure: Docker Compose with MariaDB 10.5.29 (matches prod) + PHP 8.2, PHPUnit suite split into unit/aws with explicit file lists, bootstrap split into DB-only (default) and AWS opt-in.
  • 11 new characterization tests (tests/SignatureTest.php, 308 lines) covering the three-layer HMAC-SHA256 algorithm, generateSignature (flow A step 1), verifySignature (positive + negative), SIGNATURE_TIMEOUT boundary semantics (inclusive pass, +1s fail), unknown API key / account rejection, malformed identity string, and the "undefined"/"0" session quirks. All reflection-based to invoke private methods without altering source.
  • 4 pre-existing bit-rotted tests fixed: ApiTest (called removed Api::init()), DatabaseTest (non-idempotent CREATE USER), ModelTest (missing SELECT keyword), and an @depends doc-comment migrated to #[Depends] attribute.
  • GitHub Actions wired up to actually run the unit suite on push/PR with a MariaDB 10.5.29 service container.
  • One production change: Api::loadModelsAndControllers() now guards its ~50 define() calls with if (!defined($name)). Behavior-preserving (PHP's define() already silently refuses to redefine; this just suppresses the warning in long-running PHPUnit processes). Flagged in the design doc as a Phase 1 consideration: new Api() has a global-state side effect that should inform how the strategy dispatcher instantiates/reuses Api.

18/18 unit tests green, 83 assertions, zero warnings, zero deprecations under PHPUnit 11 / PHP 8.2. CI green on the last push (run 24877617721, 30s).

Explicitly deferred

Not in this PR, revisit at Phase 0.5 or as Phase 1 side-work:

  • SessionManager-level session validation tests (happy-path, revocation, expired).
  • Full handleRequest response-body parity (kyte_api, kyte_pub, kyte_num, kyte_iden, kyte_app_id, account_id).

Neither is a strict blocker for Phase 1 since the strategy-dispatcher refactor doesn't touch SessionManager semantics.

Post-merge plan

  1. Tag v4.2.0.
  2. Rehearse install + rollback on the dev Kyte server (covers risk R4 — rollback has never been rehearsed). Phase 0 is the ideal no-op release for this.
  3. Open feature/phase-1-strategy-dispatcher off the tagged commit.

Test plan

  • 18/18 unit tests green locally under Docker (MariaDB 10.5.29, PHP 8.2)
  • GitHub Actions green on push (run 24877617721)
  • Dev server: install v4.2.0 via composer, verify Shipyard loads + one API round-trip
  • Dev server: downgrade to v4.1.1 via composer, verify rollback succeeds

🤖 Generated with Claude Code

kennethphough and others added 5 commits April 23, 2026 06:05
@kennethphough @claude
Phase 0: test infrastructure scaffolding
f13d4ee 
Add dockerized test environment matching production versions, split
bootstrap so DB-only tests don't drag in AWS constants, and restore
the four bit-rotted unit tests to a green baseline.

- tests/docker-compose.test.yml + tests/Dockerfile pin MariaDB 10.5.29
  and PHP 8.2 (with mysqli, curl, mbstring) to match the dev server.
- tests/bootstrap.php: DB-only, env-var first with constant fallback.
- tests/bootstrap-aws.php: AWS constants isolated, opt-in for AWS tests.
- phpunit.xml.dist: split into `unit` (default) and `aws` testsuites.
- ApiTest: Api::init() was removed; assert construction succeeded and
  core ModelObject flows work.
- DatabaseTest: add setUp that drops test users/DBs before each run
  (CREATE USER is not idempotent on re-run).
- ModelTest::testCreateTable: add missing SELECT keyword in
  customQuery call.

Baseline: 7/7 unit tests pass, 63 assertions.

Part of the auth migration initiative described in
docs/design/kyte-mcp-and-auth-migration.md (Phase 0).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kennethphough @claude
Phase 0: signature and identity characterization tests
cd20b0d 
First batch of tests locking in the current HMAC-SHA256 auth behavior
before the Phase 1 strategy-dispatcher refactor. Documents the
three-layer HMAC algorithm and verifies both flow paths (generate then
verify, single-request verify) produce identical results.

- testSignatureAlgorithmIsThreeLayerHmacSha256: pure algorithm check —
  hash1 = HMAC(token, secret); hash2 = HMAC(identifier, hash1);
  signature = HMAC(unix_epoch, hash2).
- testGenerateSignatureProducesExpectedValue: Api::generateSignature
  (flow A step 1) produces the expected signature for known inputs.
- testVerifySignatureAcceptsCorrectAndRejectsWrong: correct signature
  passes, wrong signature throws SessionException.
- testParseIdentityStringRejectsExpiredTimestamp: timestamps older than
  SIGNATURE_TIMEOUT (3600s default) throw "API request has expired".
- testParseIdentityStringRejectsMalformedString: identity strings that
  don't split into exactly 4 parts on '%' throw "Invalid identity string".

Uses reflection to reach the private methods (generateSignature,
verifySignature, parseIdentityString) without modifying production code.
When Phase 1 extracts these into HmacSessionStrategy, the tests will
still work if behavior is preserved.

Part of docs/design/kyte-mcp-and-auth-migration.md Phase 0.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kennethphough @claude
Phase 0: identity-string edge cases and TTL boundary tests
c0d5c99 
Extend SignatureTest with six additional characterization tests covering
parseIdentityString's branches and the SIGNATURE_TIMEOUT boundary
semantics.

- testParseIdentityStringAcceptsTimestampAtBoundary: the check is
  `time() > utcDate + SIGNATURE_TIMEOUT`, so equality must pass.
- testParseIdentityStringRejectsTimestampOneSecondPastBoundary: +1s
  over the window throws SessionException.
- testParseIdentityStringRejectsUnknownApiKey: throws a plain \Exception
  (not SessionException) with "API key not found".
- testParseIdentityStringRejectsUnknownAccount: throws \Exception with
  "Unable to find account".
- testParseIdentityStringTreatsUndefinedSessionAsZero: the literal
  string "undefined" from JS clients is coerced to "0".
- testParseIdentityStringWithZeroSessionDoesNotPopulateUser: anonymous
  (session=="0") leaves $this->user null.

Refactor the reflection boilerplate into invokeParseIdentity() helper.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kennethphough @claude
Phase 0: wire unit testsuite into GitHub Actions
4aaac58 
Actually run the tests on every push/PR. Previously the workflow
validated composer.json and stopped — the test step was commented out.

- Add MariaDB 10.5.29 service container with health check (matches
  production/dev-server version; GHA waits for healthy before steps).
- Set up PHP 8.2 via shivammathur/setup-php with mysqli, curl,
  mbstring, json extensions.
- Inject KYTE_DB_* env vars for bootstrap.php to pick up.
- Run vendor/bin/phpunit --testsuite unit (excludes AWS tests, which
  need real AWS credentials).
- Trigger on push to master and any feature/** branch so our Phase 0
  branch gets CI coverage.
- Add .phpunit.result.cache + clover.xml to .gitignore.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kennethphough @claude
Phase 0: silence CI noise from define() loop, PHPUnit config, and dep…
57b9f74 
…recated @Depends

Three changes, all behavior-preserving, needed to get CI to exit 0 under
PHPUnit 11 / PHP 8.2:

- src/Core/Api.php: guard model-constant define() calls with defined().
  Harmless in production (fresh PHP-FPM process per request), but each
  `new Api()` in a long-running PHPUnit process was redefining ~50
  constants and emitting PHP warnings. PHP's `define()` silently refuses
  to redefine, so adding the guard matches prior behavior without the
  warning.

- phpunit.xml.dist: migrated to current schema, removed unused <coverage>
  block. The block referenced clover.xml output but CI sets coverage:none
  (no driver), which made PHPUnit 11 emit a runner warning. clover.xml is
  not consumed anywhere — no codecov upload, no artifact, nothing.

- tests/FunctionTest.php: replaced @Depends doc-comment with #[Depends]
  attribute. Doc-comment metadata is deprecated in PHPUnit 11, removed in 12.

Prior CI run: 18 passed / 83 assertions, but exit 1 because 51 warnings +
2 deprecations + 1 runner warning. Local post-fix: 18 passed / 83
assertions, OK, no warnings, no deprecations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kennethphough
kennethphough merged commit a5baf9a into master Apr 24, 2026
3 checks passed
@kennethphough
kennethphough deleted the feature/phase-0-auth-tests branch April 24, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@kennethphough