Skip to content

Fix/misc issues#235

Merged
matthewpeterkort merged 6 commits into
developmentfrom
fix/misc-issues
May 13, 2026
Merged

Fix/misc issues#235
matthewpeterkort merged 6 commits into
developmentfrom
fix/misc-issues

Conversation

@matthewpeterkort
Copy link
Copy Markdown
Contributor

No description provided.

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: c2a82a6338

ℹ️ 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 363 to +366
if len(s.existingByHash[oid]) == 0 {
return true, nil
}
obj := s.drsObjByOID[oid]
if obj == nil {
return false, nil
return false, nil
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Check downloadability before skipping uploads

needsUpload now returns false whenever any hash match exists, without verifying that the scoped object is actually downloadable. In the common recovery case where metadata exists but the backing blob is missing/unreadable (or the access URL is stale), push will skip upload and still proceed to git push, leaving refs that point to data that cannot be pulled later. The previous behavior probed downloadability before deciding to skip, so this is a regression in data availability guarantees.

Useful? React with 👍 / 👎.

Comment thread cmd/push/progress.go
Comment on lines +77 to +81
if ev.BytesSoFar > file.current {
file.current = ev.BytesSoFar
}
if ev.BytesSoFar > file.uploaded {
r.doneBytes += ev.BytesSoFar - file.uploaded
file.uploaded = ev.BytesSoFar
}
if ev.Path != "" {
r.currentLabel = ev.Path
} else if file.path != "" {
r.currentLabel = file.path
if ev.Phase == pushsync.UploadProgressUploading {
file.started = true
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 Guard progress renderer state for concurrent updates

BatchSyncForPush emits progress from multiple upload goroutines, but OnUploadProgress mutates shared renderer/file state without any synchronization. This introduces data races during parallel uploads (e.g., one goroutine rendering while another updates file.current/file.completed), which can produce inconsistent output and race-detector failures in CI.

Useful? React with 👍 / 👎.

@matthewpeterkort matthewpeterkort merged commit 17880eb into development May 13, 2026
6 checks passed
@matthewpeterkort matthewpeterkort deleted the fix/misc-issues branch May 13, 2026 00:53
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