fix: detect existing Claude logins in supervised gateways - #222
fuller-stack-dev merged 1 commit into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: blocked before merge. Reviewed September 12, 2026, 3:41 AM ET / 07:41 UTC. ClawSweeper reviewWhat this changesPreserves nonempty USER and LOGNAME values through OCM’s background-service and gateway environments so native CLIs can locate existing logins, with regression coverage for both service managers. Merge readiness⛔ Blocked before merge - 1 item remains Keep open: this remains a useful, focused fix absent from the reviewed main and latest release. No actionable patch defect was found, and the member-authored PR is protected from automatic closure. Priority: P2 Review scores
Verification
How this fits togetherOCM registers a user-level background service and saves launch plans for the OpenClaw gateways it supervises. Environment filters determine which caller settings reach the daemon and its gateway children. flowchart TD
A[Caller environment] --> B[Service environment filter]
B --> C[User background service]
A --> D[Gateway environment filter]
D --> E[Saved gateway launch plan]
C --> F[Launch gateway child]
E --> F
F --> G[Native CLI login lookup]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve account names through the shared allowlists while retaining credential exclusions, existing saved-plan compatibility, and operator-controlled service refresh. Do we have a high-confidence way to reproduce the issue? Yes, for the OCM defect: main deterministically drops supplied USER and LOGNAME at both filters. Native Claude failure and recovery are reported in the PR; this read-only review did not execute them. Is this the best way to solve the issue? Yes. Extending the existing shared allowlists addresses both losses without introducing provider-specific configuration, copying credentials, or changing the saved-state format. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 305f965a1698. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
|
Reviewed ClawSweeper’s evidence-access concern on this exact head. The artifact host restriction is in the reviewer environment; the retained raw results were checked locally. No code or security defect was identified by ClawSweeper or Autoreview. For an inline record, the same real OCM CLI reproduction produced these results at both daemon and gateway boundaries:
An independent source-blind validator passed all four contract clauses across eight fixtures and nine supervised child runs, including persisted plans, distinct account names, empty and absent fields, and secret-exclusion controls. This synthetic proof uses a simulated service manager and real OCM child execution. The separate native Claude diagnosis found that restoring only USER changed login detection from false to true; HOME, LOGNAME, and TMPDIR alone did not. All five required CI checks passed on the reviewed head. Fresh canonical main remains |
Additional instructions
Keep Allow edits from maintainers enabled.
What Problem This Solves
Fixes an issue where users who had already signed in to Claude Code saw the OpenClaw UI report that Claude was not logged in when the gateway ran under OCM.
Why This Change Was Made
OCM discarded
USERandLOGNAMEwhen constructing both the background service environment and the supervised gateway environment. Claude's native login lookup needs the account identity. Preserve those nonempty values at both boundaries, using the existing environment allowlist. Credentials and terminal session variables remain excluded.User Impact
Supervised gateways can detect the same existing native CLI login as the user's terminal, without copying tokens or adding a provider-specific environment workaround.
Evidence
Unmodified
305f965a16982fd38bff27125a915d0b5984366c: both service boundaries dropped the supplied identity. The same CLI fixture passes on this PR.A native Claude probe with the actual failing service environment became authenticated when only
USERwas restored. Native Opus and Sonnet gateway turns then succeeded.The regression exercises launchd and systemd registration and executes a real capture-only child from the saved plan with the daemon caller's identity removed.
Empty-value and secret-exclusion controls are covered. Formatting, all-target compilation, and the focused regression pass.
391 library tests and all 40 service-command tests pass. The local full suite encountered two failures also reproduced on unmodified main:
bin_wrapper_runs_with_an_overridden_homeand thesibling_was_startedassertion indaemon_defers_a_saved_service_start_until_source_watch_releases_the_env. PR CI is still running.Source-blind validation passed all four clauses across eight identity fixtures and nine real supervised child runs, including absent/empty identity, a second account, persisted-state reuse, and secret-exclusion controls.
Autoreview: no findings in the selected scope; default P0 threshold.
json: green.json
json: proof-manifest.json
json: red.json
manifest: artifact-manifest.json
The PR's non-required macOS npm jobs hit the pre-existing fork limitation:
MACOS_TEAM_IDis empty, so signature verification rejects--team-id ''. This is tracked separately in #216. The Windows CI job passed.