feat: 組織情報を含む設定を private-config へ分離 - #942
Conversation
- agent-deck config.toml / 1Password テンプレート / codespaces repos.txt を削除し keito4/private-config へ移設 - dotfiles.nix に privateConfig ヘルパー(mkOutOfStoreSymlink)を追加し private-config のローカルチェックアウトを参照 - devcontainer-env の許可キーリストを ~/.config/devcontainer-env-keys.txt(private-config 管理)から読む方式に変更 - credentials.sh のテンプレート解決を private-config 優先+従来パスへフォールバックに変更 - agent-deck Web UI (headless) の launchd 定義を追加(loopback バインドのみ) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📝 WalkthroughWalkthroughOrganization-specific credential and configuration files (1Password templates, agent-deck config, codespaces-secrets list) are removed from the public repository. Nix dotfiles and zsh devcontainer-env logic now reference a private-config checkout via out-of-store symlinks and a dynamic key allowlist. ChangesPrivate Config Migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ZshInit as Zsh Shell Init
participant AllowlistFile as devcontainer-env-keys.txt
participant EnvFile as .devcontainer.env
participant Environment as Shell Environment
ZshInit->>AllowlistFile: read allowlisted key names
ZshInit->>Environment: unset previously exported keys
ZshInit->>EnvFile: parse KEY=VALUE lines
ZshInit->>AllowlistFile: check if KEY is allowlisted
alt key matches allowlist
ZshInit->>Environment: export KEY=VALUE (CRLF stripped)
else key not allowlisted
ZshInit-->>ZshInit: skip key
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
test/nix-darwin-config.test.jsOops! Something went wrong! :( ESLint: 10.6.0 ReferenceError: describe is not defined 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0c7fe89a2
ℹ️ 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".
| echo "Fetching all credentials via provider ($(provider_name))..." | ||
|
|
||
| for template in "$CREDENTIALS_DIR"/templates/*.env.template; do | ||
| for template in "$CREDENTIALS_TEMPLATES_DIR"/*.env.template; do |
There was a problem hiding this comment.
Handle missing template directory before globbing
When private-config has not been cloned and CREDENTIALS_TEMPLATES_DIR is not set, the new fallback points at credentials/templates, but this commit deletes that directory from the public repo. With zsh's default NOMATCH behavior, the *.env.template glob here aborts fetch (and the analogous list loop) with a shell error instead of producing a clear missing-private-config message or an empty result, so fresh checkouts cannot run the documented credential commands successfully.
Useful? React with 👍 / 👎.
- credentials_templates.bats: テンプレートを private-config から解決し、無い環境(CI)ではスキップ - nix-darwin-config.test.js: 移設した2ファイルの実在チェックを削除し、privateConfig/mkOutOfStoreSymlink の参照検証を追加 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zsh の set -e 下では未一致 glob が致命的エラーになるため、(N) nullglob 修飾子で テンプレートが無い環境(CI 等)でも list/fetch が正常終了するようにする Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
nix/hosts/darwin/default.nix (1)
144-162: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider tightening
KeepAlivefor the agent-deck-web agent.
RunAtLoad = truewith unconditionalKeepAlive = truemeans launchd will keep respawning the process even ifagent-deckexits cleanly (e.g., misconfiguration) or the binary is temporarily missing. ScopingKeepAliveto only restart on crash avoids unnecessary respawn churn for what looks like an on-demand personal web UI.♻️ Optional refinement
RunAtLoad = true; - KeepAlive = true; + KeepAlive = { + SuccessfulExit = false; + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nix/hosts/darwin/default.nix` around lines 144 - 162, The agent-deck-web launchd agent currently uses unconditional KeepAlive alongside RunAtLoad, which can cause launchd to respawn the service even after clean exits or missing-binary failures. Update the agent-deck-web serviceConfig in default.nix to scope KeepAlive so it only restarts on crash/failure, while keeping the existing ProgramArguments, RunAtLoad, and logging settings intact.nix/home/dotfiles.nix (1)
10-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the base path from
config.home.homeDirectoryinstead of hardcoding it.
configis already in scope, so the absolute/Users/keitoprefix (username +/Users) can be replaced to avoid breakage if the home directory or username changes.♻️ Proposed refactor
privateConfig = path: { - source = config.lib.file.mkOutOfStoreSymlink "/Users/keito/develop/github.com/keito4/private-config/${path}"; + source = config.lib.file.mkOutOfStoreSymlink "${config.home.homeDirectory}/develop/github.com/keito4/private-config/${path}"; force = true; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nix/home/dotfiles.nix` around lines 10 - 13, The privateConfig helper currently hardcodes the absolute /Users/keito base path, which should instead be derived from config.home.homeDirectory. Update the privateConfig function in dotfiles.nix to build the mkOutOfStoreSymlink target from the configured home directory plus the private-config subpath, keeping config in scope and avoiding username-specific paths. Use the existing privateConfig symbol to locate the refactor and preserve the force behavior.script/credentials.sh (1)
11-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded private-config path duplicated across script and test.
The
DEFAULT_PRIVATE_TEMPLATESpath is duplicated verbatim intest/integration/credentials_templates.bats(line 25). Consider centralizing this default (e.g., a shared env file or constant) to avoid drift if the private-config location changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@script/credentials.sh` around lines 11 - 19, The default private-config templates path is hardcoded in both the credentials script and the integration test, so centralize it to avoid drift. Update the `DEFAULT_PRIVATE_TEMPLATES` usage in `credentials.sh` and make `test/integration/credentials_templates.bats` read the same shared source (for example via a shared env/constant file or exported variable) instead of duplicating the literal path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nix/home/zsh.nix`:
- Around line 122-139: The allowlist handling in the zsh env-loading block is
asymmetrically handling CRLF, so keys from devcontainer-env-keys.txt can retain
a trailing carriage return and fail both the unset and grep -qxF checks. Update
the allowlist-read path in the zsh setup logic to normalize keys by stripping
trailing CR before using them, and ensure the matching/unset logic in the block
around _codex_env_allowlist, _codex_env_key, and the grep/export flow compares
CR-free keys consistently with the .devcontainer.env parsing.
---
Nitpick comments:
In `@nix/home/dotfiles.nix`:
- Around line 10-13: The privateConfig helper currently hardcodes the absolute
/Users/keito base path, which should instead be derived from
config.home.homeDirectory. Update the privateConfig function in dotfiles.nix to
build the mkOutOfStoreSymlink target from the configured home directory plus the
private-config subpath, keeping config in scope and avoiding username-specific
paths. Use the existing privateConfig symbol to locate the refactor and preserve
the force behavior.
In `@nix/hosts/darwin/default.nix`:
- Around line 144-162: The agent-deck-web launchd agent currently uses
unconditional KeepAlive alongside RunAtLoad, which can cause launchd to respawn
the service even after clean exits or missing-binary failures. Update the
agent-deck-web serviceConfig in default.nix to scope KeepAlive so it only
restarts on crash/failure, while keeping the existing ProgramArguments,
RunAtLoad, and logging settings intact.
In `@script/credentials.sh`:
- Around line 11-19: The default private-config templates path is hardcoded in
both the credentials script and the integration test, so centralize it to avoid
drift. Update the `DEFAULT_PRIVATE_TEMPLATES` usage in `credentials.sh` and make
`test/integration/credentials_templates.bats` read the same shared source (for
example via a shared env/constant file or exported variable) instead of
duplicating the literal path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea29bcbe-9854-4070-8a43-10a86e6c4cfc
📒 Files selected for processing (10)
credentials/templates/devcontainer.env.templatecredentials/templates/mcp.env.templatedot/config/agent-deck/config.tomldot/config/codespaces-secrets/repos.txtnix/home/dotfiles.nixnix/home/zsh.nixnix/hosts/darwin/default.nixscript/credentials.shtest/integration/credentials_templates.batstest/nix-darwin-config.test.js
💤 Files with no reviewable changes (4)
- credentials/templates/mcp.env.template
- credentials/templates/devcontainer.env.template
- dot/config/codespaces-secrets/repos.txt
- dot/config/agent-deck/config.toml
| # 許可キー一覧は組織名を含むため private-config 管理の外部ファイルに置く | ||
| _codex_env_allowlist="$HOME/.config/devcontainer-env-keys.txt" | ||
| if [[ -r "$_codex_env_allowlist" ]]; then | ||
| # Clear previously exported tokens so removed entries in ~/.devcontainer.env don't linger. | ||
| while IFS= read -r _codex_env_key || [[ -n $_codex_env_key ]]; do | ||
| [[ -n $_codex_env_key && $_codex_env_key != \#* ]] && unset "$_codex_env_key" | ||
| done < "$_codex_env_allowlist" | ||
| if [[ -r "$HOME/.devcontainer.env" ]]; then | ||
| while IFS='=' read -r _codex_env_key _codex_env_value || [[ -n $_codex_env_key ]]; do | ||
| # Strip trailing CR so Windows-style CRLF files work correctly. | ||
| _codex_env_value="''${_codex_env_value%$'\r'}" | ||
| if [[ -n $_codex_env_key && $_codex_env_key != \#* ]] && grep -qxF -- "$_codex_env_key" "$_codex_env_allowlist"; then | ||
| export "$_codex_env_key=$_codex_env_value" | ||
| ;; | ||
| esac | ||
| done < "$HOME/.devcontainer.env" | ||
| unset _codex_env_key _codex_env_value | ||
| fi | ||
| done < "$HOME/.devcontainer.env" | ||
| fi | ||
| fi | ||
| unset _codex_env_key _codex_env_value _codex_env_allowlist |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
CRLF normalization is asymmetric: allowlist file keys aren't CR-stripped.
Line 132 strips a trailing \r from .devcontainer.env values, but the allowlist read on Line 126 and the grep -qxF match on Line 133 use the allowlist file as-is. If devcontainer-env-keys.txt is saved with CRLF endings, each allowlist line is KEY\r, so grep -x against the CR-stripped KEY fails and matching keys are silently never exported (and Line 127 would unset "KEY$'\r'"). Since the env file already gets CRLF handling, the allowlist should too for consistency.
🔧 Proposed fix to normalize allowlist keys
if [[ -r "$_codex_env_allowlist" ]]; then
# Clear previously exported tokens so removed entries in ~/.devcontainer.env don't linger.
while IFS= read -r _codex_env_key || [[ -n $_codex_env_key ]]; do
+ _codex_env_key="''${_codex_env_key%$'\r'}"
[[ -n $_codex_env_key && $_codex_env_key != \#* ]] && unset "$_codex_env_key"
done < "$_codex_env_allowlist"
if [[ -r "$HOME/.devcontainer.env" ]]; then
while IFS='=' read -r _codex_env_key _codex_env_value || [[ -n $_codex_env_key ]]; do
# Strip trailing CR so Windows-style CRLF files work correctly.
+ _codex_env_key="''${_codex_env_key%$'\r'}"
_codex_env_value="''${_codex_env_value%$'\r'}"
- if [[ -n $_codex_env_key && $_codex_env_key != \#* ]] && grep -qxF -- "$_codex_env_key" "$_codex_env_allowlist"; then
+ if [[ -n $_codex_env_key && $_codex_env_key != \#* ]] && grep -qxF -- "$_codex_env_key" <(tr -d '\r' < "$_codex_env_allowlist"); then
export "$_codex_env_key=$_codex_env_value"
fi
done < "$HOME/.devcontainer.env"
fi
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # 許可キー一覧は組織名を含むため private-config 管理の外部ファイルに置く | |
| _codex_env_allowlist="$HOME/.config/devcontainer-env-keys.txt" | |
| if [[ -r "$_codex_env_allowlist" ]]; then | |
| # Clear previously exported tokens so removed entries in ~/.devcontainer.env don't linger. | |
| while IFS= read -r _codex_env_key || [[ -n $_codex_env_key ]]; do | |
| [[ -n $_codex_env_key && $_codex_env_key != \#* ]] && unset "$_codex_env_key" | |
| done < "$_codex_env_allowlist" | |
| if [[ -r "$HOME/.devcontainer.env" ]]; then | |
| while IFS='=' read -r _codex_env_key _codex_env_value || [[ -n $_codex_env_key ]]; do | |
| # Strip trailing CR so Windows-style CRLF files work correctly. | |
| _codex_env_value="''${_codex_env_value%$'\r'}" | |
| if [[ -n $_codex_env_key && $_codex_env_key != \#* ]] && grep -qxF -- "$_codex_env_key" "$_codex_env_allowlist"; then | |
| export "$_codex_env_key=$_codex_env_value" | |
| ;; | |
| esac | |
| done < "$HOME/.devcontainer.env" | |
| unset _codex_env_key _codex_env_value | |
| fi | |
| done < "$HOME/.devcontainer.env" | |
| fi | |
| fi | |
| unset _codex_env_key _codex_env_value _codex_env_allowlist | |
| # 許可キー一覧は組織名を含むため private-config 管理の外部ファイルに置く | |
| _codex_env_allowlist="$HOME/.config/devcontainer-env-keys.txt" | |
| if [[ -r "$_codex_env_allowlist" ]]; then | |
| # Clear previously exported tokens so removed entries in ~/.devcontainer.env don't linger. | |
| while IFS= read -r _codex_env_key || [[ -n $_codex_env_key ]]; do | |
| _codex_env_key="''${_codex_env_key%$'\r'}" | |
| [[ -n $_codex_env_key && $_codex_env_key != \#* ]] && unset "$_codex_env_key" | |
| done < "$_codex_env_allowlist" | |
| if [[ -r "$HOME/.devcontainer.env" ]]; then | |
| while IFS='=' read -r _codex_env_key _codex_env_value || [[ -n $_codex_env_key ]]; do | |
| # Strip trailing CR so Windows-style CRLF files work correctly. | |
| _codex_env_key="''${_codex_env_key%$'\r'}" | |
| _codex_env_value="''${_codex_env_value%$'\r'}" | |
| if [[ -n $_codex_env_key && $_codex_env_key != \#* ]] && grep -qxF -- "$_codex_env_key" <(tr -d '\r' < "$_codex_env_allowlist"); then | |
| export "$_codex_env_key=$_codex_env_value" | |
| fi | |
| done < "$HOME/.devcontainer.env" | |
| fi | |
| fi | |
| unset _codex_env_key _codex_env_value _codex_env_allowlist |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nix/home/zsh.nix` around lines 122 - 139, The allowlist handling in the zsh
env-loading block is asymmetrically handling CRLF, so keys from
devcontainer-env-keys.txt can retain a trailing carriage return and fail both
the unset and grep -qxF checks. Update the allowlist-read path in the zsh setup
logic to normalize keys by stripping trailing CR before using them, and ensure
the matching/unset logic in the block around _codex_env_allowlist,
_codex_env_key, and the grep/export flow compares CR-free keys consistently with
the .devcontainer.env parsing.
|
🎉 This PR is included in version 1.120.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Why
Closes #941
公開リポジトリに 1Password の vault/item 構造・組織リポジトリ名・組織名入り環境変数名・agent-deck の組織別設定が含まれていた(または含まれる予定だった)ため、非公開リポジトリへ分離する。
What
credentials/templates/*.env.template(op:// パス)dot/config/agent-deck/config.tomldot/config/codespaces-secrets/repos.txtprivateConfigヘルパーを追加。private-config のローカルチェックアウト(~/develop/github.com/keito4/private-config)をmkOutOfStoreSymlinkで参照。編集は rebuild 不要で即反映~/.config/devcontainer-env-keys.txt(private-config 管理)から読む方式へ変更し、組織名入り変数名のハードコードを排除CREDENTIALS_TEMPLATES_DIRで上書き可)How
darwin-rebuild build --flake ./nix#keitonoMacBook-Proでビルド検証済みscript/credentials.sh listで private-config テンプレートの解決を確認済みRisk
codex/sync-macos-bootstrap-configブランチは zsh.nix の同一ブロックを変更しているため、本 PR マージ後に conflict 解消が必要(外部ファイル方式を採用する)~/.config/devcontainer-env-keys.txtが存在しない場合、環境変数の展開がスキップされる(従来の未設定時と同挙動)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes