Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 3 additions & 37 deletions .devcontainer/ai_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,27 +25,15 @@
from pathlib import Path
from typing import Callable, Dict, List, NoReturn, Optional, Sequence, Tuple

from env_local import load_environment

PROFILE_NAME = "devcontainer-zai"
OPENROUTER_PROFILE_NAME = "devcontainer-openrouter"
ZAI_ANTHROPIC_URL = "https://api.z.ai/api/anthropic"
OPENROUTER_ANTHROPIC_URL = "https://openrouter.ai/api"

LAUNCHER_NAMES = ("codex-zai", "claude-zai", "codex-openrouter", "claude-openrouter")

# Credential names whose .env.local value may override an already-exported
# stale copy when DATAVIZ_ENV_LOCAL_PRECEDENCE=file. Deliberately narrow:
# PATH and every other variable keep "non-empty preexisting wins".
MCP_CREDENTIAL_KEYS = frozenset(
{
"GITHUB_PERSONAL_ACCESS_TOKEN",
"GITHUB_MCP_PAT",
"GITHUB_TOKEN",
"ZAI_API_KEY",
"Z_AI_API_KEY",
"UNITY_MCP_TOKEN",
}
)

# Environment variables each Claude launch action unsets before re-exporting.
# Membership matches the bash original's unset list; order is not observable.
CLAUDE_ZAI_UNSET_KEYS = (
Expand Down Expand Up @@ -126,29 +114,7 @@ def load_env_local(env_file: Optional[str] = None) -> None:
else:
return

precedence_inverted = os.environ.get("DATAVIZ_ENV_LOCAL_PRECEDENCE") == "file"
with open(resolved, "r", encoding="utf-8", newline="") as handle:
content = handle.read()
if content.startswith("\ufeff"):
content = content[1:]
for raw_line in content.split("\n"):
line = raw_line.rstrip("\r").strip()
if not line or line.startswith("#") or "=" not in line:
continue
key, _, value = line.partition("=")
if key.startswith("export "):
key = key[len("export "):]
key = key.strip()
value = value.strip()
if len(value) >= 1 and value[0] == value[-1] and value[0] in ('"', "'"):
value = value[1:-1]
if not key or any(not (character.isalnum() or character == "_") for character in key):
continue
if os.environ.get(key):
overwrite = precedence_inverted and key in MCP_CREDENTIAL_KEYS
if not overwrite:
continue
os.environ[key] = value
os.environ.update(load_environment(Path(resolved), os.environ))


def resolve_action(invoked_as: str, first_argument: Optional[str]) -> str:
Expand Down
40 changes: 40 additions & 0 deletions .devcontainer/env_local.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#!/usr/bin/env python3
"""Load literal environment-file assignments for Python startup tools (issue #171)."""
from __future__ import annotations

import re
from pathlib import Path
from typing import Dict, Mapping, Optional

MCP_CREDENTIAL_KEYS = frozenset({
'GITHUB_PERSONAL_ACCESS_TOKEN', 'GITHUB_MCP_PAT', 'GITHUB_TOKEN',
'ZAI_API_KEY', 'Z_AI_API_KEY', 'UNITY_MCP_TOKEN',
})


def load_environment(path: Path, environment: Mapping[str, str], prefer_file: Optional[bool] = None) -> Dict[str, str]:
"""Return a copy with file assignments merged using the sourced loader's rules.

Values remain literal. Managed credentials may use file precedence; all
other non-empty environment values win. Process entries in order so repeated
keys follow the same precedence rule as repeated shell exports.
"""
result = dict(environment)
if not path.is_file():
return result
content = path.read_bytes().decode('utf-8', errors='surrogateescape')
for raw in content.split('\n'):
line = raw.removeprefix('\ufeff').strip()
if not line or line.startswith('#') or '=' not in line:
continue
key, value = (part.strip() for part in line.split('=', 1))
if key.startswith('export '):
key = key[7:].strip()
if not re.fullmatch(r'[A-Za-z_][A-Za-z0-9_]*', key):
continue
if len(value) >= 2 and value[0] == value[-1] and value[0] in "\"'":
value = value[1:-1]
file_wins = result.get('DATAVIZ_ENV_LOCAL_PRECEDENCE') == 'file' if prefer_file is None else prefer_file
if not result.get(key) or (file_wins and key in MCP_CREDENTIAL_KEYS):
result[key] = value
return result
56 changes: 14 additions & 42 deletions .devcontainer/post_start.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

sys.dont_write_bytecode = True

from env_local import load_environment

from _lifecycle_common import (
ENV_BLOCK_BEGIN,
ENV_BLOCK_END,
Expand Down Expand Up @@ -104,50 +106,20 @@ def repair_cache_ownership(home: Path) -> None:
repair_ownership(npm_cache)


def child_env_with_env_local(repo_root: Path) -> Optional[Dict[str, str]]:
"""Load .env.local with file precedence in a child bash and capture its environment.

The bash original sourced ``env-local.sh`` into its own shell so every later
child saw the loaded credentials; a Python process cannot source bash, so
the same loader runs in a child that prints its final environment with
``env -0``. devcontainer remoteEnv forwards ``${localEnv:VAR}`` once at
container start, and OpenCode's shared ``serve --service`` inherits the
environment of whichever client started it, so a credential rotated in
.env.local is otherwise invisible until the container is rebuilt. Scoped to
credential names by the loader; PATH and all other vars keep "non-empty
preexisting wins".
def child_env_with_env_local(repo_root: Path) -> Dict[str, str]:
"""Load credentials for child tools without spawning a shell.

Returns None when the loader is missing; exits with the loader's code when
it fails, matching the bash original's ``set -e`` behavior.
File precedence applies only to managed credentials; PATH and unrelated
non-empty variables retain their inherited values.
"""
if not (repo_root / ".devcontainer" / "env-local.sh").is_file():
return None
script = (
'. "$1/.devcontainer/env-local.sh" 1>&2 || exit 10\n'
"DATAVIZ_ENV_LOCAL_PRECEDENCE=file\n"
'load_env_local "$1/.env.local" 1>&2 || exit 11\n'
"env -0"
)
completed = subprocess.run(
["bash", "-c", script, "env-local-loader", str(repo_root)],
capture_output=True,
check=False,
)
if completed.returncode != 0:
sys.stderr.buffer.write(completed.stderr or b"")
raise SystemExit(completed.returncode)
entries: Dict[str, str] = {}
for record in (completed.stdout or b"").split(b"\0"):
if not record or b"=" not in record:
continue
name, value = record.split(b"=", 1)
entries[name.decode("utf-8", errors="surrogateescape")] = value.decode("utf-8", errors="surrogateescape")
if not entries:
# An environment dump with no entries cannot be an honest snapshot;
# abort instead of silently continuing with stale credentials.
print("env-local loader produced an empty environment; aborting.", file=sys.stderr)
raise SystemExit(1)
return entries
environment = dict(os.environ)
if 'DATAVIZ_ENV_LOCAL_PRECEDENCE' in environment:
environment['DATAVIZ_ENV_LOCAL_PRECEDENCE'] = 'file'
try:
return load_environment(repo_root / '.env.local', environment, prefer_file=True)
except OSError:
print('Cannot read .env.local; aborting startup.', file=sys.stderr)
raise SystemExit(11)


def repair_credential_aliases(workspace: Path, env: Optional[Dict[str, str]]) -> None:
Expand Down
66 changes: 15 additions & 51 deletions .devcontainer/recycle_stale_opencode_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,19 @@
it lets the next client respawn it from the corrected environment, which is
the only fix that does not require rebuilding the container.

Bash stays in the loop for the operations whose semantics must not fork:

* Reading the env file sources ``env-local.sh`` so the loader's rules (BOM,
CRLF, ``export `` prefix, symmetric quotes) keep exactly one implementation.
A hand-rolled parser that disagrees with the loader would compare the service
against a string the loader never produces and recycle a healthy service on
every start - the exact bug that moved parsing behind the loader once before.
The child unsets every candidate key first, so a value inherited from the
caller can never read as present in the file, and it sets
``DATAVIZ_ENV_LOCAL_PRECEDENCE=file`` so the file's value wins for the
managed credential names.
* Probing, signalling, and reading the environment of the service process run
through bash's ``kill`` and ``/proc``, so pid namespaces stay consistent on
every host: MSYS pid tables differ from Windows pids, and ``os.kill(pid, 0)``
on Windows terminates the process instead of probing it.
Environment-file parsing uses the same typed loader as startup and backend
launchers. Shell-equivalence tests pin BOM, CRLF, quote, and precedence rules.
Only values actually defined in the file may judge the service's credentials.
Bash still probes and signals processes so the PID namespace stays consistent
on MSYS, where Windows process IDs differ and os.kill(pid, 0) is unsafe.

Safety properties, each covered by a contract test:

* A service whose credential MATCHES .env.local is never touched.
* A record whose pid is gone is removed (clients must not attach to it).
* No .env.local means nothing to compare against, so nothing to do.
* Credential VALUES are never printed - only variable NAMES.
* Missing evidence fails safe: a missing loader, an unreadable environment, or
* Missing evidence fails safe: an unreadable environment or
an empty file value leaves a key unjudged, and an unjudged key never kills.

The recycler is a Linux devcontainer component: it reads the service's
Expand All @@ -61,6 +51,8 @@
from pathlib import Path
from typing import Dict, List, Optional, Sequence, Tuple

from env_local import load_environment

# Credential names worth comparing. The github pair and the z.ai pair are the
# ones that circulate under two names; the rest are single-name secrets whose
# rotation has the same staleness problem.
Expand All @@ -72,29 +64,9 @@
"Z_AI_API_KEY",
)

SCRIPT_DIR = Path(__file__).resolve().parent

# The bash child templates. Each one is a fixed string, so nothing the record
# or the environment contains can reach a shell parser: pids and paths travel
# as argv, never as interpolated script text.
ENV_VALUES_CHILD = """
loader="$1"
env_file="$2"
shift 2
for key in "$@"; do
unset "$key"
done
. "$loader"
DATAVIZ_ENV_LOCAL_PRECEDENCE=file
load_env_local "$env_file"
for key in "$@"; do
value="${!key-}"
if [ -n "$value" ]; then
printf '%s=%s\\0' "$key" "$value"
fi
done
"""

SERVICE_ENVIRON_CHILD = 'cat "/proc/$1/environ" 2>/dev/null'

PID_ALIVE_CHILD = 'kill -0 "$1" 2>/dev/null'
Expand All @@ -105,9 +77,8 @@
def _bash_child_output(script: str, arguments: Sequence[str]) -> bytes:
"""Run one bash child and return its stdout bytes, or empty on any failure.

A failed child (missing bash, a loader that cannot source, a dead pid's
unreadable /proc entry) returns empty output; every caller treats empty as
"no evidence", which is the fail-safe direction.
A failed child (missing bash or an unreadable /proc entry) returns empty
output. Callers treat it as no evidence and leave the service alone.
"""
try:
completed = subprocess.run(
Expand Down Expand Up @@ -154,19 +125,12 @@ def _parse_env_entries(raw: bytes) -> Dict[str, str]:


def read_env_file_values(env_file: Path) -> Dict[str, str]:
"""Return the tracked credential keys the env file defines, with their values.

The values come from the sanctioned loader, so the recycle decision compares
the service against exactly the strings a real session would load.
"""
loader = SCRIPT_DIR / "env-local.sh"
if not loader.is_file():
# Standalone fallback the bash original had (a copied script without
# its sibling loader) is deliberately gone: without the loader there
# are no file values, and no file values means nothing is killed.
"""Return non-empty tracked values defined by the file, without inherited keys."""
try:
values = load_environment(env_file, {}, prefer_file=True)
except OSError:
return {}
raw = _bash_child_output(ENV_VALUES_CHILD, [str(loader), str(env_file), *CREDENTIAL_KEYS])
return _parse_env_entries(raw)
return {key: value for key, value in values.items() if key in CREDENTIAL_KEYS and value}


def service_environment(pid: int) -> Dict[str, str]:
Expand Down
3 changes: 2 additions & 1 deletion .llm/context.md
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,8 @@ runs everything, including the Prettier Markdown check, on ubuntu and windows. C
- The devcontainer (`.devcontainer/`) installs the four coding CLIs as the non-root user and refreshes them on create/start; **never** suggest `sudo npm` — rerun
`uv run .devcontainer/install_npm_tools.py` or rebuild instead.
- Gateway launchers (`codex-zai`, `claude-zai`, `codex-openrouter`, `claude-openrouter`) come from `.devcontainer/ai_backends.py`. They self-load the gitignored `.env.local` for
keys not already exported (preexisting environment variables win), so they work in non-interactive shells too; contract tests live in `scripts/tests/`.
keys not already exported (preexisting environment variables win), so they work in non-interactive shells too. Python startup, backend, and service tools share
`.devcontainer/env_local.py`; the sourced Bash loader remains for parent-shell exports, with parity pinned by `scripts/tests/test_env_local_precedence.py`.
- `npm run mcp:sync` regenerates machine-local MCP configs for Claude Code, Codex, OpenCode, Nanocoder, VS Code, and Cursor from `.llm/mcp/configure.mjs`. Those outputs are
gitignored and must never be committed.
- Secrets live in gitignored `.env.local` (template: `.env.local.example`); config files reference env vars instead of embedding credentials.
Expand Down
1 change: 1 addition & 0 deletions scripts/harness_scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@
".devcontainer/install_npm_tools.py": ("test_install_npm_tools.py",),
".devcontainer/install-npm-tools.sh": ("test_install_npm_tools.py",),
"scripts/tests/test_install_npm_tools.py.meta": ("test_install_npm_tools.py",),
".devcontainer/env_local.py": ("test_ai_backends.py", "test_env_local_precedence.py", "test_devcontainer_lifecycle.py", "test_opencode_service_recycle.py"),
".devcontainer/ai_backends.py": ("test_ai_backends.py", "test_env_local_precedence.py"),
".devcontainer/env-local.sh": ("test_ai_backends.py", "test_env_local_precedence.py"),
".devcontainer/backend_state.py": ("test_backend_state.py", "test_ai_backends.py", "test_env_local_precedence.py"),
Expand Down
5 changes: 4 additions & 1 deletion scripts/tests/test_ai_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from __future__ import annotations

import os
import shutil
import subprocess
import sys
from pathlib import Path
Expand Down Expand Up @@ -50,6 +51,7 @@

STUB_SCRIPT = """#!/usr/bin/env python3
import os
import shutil
import sys

lines = [f"cmd: {os.path.basename(sys.argv[0])}"]
Expand Down Expand Up @@ -132,7 +134,7 @@ def link_launchers(self) -> None:
self._symlinked = True

def copy_launcher_pair(self, broken_state_writer: bool) -> None:
"""Stage the launcher + state writer as a copied self-contained pair.
"""Stage the launcher, environment helper, and optional failing state writer.

A copied launcher computes its workspace from its own location, so a
deliberately broken ``backend_state.py`` beside it exercises the
Expand All @@ -142,6 +144,7 @@ def copy_launcher_pair(self, broken_state_writer: bool) -> None:
target = devcontainer / "ai_backends.py"
target.write_bytes(LAUNCHER.read_text(encoding="utf-8").encode("utf-8"))
target.chmod(0o755)
shutil.copy(LAUNCHER.parent / "env_local.py", devcontainer / "env_local.py")
if broken_state_writer:
writer = devcontainer / "backend_state.py"
writer.write_bytes("import sys\nsys.exit(1)\n".encode("utf-8"))
Expand Down
Loading
Loading