fix(buck): improve Complete API with optional body and docs

[VansamaYD]

• Make Complete request body optional (Option<Json>)
• Update rustdoc to reflect commit_message from session
• Add regression tests for commit_message preservation

link #1926

[genedna]

@claude

[github-actions]

Claude finished @genedna's task in 2m 12s —— View job

---

Code Review Complete ✓

• Read repository guidelines (CLAUDE.md)
• Review changed files and understand modifications
• Analyze code quality and best practices
• Check security considerations
• Evaluate test coverage
• Provide comprehensive feedback

---

Summary

This PR improves the Complete API by making the request body optional and enhancing documentation. The changes build logically on PR #1929, which established session.commit_message as the single source of truth. Overall, the implementation is sound and well-tested, with good regression test coverage.

✅ Strengths

1. Clear Intent: Making the body optional aligns perfectly with the fact that CompletePayload is now empty and unused
2. Good Documentation: The rustdoc updates clearly explain the data flow (commit message comes from session, not payload)
3. Excellent Test Coverage: Regression tests verify both session.commit_message preservation and CL title usage
4. API Consistency: The change makes the API more intuitive - why require a body if it's ignored?
5. Backward Compatibility: The Option<Json<CompletePayload>> with unwrap_or fallback maintains backward compatibility with existing clients

⚠️ Areas for Improvement

1. Client Code Synchronization (Critical)

The TypeScript client at moon/packages/types/generated.ts:3984-3986 still has the OLD definition:

export type CompletePayload = {
  /** Optional commit message (overrides manifest message) */
  commit_message?: string | null
}

This is outdated and misleading. The generated types need to be regenerated from the updated OpenAPI spec.

Action needed: Regenerate TypeScript client types after this PR merges to reflect:

• Empty CompletePayload struct
• Optional request body in the complete endpoint

2. Code Improvements (See inline comments)

• Consider using unwrap_or_default() instead of unwrap_or(CompletePayload {}) in buck_router.rs:264
• OpenAPI doc could be more explicit about body being completely omittable

3. Test Enhancement (Minor)

Current tests verify service-layer behavior. Consider adding an HTTP-layer test that explicitly passes None to verify the Option<Json<CompletePayload>> handling at the router level.

🔒 Security

No security concerns identified. The change actually improves security posture by:

• Removing an unused parameter that could confuse future developers
• Enforcing single source of truth for commit messages (session store)
• Preventing potential override attacks via payload injection

📊 Code Quality

Excellent - The code is:

• Well-documented with clear rustdoc comments
• Properly tested with regression tests
• Consistent with the established pattern (PR fix(buck): remove redundant commit_message from Complete API #1929)
• Idiomatic Rust with appropriate error handling

🎯 Recommendation

Approve with minor suggestions. The core changes are solid. The inline comments suggest small improvements, but none are blocking. The only critical follow-up is regenerating the TypeScript client types.

---

Next Steps:

1. Address inline comments (optional improvements)
2. After merge: Regenerate moon/packages/types/generated.ts from updated OpenAPI spec
3. Consider adding HTTP-layer test for optional body handling

---

[github-actions]

Potential issue: Using unwrap_or(CompletePayload {}) could be problematic if CompletePayload gains fields in the future.

Consider using unwrap_or_default() instead:

Suggested change

	let payload = payload.map(|p| p.0).unwrap_or(CompletePayload {});
	let payload = payload.map(|p| p.0).unwrap_or_default();

This requires implementing Default for CompletePayload, which would be more maintainable and conventional in Rust.

[github-actions]

Documentation clarity: The OpenAPI documentation states "Can be omitted or sent as {}", but it might be clearer to explicitly state that the body can be completely omitted (no Content-Type header needed).

Consider updating to:

Suggested change

	request_body(
	description = "Optional empty payload. Request body can be completely omitted or sent as {} with application/json Content-Type",

[github-actions]

Good addition: These regression tests are excellent! They verify the core behavior that was broken in the previous version.

Minor suggestion: Consider adding a test case that explicitly calls the endpoint with None payload to verify the optional parameter behavior at the HTTP layer, not just the service layer.

[Copilot]

Pull request overview

This pull request addresses issue #1926 by eliminating redundant commit message handling in the Buck upload API. The Complete endpoint now treats its request body as optional and reads the commit message from the session (which is set during the Manifest phase), rather than accepting it redundantly in both phases.

Changes:

• Made Complete API request body optional, with commit message now exclusively sourced from session state set during Manifest phase
• Updated API documentation and parameter naming to reflect that payload is intentionally unused
• Added regression tests to verify commit_message preservation across the upload workflow

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
mono/src/api/router/buck_router.rs	Changed complete_upload handler to accept optional payload (Option<Json>); updated utoipa documentation to reflect optional body
jupiter/src/service/buck_service.rs	Updated rustdoc to clarify that commit_message is read from session; renamed payload parameter to _payload to indicate it's unused
ceres/src/api_service/mono_api_service.rs	Updated rustdoc consistently with jupiter changes; renamed payload parameter to _payload
mono/tests/buck_service_tests.rs	Added regression test assertions verifying session.commit_message is preserved and used for CL title

[Copilot]

The utoipa request_body syntax for optional payloads is incorrect. In utoipa 5.x, content = Option<CompletePayload> is not valid syntax.

For optional request bodies, you should either:

1. Simply use request_body = CompletePayload in the utoipa annotation (which documents the schema without enforcing it's required), while the actual handler uses Option<Json<CompletePayload>> to make it optional at runtime
2. Or omit the request_body attribute entirely if the body is fully optional and empty

The current syntax will likely cause OpenAPI schema generation issues. The recommended approach is to change line 245-249 to:

request_body = CompletePayload,

And update the description comment to note that the body is optional and can be omitted or sent as {}.