Skip to content

Refs #426: Document auth session flow#534

Open
Juanpablo24-06 wants to merge 2 commits into
ramimbo:mainfrom
Juanpablo24-06:codex/b426-auth-session-docs
Open

Refs #426: Document auth session flow#534
Juanpablo24-06 wants to merge 2 commits into
ramimbo:mainfrom
Juanpablo24-06:codex/b426-auth-session-docs

Conversation

@Juanpablo24-06
Copy link
Copy Markdown

@Juanpablo24-06 Juanpablo24-06 commented May 27, 2026

Refs #426

Summary

  • document the GitHub session check/logout boundaries in docs/agent-guide.md
  • add docs smoke coverage so the agent guide keeps the auth/session guidance
  • add a focused docs regression for the session flow text

Evidence

  • app/main.py registers GET /api/v1/auth/me, returning authenticated and github_login from the signed session cookie.
  • app/auth.py registers POST /auth/logout, deletes mrwk_user and mrwk_admin, and redirects to /.
  • Live unauthenticated smoke at 2026-05-27 UTC showed GET /auth/github/callback?code=dummy&state=dummy returns 401 {"detail":"invalid OAuth state"}, POST /auth/logout returns 303 Location: / with expired auth cookies, and GET /auth/logout returns 405 with no logout cookie side effects.
  • This is distinct from PR docs: document GET /api/v1/auth/me endpoint in agent guide #504, which only lists GET /api/v1/auth/me; this PR documents the end-to-end browser session boundary including logout behavior and guards it in docs smoke.

Validation

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_docs_public_urls.py::test_agent_guide_documents_auth_session_flow -q -> 1 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_docs_public_urls.py -q -> 24 passed
  • uv run --extra dev ruff check scripts/docs_smoke.py tests/test_docs_public_urls.py -> passed
  • uv run --extra dev ruff format --check scripts/docs_smoke.py tests/test_docs_public_urls.py -> passed
  • git diff --check -> clean

No cookies, OAuth callback codes, tokens, wallet material, private keys, signatures, private data, price claims, exchange claims, bridge claims, liquidity claims, or fabricated payout claims are included.

Summary by CodeRabbit

  • Documentation
    • Clarified how to check current auth session via GET /api/v1/auth/me (including unauthenticated response) and how to end a browser session via POST /auth/logout (redirects to / and clears auth cookies); added a warning not to rely on GET /auth/logout for logout side effects.
  • Tests
    • Added a documentation smoke test to verify the above session and logout guidance is present.
  • Chores
    • Updated doc validation to require those public-facing phrases.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ef308713-a900-4d3f-930c-2158177dc2eb

📥 Commits

Reviewing files that changed from the base of the PR and between 991369b and 7535949.

📒 Files selected for processing (3)
  • docs/agent-guide.md
  • scripts/docs_smoke.py
  • tests/test_docs_public_urls.py

📝 Walkthrough

Walkthrough

This PR adds agent guide documentation for checking the current authentication session (GET /api/v1/auth/me) and ending browser sessions (POST /auth/logout), extends the smoke-check to require those phrases, and adds a test that asserts the agent guide contains the documented auth-flow strings.

Changes

Authentication Session Documentation

Layer / File(s) Summary
Authentication session flow documentation
docs/agent-guide.md
Documents the /api/v1/auth/me endpoint for checking session status with unauthenticated response shape, and POST /auth/logout for ending browser sessions (redirects to / and clears cookies); cautions against using GET /auth/logout for logout side effects.
Documentation validation rules and tests
scripts/docs_smoke.py, tests/test_docs_public_urls.py
Smoke script extends REQUIRED_PUBLIC_PHRASES["docs/agent-guide.md"] with session-check and logout guidance phrases; new test reads docs/agent-guide.md and asserts presence of the session-check endpoint, unauthenticated response example, logout guidance, and the caution against GET /auth/logout.

Possibly related PRs

  • ramimbo/mergework#431: Also modified docs/agent-guide.md and extended scripts/docs_smoke.py validation for authentication-session-related documentation.
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title directly names the changed surface and main objective: documenting auth session flow.
Description check ✅ Passed The description follows the template structure with Summary, Evidence, and validation steps clearly documented.
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.
Mergework Public Artifact Hygiene ✅ Passed PR changes document authentication/authorization flows only. No investment, price, cash-out, fabricated payout claims, or private security details found.
Bounty Pr Focus ✅ Passed PR scope is focused: 3 files modified exclusively for #426 auth session documentation (docs, smoke test enforcement, regression test); no unrelated changes or credential leaks detected.

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


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

Copy link
Copy Markdown

@weilixiong weilixiong left a comment

Choose a reason for hiding this comment

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

QUALITY_GATE: LOW risk ✅

3 files, +16/-0: Documents the auth session flow (GET /api/v1/auth/me, POST /auth/logout) in agent-guide.md, adds 2 docs_smoke phrase checks, and adds a new test asserting 5 phrases in guide.

  • docs/agent-guide.md: +4/-0 (auth/me endpoint, logout flow, cookie clearing)
  • scripts/docs_smoke.py: +2/-0 (2 new REQUIRED_PUBLIC_PHRASES entries)
  • tests/test_docs_public_urls.py: +10/-0 (new test asserting 5 phrases)

