Skip to content

[WIP] Improve runtime safety and user experience in P2P market data#4

Merged
danielsobrado merged 1 commit into
mainfrom
copilot/improve-runtime-safety-correctness
Mar 27, 2026
Merged

[WIP] Improve runtime safety and user experience in P2P market data#4
danielsobrado merged 1 commit into
mainfrom
copilot/improve-runtime-safety-correctness

Conversation

Copilot AI commented Mar 27, 2026

Copy link
Copy Markdown
Contributor
  • A. Script execution lifecycle hardening
    • Refactor StopScript to avoid double-Wait() with graceful-stop → timeout → force-kill
    • Change findPythonPath to return (string, error) for early failure
    • Improve ExecuteScriptWithOutputCapture failure handling
    • Fix ValidateDependencies to write temp files outside script dir
    • Add tests: stop running/exited script, missing Python, malformed output
  • B. P2P correctness fixes
    • Fix connection.go connection count drift (idempotent connect/disconnect)
    • Fix validator.go Select() all-invalid case returns error
    • Fix message.go MarshalWithoutSignature() to include integrity fields
    • Fix host.go IsRunning() thread safety
    • Add tests: connection accounting, all-invalid selection
  • C. Migration/tooling safety
    • Fix proto_to_sql.py cleanup bug (guard schema_manager.conn.close())
    • Add identifier quoting/validation in SQL generation
  • D. Frontend script UX
    • Complete script action handlers with state refresh callbacks
    • Fix handleViewCode to surface fetched content
Original 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:

  • script execution lifecycle hardening
  • P2P correctness fixes
  • migration/tooling safety improvements
  • frontend script UX consistency

Scope

Relevant files likely include:

  • pkg/scripts/executor.go
  • pkg/scripts/manager.go
  • pkg/peer/connection.go
  • pkg/peer/discovery/validator.go
  • pkg/p2p/message/message.go
  • pkg/p2p/host/host.go
  • scripts/proto_to_sql.py
  • frontend/src/components/data/utils/handleScripts.ts
  • frontend/src/App.tsx
  • related tests

Required changes

A. Script execution lifecycle hardening

  1. Refactor script process shutdown so process reaping is reliable.

    • Avoid fragile/double-Wait() behavior.
    • Ensure stopping a script does not hang indefinitely.
    • Implement a clear graceful-stop -> timeout -> force-kill flow if needed.
  2. Improve Python interpreter handling.

    • Missing Python should fail clearly and early rather than returning an empty path that fails later.
  3. Improve structured output parsing robustness.

    • If ExecuteScriptWithOutputCapture depends on JSON output, make failure modes explicit and well-handled.
  4. Stop writing dependency-validation temporary files into the live script directory.

    • Use temp files/dirs appropriately.
  5. Add/update tests for the most important script lifecycle behaviors.

    • stopping running scripts
    • stopping already-exited scripts
    • missing Python behavior
    • malformed structured output behavior

B. P2P correctness fixes

  1. Fix peer connection/accounting logic so connection counts cannot drift from actual active connections.

    • Prefer deriving count from real state or only mutating counters on true state transitions.
  2. Fix DHT validator record selection behavior.

    • If all candidate records are invalid, selection should return an error instead of silently defaulting to index 0.
  3. Improve message signing/verification integrity.

    • Replace or refactor the current signed payload behavior so integrity-sensitive fields are covered consistently.
    • Avoid brittle signing semantics around partially signed message fields.
  4. Address obvious thread-safety/config-validation issues in host lifecycle code where practical.

    • e.g. running-state access and incomplete validation/TODOs.
  5. Add/update focused tests for the key corrected behaviors.

C. Migration/tooling safety

  1. Fix the cleanup bug in scripts/proto_to_sql.py so partially initialized objects do not cause secondary exceptions in cleanup.
  2. Improve migration SQL generation safety where feasible.
    • Validate/quote identifiers appropriately.
    • Prefer safer behavior for malformed schema/proto inputs.
  3. Add/update tests if the repo has an existing place for these, or implement lightweight validation coverage where practical.

D. Frontend script UX consistency

  1. Complete script action handlers so frontend state refreshes appropriately after upload/run/stop/delete/install/uninstall actions.
  2. Ensure fetched script content is actually surfaced/used appropriately or remove dead/incomplete behavior.
  3. Improve connection status updates where feasible.
    • Prefer event-driven or more consistent state behavior over simplistic stale polling, while keeping scope reasonable.

Acceptance criteria

  • Remaining identified high/medium-priority issues are addressed within this single PR.
  • Script stop/shutdown behavior is more reliable and better tested.
  • P2P connection and validator correctness issues are fixed.
  • Message integrity/signing behavior is more coherent.
  • Migration cleanup bug is fixed and migration generation is safer.
  • Frontend script actions update UI state more consistently.
  • Tests are added or updated for critical corrected behaviors.

Notes

  • Keep this PR focused on the remaining fixes only; do not rework the startup/config PR scope.
  • Preserve behavior where reasonable, but prioritize correctness, explicit failure modes, and maintainability.

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=code

High-level impression

The repo looks like a Wails desktop app with:

  • a Go backend (main.go, app.go, pkg/...)
  • a React/TypeScript frontend (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.

@danielsobrado danielsobrado marked this pull request as ready for review March 27, 2026 06:29
Copilot AI review requested due to automatic review settings March 27, 2026 06:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

@danielsobrado danielsobrado merged commit eebc9c5 into main Mar 27, 2026
1 of 2 checks passed
Copilot stopped work on behalf of danielsobrado due to an error March 27, 2026 06:30
Copilot AI requested a review from danielsobrado March 27, 2026 06:30
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.

3 participants