refactor(server): isolate the shell-hook side of system-env (split S09 L1/3)

[lidge-jun]

Summary

• Pure move: the shell-hook half of src/server/system-env.ts (env-file path/marker mode, writeShellEnvFile/removeShellEnvFile, installShellHook/uninstallShellHook, claudeCodeCliInstalled, reconcileShellHook, and the SystemEnvDeps resolver type — lines 15–242) moves verbatim to src/server/system-env-shell.ts (238 lines). system-env.ts keeps launchctl injection, tracking, rollback and the effective-model env computation (310 lines) and re-exports the five public shell functions plus the type, so all 12 previously exported names stay importable from the original path (14 importers unchanged).
• Why: 537-line file over the 400-line module limit; shell-hook management and launchctl/system-env tracking are separate concerns. Zero behavior change — snapshot/injection ordering and the catalog_busy handling in the residual are untouched.
• Plan and evidence: devlog/_plan/260905_now_split_train/270_server_system_env.md; rules 003_parent_decisions.md (PURE-MOVE-SIZE-01).

Stack (S09 server-management; merge bottom-up):

#	PR	Branch	Base	Review focus
3	TBD	codex/split-server-management-lab-routes	dev	lab-routes
2	TBD	codex/split-server-management-logs-usage-routes	codex/split-server-system-env	logs-usage-routes
1	this PR	codex/split-server-system-env ← you are here	dev	shell-hook leaf

Base: dev; layer 2 depends on this one. Review this PR's diff only (3 files, +261/−234; non-move diff 39 lines). Move-aware view: git diff --color-moved=dimmed-zebra dev...HEAD.

Verification

• bun run typecheck → exit 0
• Focused (system-env, claude-system-env-auto, claude-shell-hook, claude-management-api, model-visibility-management-api) → 84 pass / 0 fail
• Guards: tests/lab/core-lab-boundary.test.ts tests/codex-integration/compatibility-manifest.test.ts → 23 pass / 0 fail (system-env is reachable from the PROTECTED management-api.ts root; the leaf stays Lab-free)
• Red-drives, then restored: dropping CRLF handling in uninstallShellHook fails claude-shell-hook.test.ts:127; replacing the catalog_busy condition fails the source oracle at model-visibility-management-api.test.ts:78; a Lab import in the leaf fails the boundary guard with the chain management-api → system-env → system-env-shell → lab/paths.
• bun run privacy:scan → passed; git diff --check dev...HEAD clean.
• New test: facade/leaf identity for installShellHook and getShellEnvFilePath; leaf has no back-edge; residual still owns the catalog_busy branch.
• Full suite on the remote CI host (lidge) at this exact SHA: recorded in the devlog doc.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed (devlog unit records the layer; no user-facing change).
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults (admission-token/auth-detect handling moved byte-for-byte; no auth reads added at module load).

Summary by CodeRabbit

• New Features

  • Added shell environment support for Claude Code proxy settings, authentication, model selection, and automatic context options.
  • Added automatic installation, removal, and reconciliation of the shell hook that loads these settings.
  • Added detection for whether the Claude Code command-line tool is available.

• Bug Fixes

  • Improved shell-hook removal across different line-ending formats and added verification that cleanup completes successfully.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-05T02:44:19.462573Z	1cab08d	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: a207e4c9-1664-40d7-910f-b5de6a20aa48

📥 Commits

Reviewing files that changed from the base of the PR and between 3c920af and 1cab08d.

📒 Files selected for processing (3)

• src/server/system-env-shell.ts
• src/server/system-env.ts
• tests/server/system-env.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Changes

The shell environment implementation moved to system-env-shell.ts. It now manages claude-env.sh, .zshrc hooks, authentication markers, Claude Code CLI detection, and hook reconciliation. system-env.ts preserves the public exports. Tests verify the module boundary.

Shell environment and authentication markers

Layer / File(s)	Summary
Shell environment file and authentication markers src/server/system-env-shell.ts	The module sanitizes Anthropic environment slots, resolves proxy or subscription markers, writes protected shell exports, and removes the environment file.
Shell-hook installation and reconciliation src/server/system-env-shell.ts	The module installs and removes a CRLF-tolerant .zshrc hook, detects an executable claude CLI on PATH, and reconciles hook state on macOS.
Public re-exports and seam validation src/server/system-env.ts, tests/server/system-env.test.ts	system-env.ts re-exports the shell helpers and dependency type. Tests verify export identity and module direction.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 1cab0

