fix(p7): first-bring-up fixes — container path crash, FILE secrets, profiles, broker auth - #2223
Conversation
…rofiles, broker auth Landed #2173 and stood P7 up on 5090; five defects surfaced at first real boot, each verified fixed live (healthz ok, rooms_loaded=9, nats_connected=true, smoke 46/46): 1. main.py crashed in-container: Path(__file__).parents[3] raises IndexError at /app/main.py before the ImportError fallback can engage (restart loop). Now walks upward for services/common — and APPENDS to sys.path: services/common ships its own config.py which would otherwise shadow the service's config module. (The old fixed index accidentally never resolved in-repo either, so the shared env helper was silently unused everywhere; tests now import the real one.) 2. Fallback get_secret ignored the <KEY>_FILE convention — compose delivers P7_SIGNING_KEY_FILE, which would silently never load in-container. 3. up-p7 used --profile agents, activating the legacy p7-room-orchestrator whose supabase-kong dependency is profile-gated — compose fails the whole project as 'depends on undefined service'. New p7 needs only --profile p7. 4. Legacy service's kong dependency marked required:false so ANY --profile agents invocation resolves again without supabase-local. 5. P7_NATS_URL lacked broker credentials — P7 booted log-only (nats_connected:false, no signed lifecycle events). Also: p7-health now retries (up-p7 chains into it before the container finishes booting) and smoke-p7 runs pytest from the service dir (flat-module imports). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Docker Hardening ValidationHardening Validation ReportValidated: Sat Jul 25 12:40:57 UTC 2026Services CheckedPMOVES.AI Docker Hardening Validation[INFO] Checking: pmoves/docker-compose.hardened.yml [INFO] Validating: hi-rag-gateway-v2 [INFO] Validating: extract-worker [INFO] Validating: langextract [INFO] Validating: presign [INFO] Validating: render-webhook [INFO] Validating: retrieval-eval [INFO] Validating: pdf-ingest [INFO] Validating: jellyfin-bridge [INFO] Validating: invidious-companion-proxy [INFO] Validating: ffmpeg-whisper [INFO] Validating: media-video [INFO] Validating: media-audio [INFO] Validating: hi-rag-gateway-v2-gpu [INFO] Validating: hi-rag-gateway-gpu [INFO] Validating: deepresearch [INFO] Validating: supaserch [INFO] Validating: publisher-discord [INFO] Validating: mesh-agent [INFO] Validating: nats-echo-req [INFO] Validating: nats-echo-res [INFO] Validating: comfy-watcher [INFO] Validating: grayjay-plugin-host [INFO] Validating: agent-zero [INFO] Validating: p7-room-orchestrator [INFO] Validating: archon [INFO] Validating: channel-monitor [INFO] Validating: pmoves-yt [INFO] Validating: notebook-sync [INFO] Validating: supabase_service_role_key [INFO] Validating: supabase_jwt_secret [INFO] Validating: p7_control_token ====================================== |
github-actions
Bot
added
compose
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ce0e90ae5
ℹ️ 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".
| # Broker requires auth (documented default creds convention) — without | ||
| # them P7 boots in log-only fallback (nats_connected:false, no signed | ||
| # lifecycle events on the control plane). | ||
| - P7_NATS_URL=${P7_NATS_URL:-nats://nats:pmoves@nats:4222} |
There was a problem hiding this comment.
When production operators replace the weak default credentials in env.shared.example (lines 30–32) and update the canonical NATS_URL, this fallback still authenticates with the hard-coded nats:pmoves credentials unless they also discover and define the otherwise undocumented P7_NATS_URL. P7 then silently starts in log-only mode and emits no lifecycle events; fall back through ${NATS_URL} before using the development URL.
AGENTS.md reference: AGENTS.md:L87-L89
Useful? React with 👍 / 👎.
| file_path = os.environ.get(f"{key}_FILE") | ||
| if file_path: |
There was a problem hiding this comment.
Inside the image this fallback is always used because services/common is not copied, and it now reads <KEY>_FILE before checking <KEY>. When both forms are present—for example, during a signing-key rotation where P7_SIGNING_KEY overrides a still-mounted file—the stale file value wins, causing invalid signatures or rejected control credentials. Match services/common/env.py::get_secret by checking the direct environment value first.
Useful? React with 👍 / 👎.
2 participants
Landed #2173 then stood P7 up for real on 5090 — five defects surfaced at first boot, all fixed and live-verified (
/healthz→rooms_loaded: 9, nats_connected: true,make up-p7+make smoke-p7green, 46/46):Path(__file__).parents[3]IndexErrors at/app/main.pybefore the ImportError fallback can engage. Now walks upward — and appends to sys.path (services/common ships its ownconfig.py, which insert(0) would shadow over the service'sconfigmodule). Bonus finding: the old fixed index never resolved in-repo either, so the shared secret helper was silently unused everywhere; tests now exercise the realenv.get_secret._FILEsecrets dead in-container: compose deliversP7_SIGNING_KEY_FILEbut the fallback only read plain env — signing key would silently never load.up-p7unbootable:--profile agentsactivates the legacy orchestrator whosesupabase-kongdep is profile-gated → compose fails the whole project. Newp7needs--profile p7only.required: falseon the legacy kong dep so any--profile agentsinvocation resolves without supabase-local.P7_NATS_URLlacked credentials → log-only fallback, no signed lifecycle events. Plus:p7-healthretries (up-p7 chains into it pre-boot),smoke-p7runs from the service dir.🤖 Generated with Claude Code