Hashbox v0.8 #2
Hashbox v0.8 #2
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01faa76c7a
ℹ️ 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".
| srcTxs := store.ReadTrnFile(srcAccH, srcDatasetName) | ||
| var dstTxs []accountdb.DbTx | ||
|
|
||
| dstPath := store.DatasetFilepath(dstAccH, dstDatasetName) + accountdb.DbFileExtensionTransaction | ||
| if _, err := os.Stat(dstPath); err == nil { | ||
| dstTxs = store.ReadTrnFile(dstAccH, dstDatasetName) | ||
| renameIfExists(dstPath, dstPath+".bak") |
There was a problem hiding this comment.
If moveDataset is invoked with the same account+dataset for source and destination (e.g., user typo or scripting bug), srcTxs and dstTxs read the same .trn file, and the merge loop writes every transaction twice into the new log. That duplicates adds/deletes and can corrupt the dataset history. A no‑op guard when srcAccH == dstAccH and srcDatasetName == dstDatasetName (or skipping the merge in that case) would prevent accidental duplication.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 69 out of 77 changed files in this pull request and generated 11 comments.
Comments suppressed due to low confidence (1)
pkg/core/go.mod:3
- The
godirective should be a language version (e.g.go 1.24), not a patch version (go 1.24.12). Using a patch version can breakgotooling. If you need to pin a specific toolchain patch level, use atoolchain go1.24.12directive instead.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 70 out of 78 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| core.Log(core.LogInfo, "%s - Connection closed", remoteID) | ||
| conn.Close() | ||
| core.AbortOnError(conn.Close()) | ||
| }() |
There was a problem hiding this comment.
core.AbortOnError(conn.Close()) can panic inside this deferred cleanup, and that panic is not covered by the earlier recover() (since it runs after the recover() call). A close error would therefore crash the whole server. Prefer ignoring the close error or logging it (without panicking), e.g. if err := conn.Close(); err != nil { core.Log(...) }.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c5788c1d9
ℹ️ 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".
util/hashbox-util.go
Outdated
| if idx := strings.LastIndex(input, ":"); idx != -1 { | ||
| host = input[:idx] | ||
| p, err := strconv.Atoi(input[idx+1:]) | ||
| core.AbortOnError(err, "invalid port %q: %v", input[idx+1:], err) |
There was a problem hiding this comment.
The host parser treats any colon as a host/port separator. For IPv6 literals without an explicit port (e.g., ::1 or [::1]), strings.LastIndex splits the address and attempts to parse the tail as a port, which triggers AbortOnError and makes hashbox-util ping/sync unusable with default ports on IPv6. Consider using net.SplitHostPort (with bracket handling) or only parsing a port when the suffix is numeric and the host is not an unbracketed IPv6 literal.
Useful? React with 👍 / 👎.
Introduces hashbox-util for account/dataset/state management, sync for remote synchronization, Docker containerization, yearly retention, pkg refactor.
Major Changes:
Improvements and Fixes: