Skip to content

fix: route autopg auth through wrapper#139

Merged
namastex888 merged 1 commit into
mainfrom
fix/auth-subcommand-routing
Jun 4, 2026
Merged

fix: route autopg auth through wrapper#139
namastex888 merged 1 commit into
mainfrom
fix/auth-subcommand-routing

Conversation

@namastex888
Copy link
Copy Markdown
Contributor

@namastex888 namastex888 commented Jun 4, 2026

Summary

  • Route autopg auth through the v3 wrapper's pure-node dispatcher.
  • Prevent missing concrete static assets such as /app.js from falling back to index.html as text/html.
  • Add regression coverage for auth routing and module-asset MIME behavior.

Test Plan

  • bun test tests/console/no-cdn.test.js
  • bun test tests/cli-install.test.js --test-name-pattern 'autopg auth is routed'
  • bun test tests/console/auth.test.js
  • AUTOPG_CONFIG_DIR=$(mktemp -d) node bin/autopg-wrapper.cjs auth show-admin-path
  • added-line secret scan: clean

Notes

This targets autopg v3 (package.json version 3.0.7) on main.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Corrected static file serving to return proper 404 responses for missing assets with file extensions, preventing incorrect SPA fallback behavior
    • Improved admin authentication command routing through the CLI dispatcher
  • Tests

    • Added verification for admin authentication command routing
    • Strengthened static asset reachability validation

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1cf129e3-495f-491e-88ab-54ab430dc766

📥 Commits

Reviewing files that changed from the base of the PR and between be5b8a5 and 91cef8f.

📒 Files selected for processing (4)
  • bin/autopg-wrapper.cjs
  • src/cli-ui.cjs
  • tests/cli-install.test.js
  • tests/console/no-cdn.test.js

📝 Walkthrough

Walkthrough

The PR routes the autopg auth command through the pre-bun Node dispatcher and fixes static-asset fallback to return 404 for missing extension-based requests instead of serving index.html. Changes span CLI dispatch configuration, static handler logic, and corresponding test coverage.

Changes

Auth routing and static asset 404 fixes

Layer / File(s) Summary
CLI auth subcommand routing
bin/autopg-wrapper.cjs, tests/cli-install.test.js
The wrapper allowlist adds auth to pre-bun-dispatch verbs. A test verifies autopg auth show-admin-path routes successfully through the wrapper and outputs the expected admin.json path.
Static asset 404 for extension-based requests
src/cli-ui.cjs, tests/console/no-cdn.test.js
The static handler returns 404 (instead of falling back to index.html) when a request path has a file extension and the asset is missing. A strengthened test validates the /app.js response content-type and confirms the 404 fallback behavior.

Possibly related PRs

  • automagik-dev/autopg#123: Establishes/reworks the autopg wrapper dispatcher that this PR extends with the auth verb allowlist.
  • automagik-dev/autopg#137: Unifies compiled entry to dispatch install-style commands in-process, complementing this PR's wrapper-level routing of auth.
  • automagik-dev/autopg#92: Adds pgserve provision routing via the same pre-bun dispatcher mechanism used here for auth.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A rabbit hops through CLI verbs so clean,
Auth now routes where health checks convene,
While .js requests no longer pretend,
404 marks the SPA's true end!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: route autopg auth through wrapper' directly summarizes the main change: routing the autopg auth command through the wrapper's Node dispatcher, which is the primary focus of the PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/auth-subcommand-routing

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds the auth subcommand to the autopg-wrapper to bypass the bun probe, prevents SPA fallback for missing concrete assets with file extensions in cli-ui.cjs, and adds corresponding tests. A review comment points out that using path.extname(url) to block SPA fallback can break valid client-side routes containing dots (e.g., /users/john.doe). It suggests only blocking the fallback if the extension matches a known static asset type.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/cli-ui.cjs
Comment on lines +437 to +440
if (path.extname(url)) {
sendError(res, 404, 'NOT_FOUND', `no file at ${url}`);
return;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using path.extname(url) to prevent SPA fallback for any path with an extension can break client-side routing (SPA fallback) for valid routes that contain a dot in the last segment (e.g., /users/john.doe, /docs/v1.0, or /databases/my.db).

To avoid this, we should only prevent the SPA fallback if the extension matches a known static asset type defined in MIME_TYPES.

      const ext = path.extname(url).toLowerCase();
      if (ext && MIME_TYPES[ext]) {
        sendError(res, 404, 'NOT_FOUND', `no file at ${url}`);
        return;
      }

@namastex888 namastex888 merged commit 7e8d508 into main Jun 4, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants