Skip to content

docs+milestone updates: finish milestone 2/3 and auth logout#34

Open
wauputr4 wants to merge 9 commits into
mainfrom
codex-phase-1-checkpoint
Open

docs+milestone updates: finish milestone 2/3 and auth logout#34
wauputr4 wants to merge 9 commits into
mainfrom
codex-phase-1-checkpoint

Conversation

@wauputr4
Copy link
Copy Markdown
Contributor

Summary

  • Marked Milestone 2 database foundation as complete (migration coverage + idempotent behavior already validated).
  • Marked Milestone 3 auth/API-keys milestone as complete.
  • Added POST /auth/logout endpoint for session revocation.
  • Kept implementation and docs in milestone-aligned crates (crates/mizan-api/src/auth.rs, crates/mizan-api/src/lib.rs, docs/BACKEND_IMPLEMENTATION_PLAN.md, docs/ISSUE_BACKLOG.md).
  • Validation:
    • cargo test --workspace -- --nocapture pass.
    • Local smoke flow checked: register/login/logout and token invalidation after logout.

References

  • Mizan roadmap milestones:
    • Issue 2 (database foundation)
    • Issue 3 (auth + API keys)

GitHub issues for next milestones

  • P5: Implement streaming chat completions gateway
  • P5: Normalize upstream errors and provider health state
  • P6: Record usage events from completed gateway requests
  • P6: Add balance safety and token estimation fallback
  • P6: Implement ledger-first credit grants and usage charges
  • P7: Implement Redis RPM, TPM, and concurrency primitives
  • P7: Enforce insufficient-credit and limit error responses
  • P8: Add Prometheus metrics endpoint
  • P8: Harden structured tracing and log redaction

@wauputr4 wauputr4 marked this pull request as ready for review May 11, 2026 08:56
@wauputr4
Copy link
Copy Markdown
Contributor Author

@codex review

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 implements the logout functionality by adding a new /auth/logout endpoint and its corresponding handler in auth.rs. It also updates the project documentation and README to reflect the completion of Milestones 2 and 3. Feedback was provided regarding the logout handler's response behavior; it is recommended to return a 401 Unauthorized status instead of a 200 OK when a session token is invalid or already revoked to maintain consistency with other authenticated endpoints.

Comment thread crates/mizan-api/src/auth.rs Outdated
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@wauputr4
Copy link
Copy Markdown
Contributor Author

/gemini review

@wauputr4
Copy link
Copy Markdown
Contributor Author

@codex review

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 implements the logout functionality, adding a new API route and database logic to revoke session tokens. It also updates the project documentation and README to reflect the completion of Milestones 2 and 3. Feedback suggests removing hardcoded commit hashes and ephemeral phrases from the documentation to improve maintainability. Additionally, it is recommended to make the logout operation idempotent by returning a success status even if the session has already been revoked, rather than a 401 Unauthorized error.

Comment thread docs/BACKEND_IMPLEMENTATION_PLAN.md Outdated
Comment thread docs/BACKEND_IMPLEMENTATION_PLAN.md Outdated
Comment thread docs/ISSUE_BACKLOG.md Outdated
Comment thread docs/ISSUE_BACKLOG.md Outdated
Comment thread crates/mizan-api/src/auth.rs Outdated
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 64963c0d24

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/mizan-api/src/gateway.rs Outdated
Comment thread crates/mizan-api/src/gateway.rs Outdated
Copy link
Copy Markdown
Contributor Author

Update: sudah saya implement perbaikan review yang tersisa.

  • gateway: header parser sekarang toleran untuk x-request-id/x-trace-id yang non-UUID dan tetap generate UUID server.
  • gateway: response error kini dibangun via helper agar x-request-id + x-trace-id tetap ikut di error path.
  • auth: POST /auth/logout jadi idempotent (revoked: false jika sesi sudah revoked/tidak ketemu, sukses return tetap 200).
  • docs: hapus hardcoded commit hash / frase this pass di status milestone.

Commit: bcb860d.

Silakan cek review threads; revisi baru sudah di-push sehingga thread lama seharusnya jadi outdated/resolved by context.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2513479da

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/mizan-api/src/gateway.rs Outdated
Comment thread crates/mizan-providers/src/lib.rs Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 60d826bf37

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/mizan-providers/src/lib.rs
Implements PR #34 follow-ups for true end-to-end SSE forwarding and multi-line SSE data parsing. Covers #35 and #36.
@wauputr4
Copy link
Copy Markdown
Contributor Author

Update: implemented the PR #34 streaming review follow-ups in aba5455.

Verification is green locally and in CI:

  • cargo fmt
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace
  • smoke GET /healthz on 127.0.0.1:18281
  • GitHub Actions CI run 25699537547: success

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aba5455e67

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/mizan-api/src/gateway.rs Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7db7f1c070

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +255 to +261
let mut content = String::new();
if let Some(first_choice) = response.choices.into_iter().next() {
content = first_choice
.message
.and_then(|message| message.content)
.unwrap_or_default();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject chat completions that contain no choices

If the upstream returns HTTP 200 with an empty choices array, this parser falls back to content = "" and the gateway emits a normal successful completion instead of surfacing a provider error. In that scenario clients receive a false-success response (empty assistant message) and cannot distinguish malformed upstream output from a real completion, which can silently corrupt downstream behavior and accounting.

Useful? React with 👍 / 👎.

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.

1 participant