Skip to content

feat: introduce canonical ports.json + generator as single source of …#356

Open
eva57gr wants to merge 2 commits intoLight-Heart-Labs:mainfrom
eva57gr:pr-04-canonical-port-contract
Open

feat: introduce canonical ports.json + generator as single source of …#356
eva57gr wants to merge 2 commits intoLight-Heart-Labs:mainfrom
eva57gr:pr-04-canonical-port-contract

Conversation

@eva57gr
Copy link
Contributor

@eva57gr eva57gr commented Mar 17, 2026

…truth

Copy link
Collaborator

@Lightheartdevs Lightheartdevs left a comment

Choose a reason for hiding this comment

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

Review: REQUEST CHANGES — Critical consistency bugs

The concept is right — a canonical ports.json is the correct architectural direction. But the execution has fundamental inconsistencies.

Blocker: ports.json disagrees with itself

  • config/ports.json OLLAMA_PORT default: 8080
  • .env.example OLLAMA_PORT: 11434
  • .env.schema.json OLLAMA_PORT default: 8080
  • Every shell script fallback in the diff: 11434

Three different values claiming to be the default across the codebase. The "single source of truth" disagrees with itself on its flagship port.

Blocker: Contract test is DOA — schema mismatch

The test reads field names that don't exist in ports.json:

  • Test reads entry.get("env_var") → file has "env"
  • Test reads entry.get("external_default") → file has "default"
  • Test reads entry.get("internal_port") → file has "container_port"
  • Test reads entry.get("manifest_service") → file has no such field
  • Test reads entry.get("compose_managed") → file has "require_compose" (inverted semantics)

Test will fail immediately on first entry. Written against a different draft of ports.json.

High: Breaking change with no migration

Changing OLLAMA_PORT default from 8080→11434 breaks existing installs that lack an explicit OLLAMA_PORT in .env. Shell fallbacks resolve to 11434 while Docker is still mapping to 8080. No migration logic in dream-update.sh handles this transition.

High: Incomplete sweep

20+ files still hardcode 8080: health-check.sh, mode-switch.sh, demo-offline.sh, test files, QUICKSTART.md, config/backends/*.json.

Required fixes:

  1. Resolve the 8080 vs 11434 conflict — every source must agree
  2. Fix contract test field names to match actual ports.json schema
  3. Update .env.schema.json to match canonical default
  4. Add upgrade migration in dream-update.sh
  5. Sweep remaining hardcoded 8080 references
  6. Document PyYAML dependency needed by contract test

Note: Coordinate with PR #359

Both PRs attack port drift independently and contradict each other on the default value.

🤖 Reviewed with Claude Code

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.

2 participants