Skip to content

fix: detect existing Claude logins in supervised gateways - #222

Merged
fuller-stack-dev merged 1 commit into
openclaw:mainfrom
fuller-stack-dev:fix/native-cli-service-identity
Sep 12, 2026
Merged

fuller-stack-dev merged 1 commit into
openclaw:mainfrom
fuller-stack-dev:fix/native-cli-service-identity

Conversation

@fuller-stack-dev

@fuller-stack-dev fuller-stack-dev commented Sep 12, 2026

Copy link
Copy Markdown
Member
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 USER and LOGNAME when 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 USER was 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_home and the sibling_was_started assertion in daemon_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_ID is empty, so signature verification rejects --team-id ''. This is tracked separately in #216. The Windows CI job passed.

@clawsweeper

clawsweeper Bot commented Sep 12, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 12, 2026
@clawsweeper

clawsweeper Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed September 12, 2026, 3:41 AM ET / 07:41 UTC.

ClawSweeper review

What this changes

Preserves 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
Reviewed head: c7f42eac624c55c5e2569bfd0b179ef8d5794793

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused implementation with relevant boundary and exclusion coverage and no identified correctness or security defect.
Proof confidence 🌊 off-meta tidepool Not applicable: The MEMBER-authored PR is exempt from the ordinary contributor proof gate. Its body reports native login recovery and saved-plan child runs matching the changed filters; linked raw artifacts were inaccessible to this reviewer, and no material authorization change requires an authority-chain proof gate.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The MEMBER-authored PR is exempt from the ordinary contributor proof gate. Its body reports native login recovery and saved-plan child runs matching the changed filters; linked raw artifacts were inaccessible to this reviewer, and no material authorization change requires an authority-chain proof gate.
Evidence reviewed 9 items Repository policy and scope: Read the complete root AGENTS.md and contribution guidance. No nested AGENTS.md or maintainer-notes directory was found for the changed paths. Applied fixture isolation and operator-safety guidance; no builds, tests, or services were executed.
Verified introduced change: The complete base-to-head diff adds only USER and LOGNAME to the two existing filters, plus unit and integration coverage. Local HEAD matches the supplied original PR head.
Main still drops account names: Main’s gateway allowlist excludes both account-name keys; the daemon filter likewise copies only HOME, PATH, proxies, certificate settings, and OCM settings. The ordinary environment builder retains caller account names before these supervisor filters.
Findings None None.
Security None None.

How this fits together

OCM 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]
Loading

Before merge

  • Resolve merge risk (P1) - The linked native validation artifacts could not be inspected because their host is blocked in the reviewer environment; native authentication results remain contributor-reported.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta production +8/-1; tests +85/-0 The small production change repairs both filtering boundaries, with most added lines devoted to regression coverage.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Preserve account names through the shared allowlists while retaining credential exclusions, existing saved-plan compatibility, and operator-controlled service refresh.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best 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.

Labels

Label changes:

  • add P2: This is a bounded correction to native-login discovery in supervised gateways, with no demonstrated broader outage.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The MEMBER-authored PR is exempt from the ordinary contributor proof gate. Its body reports native login recovery and saved-plan child runs matching the changed filters; linked raw artifacts were inaccessible to this reviewer, and no material authorization change requires an authority-chain proof gate.

Label justifications:

  • P2: This is a bounded correction to native-login discovery in supervised gateways, with no demonstrated broader outage.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The MEMBER-authored PR is exempt from the ordinary contributor proof gate. Its body reports native login recovery and saved-plan child runs matching the changed filters; linked raw artifacts were inaccessible to this reviewer, and no material authorization change requires an authority-chain proof gate.

Evidence

What I checked:

  • Repository policy and scope: Read the complete root AGENTS.md and contribution guidance. No nested AGENTS.md or maintainer-notes directory was found for the changed paths. Applied fixture isolation and operator-safety guidance; no builds, tests, or services were executed. (AGENTS.md:1, c7f42eac624c)
  • Verified introduced change: The complete base-to-head diff adds only USER and LOGNAME to the two existing filters, plus unit and integration coverage. Local HEAD matches the supplied original PR head. (src/supervisor/mod.rs:72, c7f42eac624c)
  • Main still drops account names: Main’s gateway allowlist excludes both account-name keys; the daemon filter likewise copies only HOME, PATH, proxies, certificate settings, and OCM settings. The ordinary environment builder retains caller account names before these supervisor filters. (src/supervisor/mod.rs:2826, 305f965a1698)
  • Latest-release comparison: The v0.2.45 supervisor constants also omit USER and LOGNAME. The requested allowlist correction is therefore absent from both the reviewed main and the supplied latest release. (src/supervisor/mod.rs:69, c2c5590d5c6f)
  • Saved plan and final process boundary: Gateway environments are saved in the existing string map, then passed directly to Command after env_clear. The added integration test registers launchd and systemd definitions, removes the daemon caller’s account names, and checks the executed child receives the saved names without the token or session marker. Existing records remain readable; older plans acquire the names when rebuilt through existing synchronization paths. (tests/service_command_tests.rs:354, c7f42eac624c)
  • Security boundary inspection: Service definitions retain existing XML/systemd escaping and validation. Launchd selects the current numeric UID and systemd runs with --user; copying account-name strings does not change process credentials. Native supervisor-control identifiers remain scrubbed, and no dependency, workflow, credential, or execution-source change is introduced. (src/service/platform.rs:995, c7f42eac624c)

Likely related people:

  • Shakker: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • hannesrudolph: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@fuller-stack-dev

Copy link
Copy Markdown
Member Author

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:

Revision USER LOGNAME HOME/PATH GH_TOKEN, CODEX_SESSION_ID, DYLD_INSERT_LIBRARIES
Base 305f965a16982fd38bff27125a915d0b5984366c absent absent preserved excluded
Head c7f42eac624c55c5e2569bfd0b179ef8d5794793 fixture-user fixture-user preserved excluded

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 305f965a16982fd38bff27125a915d0b5984366c. Proceeding with the maintainer-authorized squash merge. The two nonrequired macOS npm checks have the already documented fork-variable failure and do not concern this change.

@fuller-stack-dev
fuller-stack-dev merged commit c749b58 into openclaw:main Sep 12, 2026
8 of 10 checks passed
@fuller-stack-dev

Copy link
Copy Markdown
Member Author

Merged as c749b58 after all five required checks passed on c7f42ea. Autoreview and the independent behavior validation found no actionable issue. Release preparation for v0.2.46 is underway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant