Refs #426: Document auth session flow#534
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds agent guide documentation for checking the current authentication session ( ChangesAuthentication Session Documentation
Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
weilixiong
left a comment
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
📒 Files selected for processing (3)
docs/agent-guide.mdscripts/docs_smoke.pytests/test_docs_public_urls.py
adliebe
left a comment
There was a problem hiding this comment.
Reviewed current head 991369bd54fc6b7c2b7de5e43c370421c9ccee3f as a non-author.
Scope checked:
docs/agent-guide.mdadds the session check/logout guidance forGET /api/v1/auth/me,POST /auth/logout, redirect behavior, cookie clearing, and the warning againstGET /auth/logoutside effects.scripts/docs_smoke.pynow requires the auth/session endpoint text and theGET /auth/logoutwarning.tests/test_docs_public_urls.pyadds 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 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_docs_public_urls.py -q->24 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python scripts/docs_smoke.py->docs smoke okuv run --extra dev ruff check docs/agent-guide.md scripts/docs_smoke.py tests/test_docs_public_urls.py-> passeduv run --extra dev ruff format --check scripts/docs_smoke.py tests/test_docs_public_urls.py-> passedgit diff --check origin/main...HEAD-> clean- GitHub check
Quality, readiness, docs, and image checksis 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 guideI 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.
adliebe
left a comment
There was a problem hiding this comment.
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_flownow asserts bothredirects to \/`andclears the MergeWork auth cookies`.scripts/docs_smoke.pynow protects the agent-guide auth/session text, includingclears 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 againstGET /auth/logoutside 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 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_docs_public_urls.py -q->24 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python scripts/docs_smoke.py->docs smoke okuv run --extra dev ruff check docs/agent-guide.md scripts/docs_smoke.py tests/test_docs_public_urls.py-> passeduv run --extra dev ruff format --check scripts/docs_smoke.py tests/test_docs_public_urls.py-> passedgit diff --check origin/main...HEAD-> clean- GitHub
Quality, readiness, docs, and image checks-> success
Approved.
|
Reviewed PR #534 at current head No blocker found. The follow-up commit resolves the earlier docs-contract gap: the agent guide still documents Validation run on this head:
GitHub readback shows the PR is open, non-draft, |
Refs #426
Summary
docs/agent-guide.mdEvidence
app/main.pyregistersGET /api/v1/auth/me, returningauthenticatedandgithub_loginfrom the signed session cookie.app/auth.pyregistersPOST /auth/logout, deletesmrwk_userandmrwk_admin, and redirects to/.GET /auth/github/callback?code=dummy&state=dummyreturns401 {"detail":"invalid OAuth state"},POST /auth/logoutreturns303 Location: /with expired auth cookies, andGET /auth/logoutreturns405with no logout cookie side effects.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 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python scripts/docs_smoke.py-> docs smoke okPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_docs_public_urls.py -q-> 24 passeduv run --extra dev ruff check scripts/docs_smoke.py tests/test_docs_public_urls.py-> passeduv run --extra dev ruff format --check scripts/docs_smoke.py tests/test_docs_public_urls.py-> passedgit diff --check-> cleanNo 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