Skip to content

[codex] harden Synapse upload result coverage#20

Merged
snissn merged 1 commit into
mainfrom
codex/synapse-v1-final-review
Jun 12, 2026
Merged

[codex] harden Synapse upload result coverage#20
snissn merged 1 commit into
mainfrom
codex/synapse-v1-final-review

Conversation

@snissn

@snissn snissn commented Jun 12, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up after reviewing the merged Synapse v1 upgrade and #19 again. This tightens the high-level upload result mapping and closes the remaining test coverage gap around Synapse v1 UploadResult.complete semantics.

Findings Fixed

  • copyResults now converts dataSetId, pieceId, and providerId to strings before returning the command payload, matching the declared output schema directly instead of relying on later deep serialization.
  • The upload command tests now cover complete: true with a recovered failed attempt, matching Synapse v1's documented rule that failedAttempts can be non-empty without making the upload incomplete.
  • A separate regression test now covers complete: false / partially_uploaded when fewer copies are committed than requested.

Mergeability Evidence

  • Scope: cli/src/commands/upload.ts and cli/tests/synapse-commands.test.ts.
  • Linked work: follow-up to [codex] fix Synapse v1 follow-up review findings #19 / Synapse SDK v1 issue graph Upgrade foc-cli to Synapse SDK v1 #12-Run calibration smoke validation for Synapse SDK v1 foc-cli #16.
  • Internal deep review: no remaining wallet summary, SDK API drift, output schema, or upload status concerns found after this patch.
  • Performance: not performance-sensitive; command output shaping and tests only.
  • Latest head: 8c96b54ad51350b93ec58dffaf8627c3bf5c5504.
  • CI: GitHub Actions CLI passed on the latest head.
  • AI review: @codex review, @copilot review, and @coderabbitai review were requested after the PR was mature; no review comments or unresolved threads were returned before merge.
  • Known caveats: none.

Validation

  • bun test tests/synapse-commands.test.ts
  • bun test
  • bunx tsc --noEmit
  • bunx biome check src tests
  • bun run build
  • git diff --check

@snissn

snissn commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

@codex review

@snissn

snissn commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

@copilot review

@snissn

snissn commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 8c96b54ad5

ℹ️ 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".

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