The shell environment refactor preserves the public API and its lifecycle behavior without an actionable merge risk.

Sequence Diagram(s)

sequenceDiagram
  participant SystemEnv
  participant SystemEnvShell
  participant ClaudeCLI
  participant Zshrc
  SystemEnv->>SystemEnvShell: reconcileShellHook(systemEnvInjected)
  SystemEnvShell->>ClaudeCLI: inspect PATH for executable claude
  ClaudeCLI-->>SystemEnvShell: installation status
  alt Environment injected and CLI installed
    SystemEnvShell->>Zshrc: install marker-guarded source hook
  else Environment inactive or CLI absent
    SystemEnvShell->>Zshrc: remove marker-guarded source hook
  end
  SystemEnvShell-->>SystemEnv: return hook state

Loading

Possibly related PRs

• lidge-jun/opencodex#2105: Introduced shell-hook functionality that this PR extracts and re-exports.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: isolating the shell-hook functionality from system-env into a separate module.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/split-server-system-env

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[lidge-jun]

리뷰 · 우선순위 50 / 80

이 PR은 서버 쪽 src/server/system-env.ts 안에서 서로 다른 일을 하던 두 덩어리 중, 셸 훅(shell-hook) 쪽만 새 파일 src/server/system-env-shell.ts로 옮기는 순수 이동(pure move)입니다. 기준 브랜치는 지금 dev HEAD 3c920af5f7b18ecd98f87a589d21d299f5cbe172 입니다. 그 HEAD에는 Windows 스위트 안정화가 막 들어갔고, 패키지는 2.43.0이며, 열려 있는 큰 기차로는 S0x 분할과 provider-usage 계열이 있습니다. 이 PR은 그 중 S09(server-management) 스택의 맨 아래 잎(L1/3)입니다.

현재 dev의 system-env.ts는 약 537줄입니다. 여기에는 (1) claude-env.sh를 쓰고 .zshrc 훅을 설치·제거·맞추는 셸 쪽 로직과, (2) launchctl으로 시스템 환경을 주입·추적·롤백하고 유효 모델 env를 계산하는 쪽이 한 파일에 섞여 있습니다. 모듈 한도(대략 400줄)를 넘겼고, 관심사가 다르므로 먼저 셸 훅 절반을 떼어 내는 것이 이 PR의 목적입니다. 동작 변경은 없다고 명시되어 있으며, 스냅샷/주입 순서와 잔여 파일의 catalog_busy 처리도 그대로 둡니다.

옮긴 내용은 대략 이렇게 읽히면 됩니다. SystemEnvDeps, systemEnvMarkerMode, getShellEnvFilePath/writeShellEnvFile/removeShellEnvFile, installShellHook/uninstallShellHook, claudeCodeCliInstalled, reconcileShellHook가 새 잎 파일로 갑니다. 원래 system-env.ts는 공개 다섯 함수와 타입을 다시보내기(re-export)하고, 내부에서만 쓰던 writeShellEnvFile / removeShellEnvFile / systemEnvMarkerMode는 잎에서 import해서 계속 씁니다. 그래서 management-api·cli 등 기존 importer 경로는 ./system-env / ../server/system-env를 그대로 유지해도 됩니다. 확인한 dev 경로에도 src/server/system-env.ts와 tests/server/system-env.test.ts가 실제로 있습니다. system-env-shell.ts는 아직 dev에 없고 이 PR에서만 생깁니다.

테스트 쪽은 tests/server/system-env.test.ts에 파사드/잎 동일성 검사를 16줄 정도 추가합니다. installShellHook와 getShellEnvFilePath가 잎 심볼과 같은 참조인지 보고, 잎이 ./system-env로 다시 import하지 않는지 보고, 잔여 system-env.ts에 catalog_busy 문자열이 남았는지 확인합니다. 스택 표에 따르면 위층(L2 logs-usage-routes, L3 lab-routes)은 아직 TBD이고, 이 잎이 dev에 먼저 들어가야 다음 층이 쌓일 수 있습니다. 라벨은 chore이고 상태는 MERGEABLE입니다.

