[WIP] Improve runtime safety and user experience in P2P market data#4
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot stopped work on behalf of
danielsobrado due to an error
March 27, 2026 06:30
danielsobrado
approved these changes
Mar 27, 2026
danielsobrado
approved these changes
Mar 27, 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.
StopScriptto avoid double-Wait()with graceful-stop → timeout → force-killfindPythonPathto return(string, error)for early failureExecuteScriptWithOutputCapturefailure handlingValidateDependenciesto write temp files outside script dirconnection.goconnection count drift (idempotent connect/disconnect)validator.goSelect()all-invalid case returns errormessage.goMarshalWithoutSignature()to include integrity fieldshost.goIsRunning()thread safetyproto_to_sql.pycleanup bug (guardschema_manager.conn.close())handleViewCodeto surface fetched contentOriginal prompt
Create the second of two pull requests for repository
danielsobrado/P2P-Market-Data.This PR should cover all remaining recommended fixes after the startup/config/error-handling PR. Consolidate the remaining work into this single PR so that the 2-PR plan covers everything material previously identified.
Goals
Improve runtime safety, correctness, and UX in the remaining high/medium-priority areas:
Scope
Relevant files likely include:
pkg/scripts/executor.gopkg/scripts/manager.gopkg/peer/connection.gopkg/peer/discovery/validator.gopkg/p2p/message/message.gopkg/p2p/host/host.goscripts/proto_to_sql.pyfrontend/src/components/data/utils/handleScripts.tsfrontend/src/App.tsxRequired changes
A. Script execution lifecycle hardening
Refactor script process shutdown so process reaping is reliable.
Wait()behavior.Improve Python interpreter handling.
Improve structured output parsing robustness.
ExecuteScriptWithOutputCapturedepends on JSON output, make failure modes explicit and well-handled.Stop writing dependency-validation temporary files into the live script directory.
Add/update tests for the most important script lifecycle behaviors.
B. P2P correctness fixes
Fix peer connection/accounting logic so connection counts cannot drift from actual active connections.
Fix DHT validator record selection behavior.
Improve message signing/verification integrity.
Address obvious thread-safety/config-validation issues in host lifecycle code where practical.
Add/update focused tests for the key corrected behaviors.
C. Migration/tooling safety
scripts/proto_to_sql.pyso partially initialized objects do not cause secondary exceptions in cleanup.D. Frontend script UX consistency
Acceptance criteria
Notes
The following is the prior conversation context from the user's chat exploration (may be truncated):
User: Code review
Assistant: GitHub Copilot Chat Assistant here.
Here’s a focused code review for
danielsobrado/P2P-Market-Data.Important: I could only inspect a limited subset of files/results, so this review may be incomplete. To inspect more code in GitHub’s UI, use code search in the repo:
https://github.com/search?q=repo%3Adanielsobrado%2FP2P-Market-Data&type=codeHigh-level impression
The repo looks like a Wails desktop app with:
main.go,app.go,pkg/...)frontend/...)This pull request was created from Copilot chat.
⚡ Quickly spin up Copilot coding agent tasks from anywhere on your macOS or Windows machine with Raycast.