Verification: pytest tests/test_docs_public_urls.py::test_agent_guide_documents_auth_session_flow → PASSED. docs_smoke.py → ok (RC=0).

Docs-only change, no functional code. No unrelated files.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4204c400-f353-4fed-8ea8-110177b7b0e3

📥 Commits

Reviewing files that changed from the base of the PR and between d8532d4 and 991369b.

📒 Files selected for processing (3)
  • docs/agent-guide.md
  • scripts/docs_smoke.py
  • tests/test_docs_public_urls.py

Comment thread tests/test_docs_public_urls.py
Copy link
Copy Markdown

@adliebe adliebe left a comment

Choose a reason for hiding this comment

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

Reviewed current head 991369bd54fc6b7c2b7de5e43c370421c9ccee3f as a non-author.

Scope checked:

  • docs/agent-guide.md adds the session check/logout guidance for GET /api/v1/auth/me, POST /auth/logout, redirect behavior, cookie clearing, and the warning against GET /auth/logout side effects.
  • scripts/docs_smoke.py now requires the auth/session endpoint text and the GET /auth/logout warning.
  • tests/test_docs_public_urls.py adds focused coverage for the new agent-guide auth/session section.

Validation:

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_docs_public_urls.py::test_agent_guide_documents_auth_session_flow -q -> 1 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_docs_public_urls.py -q -> 24 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
  • uv run --extra dev ruff check docs/agent-guide.md scripts/docs_smoke.py tests/test_docs_public_urls.py -> passed
  • uv run --extra dev ruff format --check scripts/docs_smoke.py tests/test_docs_public_urls.py -> passed
  • git diff --check origin/main...HEAD -> clean
  • GitHub check Quality, readiness, docs, and image checks is successful.

Requested change: please lock the full new logout contract into the docs tests. The guide now says POST /auth/logout redirects to / and clears the MergeWork auth cookies, but test_agent_guide_documents_auth_session_flow() only asserts that POST /auth/logout is mentioned plus the GET /auth/logout warning. That means the test would still pass if the redirect/cookie-clearing behavior disappears from the guide while the endpoint names remain.

Suggested minimum coverage:

assert "redirects to `/`" in guide
assert "clears the MergeWork auth cookies" in guide

I would also add at least one of those phrases to REQUIRED_PUBLIC_PHRASES["docs/agent-guide.md"] so scripts/docs_smoke.py protects the same safety boundary. The docs change is otherwise tight and correctly avoids cookies, tokens, private keys, payout claims, or off-ramp claims.

Copy link
Copy Markdown

@adliebe adliebe left a comment

Choose a reason for hiding this comment

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

Reviewed current head 7535949598be8fcb8647c532c580ea90c376f102 after the author follow-up.

The requested docs-contract gap is resolved:

  • tests/test_docs_public_urls.py::test_agent_guide_documents_auth_session_flow now asserts both redirects to \/`andclears the MergeWork auth cookies`.
  • scripts/docs_smoke.py now protects the agent-guide auth/session text, including clears the MergeWork auth cookies.
  • The guide still documents GET /api/v1/auth/me, unauthenticated response shape, POST /auth/logout, redirect behavior, cookie clearing, and the warning against GET /auth/logout side effects.

Validation:

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_docs_public_urls.py::test_agent_guide_documents_auth_session_flow -q -> 1 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_docs_public_urls.py -q -> 24 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
  • uv run --extra dev ruff check docs/agent-guide.md scripts/docs_smoke.py tests/test_docs_public_urls.py -> passed
  • uv run --extra dev ruff format --check scripts/docs_smoke.py tests/test_docs_public_urls.py -> passed
  • git diff --check origin/main...HEAD -> clean
  • GitHub Quality, readiness, docs, and image checks -> success

Approved.

@tinyopsstudio
Copy link
Copy Markdown

Reviewed PR #534 at current head 7535949598be8fcb8647c532c580ea90c376f102.

No blocker found. The follow-up commit resolves the earlier docs-contract gap: the agent guide still documents GET /api/v1/auth/me, unauthenticated response shape, POST /auth/logout, redirect behavior, cookie clearing, and the GET /auth/logout warning; tests/test_docs_public_urls.py::test_agent_guide_documents_auth_session_flow now asserts both redirects to / and clears the MergeWork auth cookies, and scripts/docs_smoke.py guards the cookie-clearing phrase.

Validation run on this head:

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_docs_public_urls.py::test_agent_guide_documents_auth_session_flow tests/test_docs_public_urls.py -q -> 24 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python scripts/docs_smoke.py -> docs smoke ok
  • ./.venv/bin/python -m ruff check scripts/docs_smoke.py tests/test_docs_public_urls.py -> passed
  • ./.venv/bin/python -m ruff format --check scripts/docs_smoke.py tests/test_docs_public_urls.py -> 2 files already formatted
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m mypy app scripts/docs_smoke.py -> success
  • git diff --check origin/main...HEAD -> clean

GitHub readback shows the PR is open, non-draft, CLEAN, and the quality/readiness workflow is successful.

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.

4 participants