types/config 대분할 캠페인 관점에서는 이 파일이 그 캠페인 대상이 아닙니다. 서버 관리 모듈을 크기·관심사 기준으로 나누는 S09 열차의 첫 칸이라, 무효화되어 close-don't-rebase 할 종류가 아닙니다. 우선순위는 순수 이동 잎 기준으로 중간대(50)로 둡니다. 사용자 기능은 그대로이고, 다만 위층 분할의 바닥이므로 막히면 스택 전체가 늦어집니다. CodeRabbit 요약이 “새 기능”처럼 읽히지만 실제로는 이동이므로 릴리즈 노트에 사용자 기능으로 올리지 마세요.

경로/심볼 uninstallShellHook (system-env-shell.ts) - CRLF 대응 정규식과 marker 생존 검증은 이동만 했고 로직 자체는 그대로입니다. 회귀 위험이 큰 자리이므로 CI/포커스 테스트가 통과했다는 PR 본문 주장과 맞춰 두는 것이 좋습니다.
경로/심볼 writeShellEnvFile / removeShellEnvFile / systemEnvMarkerMode - 예전 system-env.ts에서는 모듈 밖 export가 아니었는데, 잎에서는 export로 공개됩니다. 파사드는 예전 공개 다섯 함수+타입만 다시보내므로 importer 경로는 안 깨지지만, 잎을 직접 import하면 예전보다 넓은 표면이 됩니다.
경로/심볼 잔여 system-env.ts - 잎을 re-export하면서 동시에 같은 타입·함수를 import합니다. 의도는 명확하지만, 이후 L2/L3에서 import 방향을 더 늘릴 때 순환(back-edge)이 생기지 않게 이번 테스트의 no-back-import 가드가 계속 막아주는지 확인이 필요합니다.
경로/심볼 src/server/system-env-shell.ts - Lab 경계를 건드리지 않는다는 가드(management-api → system-env → system-env-shell)를 PR 본문이 강조합니다. 리뷰 범위는 이 3파일 diff만이므로, 위층 라우트 분할 PR과 섞어 보지 않는 것이 맞습니다.
경로 tests/server/system-env.test.ts - 추가된 facade identity 테스트는 좋은 안전장치입니다. 다만 소스 문자열에 catalog_busy가 있는지만 보는 검사는 약합니다. 동작 회귀는 기존 포커스 스위트에 맡긴다는 전제를 받아들이면 됩니다.

메인테이너의 판단이 필요한 지점

• 잎에서 예전 private였던 writeShellEnvFile/removeShellEnvFile/systemEnvMarkerMode를 export로 두는 것을 PURE-MOVE로 허용할지, 아니면 잎 내부용으로 좁힐지.
• S09 L2/L3가 이 브랜치를 base로 쌓일 예정이므로, 이 잎을 먼저 dev에 머지할지 스택을 draft로 묶어 한 번에 볼지.
• Windows 스위트가 방금 dev에 안정화된 직후라, 이 chore 잎을 바로 태울지 아니면 다른 급한 provider-usage/S0x 열차 뒤를 할지.

너의 추천
MERGEABLE이고 순수 이동 + 파사드 동일성 테스트가 있으므로, 잎 export 표면 확대만 인지한 채 S09 L1으로 dev에 머지하는 쪽을 추천합니다. L2/L3는 이 SHA 위에 쌓고, types/config 분할 때문에 닫을 대상이 아닙니다. 머지 후 남는 원본 PR이 있으면 landed-via 처리 규칙을 따르면 됩니다.

이 댓글은 grok-bot이 작성했습니다

[Ingwannu]

Approved on exact head 1cab08d. I verified the extracted shell-hook declarations preserve their prior implementations, the original system-env path keeps its public facade exports, and the new leaf has no back-import into the facade or Compatibility Lab. Focused system-env/Claude management/visibility coverage plus the Lab and compatibility guards pass 107/107 under isolated HOME/OPENCODEX_HOME/CODEX_HOME; typecheck and privacy scan also pass. This is a behavior-preserving module split. The hosted macOS jobs are still running, so merge only after every required exact-head check is green.

[lidge-jun]

Superseded by aggregate PR #3668, admin-merged into dev as 6585e6a after exact-head CI and tested-tree verification. This original PR was not individually merged; its rebased content and historical records were consolidated in #3668. The original branch is preserved. Further unimplemented debt layers remain deferred.

[lidge-jun]

Landed via #3668 at 6585e6a