[codex] Harden v1 release readiness by SweetingTech · Pull Request #50 · SweetingTech/TableTop_DM

SweetingTech · 2026-05-20T04:25:34Z

Summary

add an explicit Docker reset path for disposable migration checksum drift recovery
clean up migration tracking with 017_schema_migration_cleanup.sql
make Windows start/stop handling more robust around stale Git Bash PIDs
speed up player-state snapshot generation by reusing one DB connection per snapshot
tighten filesystem release packaging and refresh the v1.0.0 artifact/checksum
document MCP runtime integrations as post-1.0 roadmap work

Validation

./scripts/start.ps1 -Mode docker -ResetDb
./scripts/verify_boot.ps1 -Mode docker
Python compile checks
JavaScript syntax checks
pytest tests/services tests/contracts tests/integration -vv → 164 passed
pytest tests/e2e -rs -vv → 19 passed
./scripts/package_release.ps1 -Version v1.0.0 -Mode filesystem
extracted release/TableTopDM-v1.0.0.zip and ran its scripts/verify_boot.ps1 -Mode docker successfully

Notes

Leaves existing untracked local files (codex_iterate.py, data/) out of scope.
Existing dependency/Eventlet warnings remain non-blocking post-1.0 cleanup items.

SweetingTech · 2026-05-20T04:25:43Z

Release-readiness validation completed locally before opening this PR:

./scripts/start.ps1 -Mode docker -ResetDb
./scripts/verify_boot.ps1 -Mode docker
Python compile checks
JavaScript syntax checks
pytest tests/services tests/contracts tests/integration -vv passed: 164 tests
pytest tests/e2e -rs -vv passed: 19 tests
Rebuilt release/TableTopDM-v1.0.0.zip in filesystem mode
Extracted the release zip and ran its scripts/verify_boot.ps1 -Mode docker successfully

The remaining warnings observed locally are the existing requests dependency warning and Eventlet deprecation warning; both are non-blocking post-1.0 cleanup items.

gemini-code-assist

Code Review

This pull request adds a --reset-db flag to startup scripts for recovering from migration checksum mismatches and refactors the player state snapshot service to reuse a single database connection. It also improves process management in the start.sh and stop.sh scripts. Review feedback identifies a potential command injection vulnerability in scripts/stop.sh and suggests reducing code duplication in scripts/start.sh by extracting helper functions.

gemini-code-assist · 2026-05-20T04:27:23Z

 fi

