fix: address PR #57 review findings — deadlocks, godoc, changelog refs - #58
Merged
Conversation
Fix three panic-deadlock bugs in WASM (non-deferred mutex unlocks in addFiles and AIA goroutine), an ERR-6 fall-through after json.Marshal failure, a godoc mis-attachment on VerifyChainTrust, and three changelog entries referencing a non-existent commit SHA. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses four specific issues identified during the review of PR #57, focusing on critical concurrency bugs, error handling, documentation, and changelog maintenance.
Changes:
- Fix three panic-induced deadlock bugs in WASM by replacing manual
storeMu.Unlock()calls withdeferstatements - Fix ERR-6 violation by adding early return when
json.Marshalfails in AIA goroutine - Fix API-1 violation by repositioning
VerifyChainTrustInputstruct beforeVerifyChainTrustfunction for proper godoc attachment - Fix CL-3/CL-4 violations by replacing non-existent commit references with PR references in changelog
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cmd/wasm/main.go | Adds defer storeMu.Unlock() to prevent deadlocks on panic in addFiles main goroutine and AIA goroutine; adds early return after Marshal error |
| bundle.go | Moves VerifyChainTrustInput struct definition before VerifyChainTrust function to ensure godoc attaches correctly |
| CHANGELOG.md | Replaces three references to non-existent commit b5969b0 with [#57] PR reference; adds four new entries documenting the fixes in this PR; updates reference links section |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
This was referenced Feb 24, 2026
This was referenced Mar 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
addFilesand AIA goroutine used manualstoreMu.Unlock()instead ofdefer, meaning any panic permanently deadlocks the storejson.Marshalfailure in AIA goroutine fell through toonCompletecallback with nil JSON instead of returning earlyVerifyChainTrustgodoc comment was contiguous with the struct comment, so Go attached it toVerifyChainTrustInput— function had no godocb5969b0— replaced with[#57]PR refAddresses all unresolved review comments from #57.
Test plan
go build ./...passesgo test -race ./...passesGOOS=js GOARCH=wasm go vet ./cmd/wasm/passesGOOS=js GOARCH=wasm go build ./cmd/wasm/passes🤖 Generated with Claude Code