+stop_app_on_port() {
+  local port="${PORT:-8000}"


The port variable is used directly in shell commands (powershell.exe and lsof) without validation. If the PORT environment variable is set to a malicious string, it could lead to command injection. Ensure the port is a valid numeric value before using it in the command strings.

Suggested change

local port="${PORT:-8000}"

local port="${PORT:-8000}"

if [[ ! "$port" =~ ^[0-9]+$ ]]; then

echo "[stop] ERROR: Invalid port '$port'" >&2

return 1

fi

gemini-code-assist · 2026-05-20T04:27:23Z

+  if [[ -f "$APP_PID_FILE" ]]; then
+    local pid
+    pid="$(cat "$APP_PID_FILE")"
+    if kill -0 "$pid" >/dev/null 2>&1; then
+      kill "$pid" || true
+      sleep 1
+      if kill -0 "$pid" >/dev/null 2>&1; then
+        kill -9 "$pid" >/dev/null 2>&1 || true
+      fi
+    fi
+    rm -f "$APP_PID_FILE"
+  fi


This logic for stopping the application process by reading the PID file is duplicated in the reset_docker_deps function (lines 129-140). Consider extracting this into a reusable helper function (e.g., _stop_local_app_if_running) to improve maintainability and reduce code duplication.

gemini-code-assist · 2026-05-20T04:27:23Z

+    fi
+    rm -f "$APP_PID_FILE"
+  fi
+  local compose=(docker compose --env-file "$ROOT_DIR/.env" -f "$ROOT_DIR/infra/docker-compose.yml")


The definition of the compose array is duplicated here and in start_deps_docker (line 151). It would be better to define this once as a script-level variable or a helper function to ensure consistency and simplify future updates to the Docker Compose command structure.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2ff9ef718

ℹ️ 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".

chatgpt-codex-connector · 2026-05-20T04:29:01Z

+    if kill -0 "$pid" >/dev/null 2>&1; then
+      kill "$pid" || true
+      sleep 1
+      if kill -0 "$pid" >/dev/null 2>&1; then
+        kill -9 "$pid" >/dev/null 2>&1 || true


Validate PID ownership before terminating process

When .local-run/app.pid exists, this block unconditionally sends kill/kill -9 to that PID without checking it is actually the TableTop DM app. If the pid file is stale and the PID has been reused by another local process, running start.sh can terminate an unrelated process, which is a destructive side effect introduced by the new restart path. Guard this by verifying command line/executable (or another app-specific marker) before killing.

Useful? React with 👍 / 👎.

Copilot

Pull request overview

This PR hardens the v1.0.0 release workflow by adding explicit “disposable Docker DB reset” recovery paths for migration checksum drift, improving local start/stop robustness (esp. on Windows/Git Bash), and reducing player-state snapshot DB overhead by reusing a single connection per snapshot.

Changes:

Add --reset-db / -ResetDb start options to rebuild disposable Docker dependency volumes when migration checksums drift.
Reuse a single Postgres connection across player-state snapshot queries (and thread it through supporting query helpers).
Tighten release packaging inputs, refresh the v1.0.0 artifact checksum, and document post-1.0 MCP runtime integrations.

Reviewed changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
shared/db/connection.py	Adds optional `conn` plumbing to query helpers to support connection reuse.
shared/auth/principal.py	Threads optional DB connection through principal loading queries.
services/session_intel/continuity.py	Threads optional DB connection through continuity read queries.
services/player_state/visibility.py	Threads optional DB connection through visibility queries.
services/player_state/snapshot.py	Reuses one DB connection per snapshot and passes it through dependent reads.
scripts/stop.sh	Adds port-based fallback stop logic to handle stale PID scenarios.
scripts/start.sh	Adds `--reset-db` Docker volume reset path and improves stale PID handling.
scripts/start.ps1	Exposes `-ResetDb` switch for the new Docker reset workflow.
scripts/package_release.ps1	Expands filesystem packaging exclusions and stages `release/README-FIRST.md`.
scripts/integration_boot.py	Improves migration checksum mismatch guidance with reset instructions.
release/README-FIRST.md	Documents checksum drift recovery via Docker dependency volume reset.
release/checksums.txt	Updates v1.0.0 zip checksum.
README.md	Adds checksum drift recovery instructions for disposable Docker DBs.
infra/sql/migrations/017_schema_migration_cleanup.sql	Cleans up a legacy schema migration tracking marker.
infra/scripts/migrate.sh	Improves checksum mismatch error output with recovery guidance.
docs/release/1.0-scope.md	Notes MCP runtime tool integrations as post-1.0 scope.
docs/release/1.0-golden-path-manual-qa.md	Documents QA recovery step for checksum drift in disposable DBs.
docs/MIGRATIONS_AND_SEEDING.md	Documents checksum drift recovery workflow for disposable Docker DBs.

Comments suppressed due to low confidence (1)

shared/db/connection.py:63

Same concern as execute_query: execute_one(..., conn=None) is positional. Making conn keyword-only avoids accidental positional use and keeps the connection-reuse API unambiguous.

def execute_one(query, params=None, conn=None):
    with get_cursor(conn=conn) as cur:
        cur.execute(query, params)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+def execute_query(query, params=None, fetch=True, conn=None):
+    with get_cursor(conn=conn) as cur:
        cur.execute(query, params)


+stop_app_on_port() {
+  local port="${PORT:-8000}"
+  if ! curl -fsS --max-time 1 "http://localhost:${port}/health" >/dev/null 2>&1; then
+    return 0
+  fi
+  if command -v powershell.exe >/dev/null 2>&1; then
+    powershell.exe -NoProfile -Command "\$p = Get-NetTCPConnection -LocalPort ${port} -State Listen -ErrorAction SilentlyContinue | Select-Object -First 1 -ExpandProperty OwningProcess; if (\$p) { Stop-Process -Id \$p -Force -ErrorAction SilentlyContinue }" >/dev/null 2>&1 || true
+  elif command -v lsof >/dev/null 2>&1; then


+        $Excluded = @(
+            ".git", ".venv", "__pycache__", "data", "burn-bag", ".local-run",
+            ".run", "release", ".pytest_cache", ".mypy_cache", ".ruff_cache",
+            "test-results-e2e", ".claude", ".env", "codex_iterate.py"
+        )
        Get-ChildItem -Force $Root |
            Where-Object {
-                $_.Name -notin @(".git", ".venv", "__pycache__", "data", "burn-bag", ".local-run", ".run", "release")
+                $_.Name -notin $Excluded
            } |
            ForEach-Object {
                Copy-Item -Recurse -Force -LiteralPath $_.FullName -Destination $Temp


Harden v1 release readiness

c2ff9ef

Copilot AI review requested due to automatic review settings May 20, 2026 04:25

SweetingTech merged commit 1d52fd1 into main May 20, 2026
2 of 3 checks passed

Copilot started reviewing on behalf of SweetingTech May 20, 2026 04:26 View session

gemini-code-assist Bot reviewed May 20, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed May 20, 2026

View reviewed changes

Copilot AI reviewed May 20, 2026

View reviewed changes

Navigation Menu

[codex] Harden v1 release readiness - #50

[codex] Harden v1 release readiness#50
SweetingTech merged 1 commit into
mainfrom
codex/v1-release-readiness

SweetingTech commented May 20, 2026

Uh oh!

SweetingTech commented May 20, 2026

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 20, 2026

Uh oh!

gemini-code-assist Bot May 20, 2026

Uh oh!

gemini-code-assist Bot May 20, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-  local port="${PORT:-8000}"
+  local port="${PORT:-8000}"
+  if [[ ! "$port" =~ ^[0-9]+$ ]]; then
+    echo "[stop] ERROR: Invalid port '$port'" >&2
+    return 1
+  fi

Conversation

SweetingTech commented May 20, 2026

Summary

Validation

Notes

Uh oh!

SweetingTech commented May 20, 2026

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 20, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants