Skip to content

feat(agent): alternative-provider support for the Claude core (core_config) - #1892

Closed
jsun-m wants to merge 7 commits into
sonichi:mainfrom
jsun-m:feat/agent-claude-provider
Closed

jsun-m wants to merge 7 commits into
sonichi:mainfrom
jsun-m:feat/agent-claude-provider

Conversation

@jsun-m

@jsun-m jsun-m commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #1891 (the src/agent/claude/ restructure). Until #1891 merges, that PR's commits appear in this diff too — review the top commit here. Rebased onto main once #1891 lands.

What

Adds alternative-provider support for the Claude core agent via a high-level core_config block, so the core can run on a custom Anthropic-compatible endpoint + API token instead of the Claude.ai subscription — and lets subtasks use cheaper model tiers.

core_config

"core_config": {
  "core_type": "claude_cli",
  "provider": "https://your-gateway.example.com",
  "auth_env": "ANTHROPIC_AUTH_TOKEN",
  "model": "opus-class-model",
  "models": { "sonnet": "…", "haiku": "…" }
}
  • provider empty → Claude.ai subscription (unchanged default); set → the same interactive core boots against that endpoint.
  • auth_env defaults to the sentinel ANTHROPIC_SUBSCRIPTION (OAuth, no token). A real token env — ANTHROPIC_AUTH_TOKEN (Bearer) or ANTHROPIC_API_KEY (x-api-key) — is read from the env or the vault, never the config file; the other auth var is cleared to avoid a mixed-header conflict.
  • model → ANTHROPIC_MODEL; models.{opus,sonnet,haiku} → the ANTHROPIC_DEFAULT_*_MODEL class aliases, so subagents/quick work can run cheaper models.

How

  • provider-env.sh — one shared helper resolves endpoint + token + model aliases; sourced by both launchers.
  • cli/start-cli.sh — provider-backed core: sheds the inherited credential-proxy ANTHROPIC_BASE_URL, exports the provider env, refuses to start if a provider is set but no token resolves (rather than silently using the subscription).
  • cli/dashp.sh — headless one-shot claude -p against the provider.
  • Schema + getter in sutando_config.py / sutando-config.sh; sutando_config.ts known-keys updated.

Tests

New tests/start-cli-provider-as-core.test.sh (6 cases: provider env reaches claude, proxy base_url shed, no-token refusal, provider+subscription-auth refusal, no-provider subscription path unchanged, source-tied guard). Config py/ts suites pass.

🤖 Generated with Claude Code

…de/cli/

Extract the two scripts that initialize + onboard the Claude Code core agent out
of scripts/ into a dedicated src/agent/claude/cli/, so the launch/onboarding
logic lives in one place (and a future headless / alternate-provider launcher can
grow alongside it):

  scripts/start-cli.sh           -> src/agent/claude/cli/start-cli.sh
  scripts/sutando-shell-setup.sh -> src/agent/claude/cli/sutando-shell-setup.sh

Re-points every caller -- startup.sh, health-check.py (_default_core_restart),
Sutando.app (main.swift), sutando-migrate.sh, the health-check-fallback launchd
plist, docs/host-cli-bindings.md, and comment refs in sutando_config.py/ts +
util_paths.ts -- and fixes each script's repo-root self-location (dirname/.. ->
dirname/../../../.. now that they are four levels deep). Updates the affected
tests and adds src/agent/claude/README.md.

Pure move + reference update; no behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jsun-m added a commit to jsun-m/sutando that referenced this pull request Jul 1, 2026
…de_sdk)

Add an Agent-SDK-based core (core_config.core_type: claude_sdk) that runs a
persistent, observable Claude Code session behind a live browser UI, and carries
the full Sutando runtime. Builds on sonichi#1891 + sonichi#1892.

- sdk/session-server.ts: one long-lived @anthropic-ai/claude-agent-sdk query()
  with async-iterable input (persistent multi-turn) -> SSE event stream + minimal
  ui.html + POST /input. Loads CLAUDE.md + skills (settingSources + claude_code
  systemPrompt preset) so it is not a bare Q&A session; replays event history to
  each new SSE client so a browser refresh shows the full session.
- Hybrid runtime: the Node host owns task ingestion (watch tasks/, stage on
  pickup, inject, archive on result), the per-host liveness heartbeat, and
  cron/proactive scheduling (reads crons.json, injects each prompt at its time --
  CronCreate is not available in an SDK session). SUTANDO_HOST_OWNS_WATCHER=1
  no-ops the agent's Monitor watcher so tasks do not double-process.
- src/startup.sh routes the core by core_config.core_type (claude_cli -> tmux
  core; claude_sdk -> session-server, backgrounded, heartbeat deferred to
  core_heartbeat.py). restart.sh/stop.sh now stop the session-server, and
  watch-tasks-stream.sh gains the host-owns-watcher no-op guard.

Stacked on sonichi#1891 + sonichi#1892 -- review after they merge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jsun-m
jsun-m force-pushed the feat/agent-claude-provider branch from faf7daa to 8a73ae2 Compare July 1, 2026 22:53
jsun-m added a commit to jsun-m/sutando that referenced this pull request Jul 1, 2026
…de_sdk)

Add an Agent-SDK-based core (core_config.core_type: claude_sdk) that runs a
persistent, observable Claude Code session behind a live browser UI, and carries
the full Sutando runtime. Builds on sonichi#1891 + sonichi#1892.

- sdk/session-server.ts: one long-lived @anthropic-ai/claude-agent-sdk query()
  with async-iterable input (persistent multi-turn) -> SSE event stream + minimal
  ui.html + POST /input. Loads CLAUDE.md + skills (settingSources + claude_code
  systemPrompt preset) so it is not a bare Q&A session; replays event history to
  each new SSE client so a browser refresh shows the full session.
- Hybrid runtime: the Node host owns task ingestion (watch tasks/, stage on
  pickup, inject, archive on result), the per-host liveness heartbeat, and
  cron/proactive scheduling (reads crons.json, injects each prompt at its time --
  CronCreate is not available in an SDK session). SUTANDO_HOST_OWNS_WATCHER=1
  no-ops the agent's Monitor watcher so tasks do not double-process.
- src/startup.sh routes the core by core_config.core_type (claude_cli -> tmux
  core; claude_sdk -> session-server, backgrounded, heartbeat deferred to
  core_heartbeat.py). restart.sh/stop.sh now stop the session-server, and
  watch-tasks-stream.sh gains the host-owns-watcher no-op guard.

Stacked on sonichi#1891 + sonichi#1892 -- review after they merge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jsun-m and others added 4 commits July 1, 2026 15:57
…er paths

Moving start-cli.sh / sutando-shell-setup.sh into src/agent/claude/cli/ removed
the old scripts/ paths outright. Callers in the repo were repointed, but a
not-yet-rebuilt Sutando.app binary has repoRoot + "/scripts/start-cli.sh"
compiled in and would hard-fail after a `git pull` until rebuilt (same for an
in-flight health-check --recover-core). Add thin exec-forwarding shims at the
old paths for one release so those lagging callers keep working; remove after
all callers reference the new path. New tests/start-cli-compat-shim.test.sh
asserts the shims exist, are executable, parse, and forward — without launching
the live core.

Also fix two stale comments left by the move: the config-dir test said "three
levels up" (now four: cli → claude → agent → src → repo) and the migrate-import
test referenced the pre-move scripts/sutando-shell-setup.sh path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…onfig)

Add a high-level `core_config` block that lets the core agent run against a
custom Anthropic-compatible provider + API token instead of the Claude.ai
subscription, plus per-class model overrides for subtasks. Builds on the
src/agent/claude/ restructure (sonichi#1891).

- core_config: { core_type, provider, auth_env, model, models{opus,sonnet,haiku} }
  resolved by src/sutando_config.py::resolve_core_config + the sutando-config.sh
  `core-config` getter.
- provider-env.sh (shared helper): resolves the provider endpoint + token
  (env or vault) + the model aliases (ANTHROPIC_DEFAULT_{OPUS,SONNET,HAIKU}_MODEL).
- cli/start-cli.sh: when core_config.provider is set, boots the SAME interactive
  core against the provider -- sheds the inherited credential-proxy base URL,
  env token overrides OAuth, refuses if a provider is set but no token resolves.
- cli/dashp.sh: headless one-shot `claude -p` against the provider.
- auth_env defaults to the ANTHROPIC_SUBSCRIPTION sentinel (subscription OAuth,
  no token); a real token env (ANTHROPIC_AUTH_TOKEN / ANTHROPIC_API_KEY) selects
  Bearer vs x-api-key, with the other cleared to avoid a mixed-header conflict.

Stacked on sonichi#1891 -- review after it merges.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When core_config.auth_env is the default sentinel ANTHROPIC_SUBSCRIPTION, a
shell-set ANTHROPIC_API_KEY / ANTHROPIC_AUTH_TOKEN (e.g. exported in ~/.zshrc)
silently overrode the Claude.ai subscription OAuth — Claude Code precedence is
env token > keychain/.credentials.json — so the core billed the raw API (or
401'd on an invalid key) instead of using the subscription the operator asked
for. Clear both stray token vars at launch so auth_env=ANTHROPIC_SUBSCRIPTION
means "subscription, period" rather than trusting a clean env.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…precedence

Addresses review findings on the alternate-provider path (provider-env.sh):

- auth_env injection: replaced `eval "have=\${$auth_env}"` with bash indirect
  expansion `${!auth_env}` and added an identifier check (^[A-Za-z_][A-Za-z0-9_]*)
  before it's used for expansion/export. A hostile auth_env from a synced config
  can no longer be executed as code.
- credential leak over the endpoint: when a provider TOKEN will be sent (not the
  ANTHROPIC_SUBSCRIPTION sentinel), refuse a non-https URL to any non-loopback
  host (the token would go out in plaintext) and refuse embedded userinfo
  (user:pass@host leaks via logs/proxies). Loopback http stays allowed — that's
  how the subscription credential-proxy is addressed.
- precedence: an explicit SUTANDO_PROVIDER_URL/_MODEL (the channel dashp.sh's
  flags inject through) now WINS over an inherited ANTHROPIC_BASE_URL/_MODEL,
  restoring the documented CLI > env > config order. Previously an inherited
  ANTHROPIC_MODEL silently no-op'd `dashp.sh --model`.
- config read is LOUD: a malformed sutando.config makes the core-config getter
  exit nonzero; both provider-env.sh and start-cli.sh now capture the rc and warn
  instead of `2>/dev/null … || true`-swallowing it into a silent subscription
  fallback.

New tests/provider-env-guards.test.sh covers all four (injection rejected, http
+ userinfo refused, loopback allowed, override precedence). Existing
start-cli-provider-as-core.test.sh still passes 6/6.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jsun-m
jsun-m force-pushed the feat/agent-claude-provider branch from 8a73ae2 to 742be42 Compare July 1, 2026 22:58
jsun-m added a commit to jsun-m/sutando that referenced this pull request Jul 1, 2026
…de_sdk)

Add an Agent-SDK-based core (core_config.core_type: claude_sdk) that runs a
persistent, observable Claude Code session behind a live browser UI, and carries
the full Sutando runtime. Builds on sonichi#1891 + sonichi#1892.

- sdk/session-server.ts: one long-lived @anthropic-ai/claude-agent-sdk query()
  with async-iterable input (persistent multi-turn) -> SSE event stream + minimal
  ui.html + POST /input. Loads CLAUDE.md + skills (settingSources + claude_code
  systemPrompt preset) so it is not a bare Q&A session; replays event history to
  each new SSE client so a browser refresh shows the full session.
- Hybrid runtime: the Node host owns task ingestion (watch tasks/, stage on
  pickup, inject, archive on result), the per-host liveness heartbeat, and
  cron/proactive scheduling (reads crons.json, injects each prompt at its time --
  CronCreate is not available in an SDK session). SUTANDO_HOST_OWNS_WATCHER=1
  no-ops the agent's Monitor watcher so tasks do not double-process.
- src/startup.sh routes the core by core_config.core_type (claude_cli -> tmux
  core; claude_sdk -> session-server, backgrounded, heartbeat deferred to
  core_heartbeat.py). restart.sh/stop.sh now stop the session-server, and
  watch-tasks-stream.sh gains the host-owns-watcher no-op guard.

Stacked on sonichi#1891 + sonichi#1892 -- review after they merge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@john-the-dev john-the-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the hardening commit 742be4220 line-by-line — it addresses every provider-auth finding from the first pass:

  • eval "have=${$auth_env:-}" → indirect expansion local have="${!auth_env:-}" (no shell code-exec path), plus an identifier whitelist (case rejects anything not [A-Za-z_][A-Za-z0-9_]*, and empty/leading-digit) before auth_env is used.
  • ANTHROPIC_BASE_URL now enforces https (loopback http allowed — that's how the subscription credential-proxy is addressed) and rejects user@host userinfo, so a provider token can't leak in plaintext or via proxy/logs.
  • Model precedence fixed: SUTANDO_PROVIDER_MODEL/--model now wins over an inherited ANTHROPIC_MODEL (previously the inherited value silently no-op'd the switch).
  • Config read is now LOUD in both start-cli.sh and provider-env.sh — a malformed sutando.config warns with the rc + captured output instead of || true swallowing it into a silent subscription fallback.
  • tests/provider-env-guards.test.sh covers the guards.

The c67a6a400 subscription-core stray-key clear (unset a shell-set ANTHROPIC_API_KEY/AUTH_TOKEN so it can't override the Claude.ai subscription OAuth) is a correct, sharp catch. No blocking issues. Approving.

…H_TOKEN

Review finding: the subscription-mode cleanup unconditionally unset
ANTHROPIC_AUTH_TOKEN too, which breaks env-token subscription/OAuth setups where
ANTHROPIC_AUTH_TOKEN is the only credential available before .credentials.json
exists. Only ANTHROPIC_API_KEY (x-api-key) is unambiguously "not the
subscription" and is the header the credential-proxy does NOT rewrite — so it's
the one that silently bills the raw API. ANTHROPIC_AUTH_TOKEN can be a valid
subscription/OAuth bearer, and the proxy rewrites the Authorization header
anyway, so leave it untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jsun-m added a commit to jsun-m/sutando that referenced this pull request Jul 2, 2026
…de_sdk)

Add an Agent-SDK-based core (core_config.core_type: claude_sdk) that runs a
persistent, observable Claude Code session behind a live browser UI, and carries
the full Sutando runtime. Builds on sonichi#1891 + sonichi#1892.

- sdk/session-server.ts: one long-lived @anthropic-ai/claude-agent-sdk query()
  with async-iterable input (persistent multi-turn) -> SSE event stream + minimal
  ui.html + POST /input. Loads CLAUDE.md + skills (settingSources + claude_code
  systemPrompt preset) so it is not a bare Q&A session; replays event history to
  each new SSE client so a browser refresh shows the full session.
- Hybrid runtime: the Node host owns task ingestion (watch tasks/, stage on
  pickup, inject, archive on result), the per-host liveness heartbeat, and
  cron/proactive scheduling (reads crons.json, injects each prompt at its time --
  CronCreate is not available in an SDK session). SUTANDO_HOST_OWNS_WATCHER=1
  no-ops the agent's Monitor watcher so tasks do not double-process.
- src/startup.sh routes the core by core_config.core_type (claude_cli -> tmux
  core; claude_sdk -> session-server, backgrounded, heartbeat deferred to
  core_heartbeat.py). restart.sh/stop.sh now stop the session-server, and
  watch-tasks-stream.sh gains the host-owns-watcher no-op guard.

Stacked on sonichi#1891 + sonichi#1892 -- review after they merge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

@cla-assistant check

@liususan091219 liususan091219 added the request-change Has an active CHANGES_REQUESTED review label Jul 3, 2026
@liususan091219

Copy link
Copy Markdown
Collaborator

Codex triage findings (detail for the REQUEST_CHANGES review that follows):

The auth-isolation claim is not actually enforced or asserted: the test never asserts TOKEN is empty, and on the no-provider path ANTHROPIC_AUTH_TOKEN can still reach claude, overriding subscription OAuth. Also please split from #1891 movement churn.

  • Maddy (MBP)

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex triage - see my comment above. - Maddy

@liususan091219

liususan091219 commented Jul 3, 2026 •

Copy link
Copy Markdown
Collaborator

@jsun-m BLOCKING for #1892: the auth-isolation claim isn't actually enforced. On the no-provider path, ANTHROPIC_AUTH_TOKEN still reaches claude and overrides subscription OAuth - and the test never asserts TOKEN is empty. Fix: scrub the env bearer on the no-provider path, and make the test assert it.

Secondary (non-blocking, stack-wide): keep relocation churn out of additive PRs, and give the old-path compat shim a behavioral test (it's only source-grep-guarded today).

Owner go/no-go on the SDK-core direction is still pending - this is engineering feedback, not approval. - Maddy (MBP)

# A provider counts as configured via config OR the env override.
if [ -n "$_cc_provider" ] || [ -n "${SUTANDO_PROVIDER_URL:-}" ]; then
CLAUDE_AS_CORE_PROVIDER=1
[ "$_cc_core_type" = "claude_sdk" ] && echo " ~ note: core_config.core_type=claude_sdk — this is the CLI core; run src/agent/claude/sdk/session-server.sh for the SDK core." >&2

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: core_config.core_type is exposed as a supported setting here, but the actual startup/restart path still unconditionally routes through src/startup.sh -> src/agent/claude/cli/start-cli.sh. At this line the script only prints a note for claude_sdk and then continues launching the CLI core anyway. That means a user who sets core_type: claude_sdk will silently come back on the wrong core after any restart or boot. Either hide/gate the knob until the SDK launcher is wired in, or dispatch to the SDK session launcher here/startup so the persisted config is actually honored.

"path": "/absolute/path/to/your/workspace"
},

"core_config": {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: this PR introduces core_config, but docs/sutando-config.schema.json still has additionalProperties: false and no core_config entry. As soon as someone copies this example into sutando.config.local.json, schema-aware editors and any schema validation will flag the config as invalid even though the runtime now expects it. The schema needs to grow with the new top-level key in the same PR.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review: bash tests/start-cli-claude-config-dir.test.sh still fails in this branch. The updated test now points at src/agent/claude/cli/start-cli.sh, but its success expectation is still based on the deprecated $SUTANDO_WORKSPACE override (CLAUDE_CONFIG_DIR=$SUTANDO_WORKSPACE/.claude-sutando). Since workspace resolution ignores that env var as of v0.8, the launcher correctly exports the repo-local path ($REPO_FAKE/workspace/.claude-sutando) instead. Please update the test to derive the expected path from the configured workspace ($REPO_FAKE/workspace or sutando-config.sh workspace) so the integration check matches current behavior.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review:

src/agent/claude/cli/start-cli.sh:229-242 only unsets ANTHROPIC_API_KEY on the subscription path. A stale ANTHROPIC_AUTH_TOKEN from the parent shell still wins over Claude Code's OAuth credentials, so a supposedly “subscription” core can silently boot against whatever bearer token happened to be exported. This shows up on direct launches where no credential-proxy is rewriting the header. The subscription branch needs to clear stray provider auth vars before launching claude.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated review:

src/agent/claude/provider-env.sh:89-145 exports ANTHROPIC_BASE_URL before returning rc=2 for the ANTHROPIC_SUBSCRIPTION sentinel, and src/agent/claude/cli/dashp.sh:134-147 treats that rc as “run on the Claude.ai subscription” without clearing the provider URL. If core_config.provider is set but auth_env=ANTHROPIC_SUBSCRIPTION, dashp.sh still points claude -p at the provider endpoint while claiming it is using subscription auth. That sends the OAuth/subscription path to the wrong host and breaks the documented fallback.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review:

[P1] The new Python loader and the TypeScript loader disagree on CLAUDE_CONFIG_DIR resolution. src/sutando_config.py:483-520 now prefers core_config_dirs[type=claude] and errors if the legacy claude_sutando_config_dir is also present, but src/sutando_config.ts:388-420 still ignores core_config_dirs entirely and always falls back to the legacy subdir/default. Any TS consumer of resolveClaudeSutandoConfigDir() will now read/write a different config tree than shell/Python on the canonical config path, which can strand sessions/memory in the wrong directory. This needs the same resolution logic and parity coverage in TS before merge.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Automated mergeability review (2026-07-09)

Signal: NOT MERGEABLE

Current blocker(s):

  • Existing review decision is CHANGES_REQUESTED; requested changes need to be resolved.
  • GitHub reports merge state DIRTY (merge conflicts or otherwise not clean).

This is a conservative backlog triage signal. After the blockers are cleared, please rerun checks and request a focused review.

@liususan091219

Copy link
Copy Markdown
Collaborator

Daily reminder — this PR has an open change-request from 2026-07-03 (9d) with no commits since. Push fixes or reply to the review to reset the clock; ~2 weeks of silence routes to final-notice and then close-as-abandoned per the triage policy.

nudge: cr-unaddressed day9

@liususan091219 liususan091219 added live-test-requested A change-request asks for live-test/repro evidence (voice/vision/runtime policy) and removed feature labels Jul 13, 2026

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes (minimal): this PR's diff introduces a $HOME/.sutando/repo reference (in the SUTANDO_CLAUDE_WORKING_DIR comment/example). maintainer cannot decide because it's unclear whether ~/.sutando/repo is legitimate. Until that's decided, please drop or genericize the ~/.sutando/repo reference — the rest of the PR isn't blocked by it, just that literal.

@liususan091219

Copy link
Copy Markdown
Collaborator

Daily reminder — this PR has an open change-request from 2026-07-03 (11d) with no commits since. Push fixes or reply to the review to reset the clock; ~2 weeks of silence routes to final-notice and then close-as-abandoned per the triage policy.

nudge: cr-unaddressed day11

@liususan091219

Copy link
Copy Markdown
Collaborator

Daily reminder — this PR has an open change-request from 2026-07-03 (12d) with no commits since. Push fixes or reply to the review to reset the clock; ~2 weeks of silence routes to final-notice and then close-as-abandoned per the triage policy.

nudge: cr-unaddressed day12

@liususan091219

Copy link
Copy Markdown
Collaborator

Context for the live-test-requested label (applied 2026-07-14 15:42 in a batch that shipped without explanations — that was a process error on the triage loop's side, now being corrected on every affected PR).

Why a live test here: this PR changes alternative-provider support for the Claude core — provider switching is runtime auth/API behavior stubbed tests can't cover.

What clears the label: one real session transcript on the alternative provider (redact keys) showing a completed turn. Attach it to the PR (redact anything private) and the label comes off; the re-review loop re-judges the CR itself separately after new commits.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

LGTM on the current head. I ran bash tests/provider-env-guards.test.sh and bash tests/start-cli-provider-as-core.test.sh; both passed end to end. That covers the new provider-env guardrails (auth env validation, URL safety, override precedence) plus the start-cli provider-mode routing, including the refusal cases when a provider is configured without a resolvable token.

Qingyun's Personal Codex Agent

@liususan091219

Copy link
Copy Markdown
Collaborator

Daily reminder — this PR has an open change-request from 2026-07-03 (14d) with no commits since. Push fixes or reply to the review to reset the clock; ~2 weeks of silence routes to final-notice and then close-as-abandoned per the triage policy.

nudge: cr-unaddressed day14

@liususan091219

Copy link
Copy Markdown
Collaborator

Daily reminder — this PR has an open change-request from 2026-07-03 (16d) with no commits since. Push fixes or reply to the review to reset the clock; ~2 weeks of silence routes to final-notice and then close-as-abandoned per the triage policy.

nudge: cr-unaddressed day16

@john-the-dev john-the-dev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact current head ca7efb8ebd9f0a68947dceb50ea1289ae5b2a0a4 against current origin/main.

Blocking:

  1. src/sutando_config.py:507-529 prefers core_config_dirs[type=claude], but src/sutando_config.ts:388-404 still ignores core_config_dirs and resolves only the legacy/default subdir. Python/shell and TypeScript consumers can therefore write different CLAUDE_CONFIG_DIR trees. Please implement matching TS resolution/error semantics and add parity coverage.

  2. bash tests/start-cli-claude-config-dir.test.sh fails at case 2: expected the deprecated $SUTANDO_WORKSPACE/.claude-sutando, while the launcher correctly produced the configured repo workspace path. Update this integration test to derive the expected path from canonical workspace resolution.

  3. The added line src/agent/claude/cli/start-cli.sh:28 contains the hardcoded host-path example $HOME/.sutando/repo. Per the repository path scan requirement, please genericize/remove that literal.

  4. This changes startup/auth/network routing. The focused stubs pass (provider-env-guards and start-cli-provider-as-core), but the PR still needs the requested real post-restart alternative-provider round trip (redacted transcript) before merge.

Also rebase onto current main: GitHub reports CONFLICTING/DIRTY and the branch is 327 commits behind. After rebasing, rerun the focused suites and current required checks. This review replaces my stale approval on 742be422; the current provider guard tests themselves passed.

@bassilkhilo-ag2 bassilkhilo-ag2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independently verified — agree with the existing CHANGES_REQUESTED, keeping this short since it doesn't change the verdict.

Not ready to merge: mergeStateStatus is DIRTY/CONFLICTING (as of 2026-07-30), and the head commit (ca7efb8e, 2026-07-02) predates every blocking review comment on this thread — nothing has landed to address them.

Confirmed still present by reading the current diff directly (not relying on the thread's summaries):

  • src/agent/claude/provider-env.sh: claude_provider_export_env exports ANTHROPIC_BASE_URL from cfg_provider unconditionally before checking the ANTHROPIC_SUBSCRIPTION sentinel, then returns rc=2 for the sentinel case. src/agent/claude/cli/dashp.sh's rc==2 branch only logs "running on subscription" and never unsets ANTHROPIC_BASE_URL. Net effect: core_config.provider set + auth_env=ANTHROPIC_SUBSCRIPTION sends the subscription OAuth session to the custom provider endpoint while claiming subscription auth — exactly the failure scenario qingyun-wu flagged 07-09, still reproducible in the current diff.
  • src/agent/claude/cli/start-cli.sh (subscription branch, ~line 230): only ANTHROPIC_API_KEY is cleared; ANTHROPIC_AUTH_TOKEN is left as-is with the stated rationale that the credential-proxy rewrites the header. That's true only when launched via start-cli.sh under startup.sh's proxy — a direct/manual launch (no proxy) still lets a stray ANTHROPIC_AUTH_TOKEN override subscription OAuth, per liususan091219's 07-03 finding.

Verdict: changes requested — the auth-isolation bug on the no-provider/subscription-sentinel path is real and unresolved, and the PR isn't mergeable in its current state.

@john-the-dev

Copy link
Copy Markdown
Collaborator

@jsun-m — I am the one holding this and it has been four weeks, so here is a clear statement of where it stands rather than more silence.

My CHANGES_REQUESTED is at ca7efb8e, which is still the head, so my block is current rather than stale — I checked before writing this. But nothing has moved since 2026-08-03 and the branch is DIRTY against main, and at +2188/-1189 a rebase is real work.

I swept my standing review blocks today and found ten of mine sitting on other people's PRs, some for a month. That is my cost to you, not the other way round, and I would rather resolve it than let it age further.

Three ways this can go, and any of them is fine by me:

  1. You still want it. Say so and I will re-review promptly rather than on my own schedule — the block is mine to clear the moment it is addressed.
  2. You want it but not now. Say that and I will stop treating the silence as ambiguous. I will leave the review in place and not nudge again.
  3. It has been overtaken. You have merged four PRs in the last two days and this one has not moved since early August; if it is no longer the direction you want, closing it is a legitimate outcome and I will not read it as anything else.

I am explicitly not proposing to close it. I nearly listed this and #1893 as abandoned earlier today, then checked your recent activity and found you are one of the more active contributors right now — which makes "deprioritised by a present author" a completely different situation from "abandoned", and only you can tell me which it is.

@john-the-dev

Copy link
Copy Markdown
Collaborator

@jsun-m — re-checked my standing block. Head unchanged at ca7efb8e since my 2026-07-30 review, so the finding stands.

This one genuinely has two distinct objections, unlike its neighbours — worth stating so you can plan one pass rather than two:

  • mine (07-30) — src/sutando_config.py:507-529 prefers core_config_dirs[type=claude] while src/sutando_config.ts does not; a cross-language parity gap.
  • @liususan091219 (07-15) — separate and smaller: the diff introduces a $HOME/.sutando/repo reference in the SUTANDO_CLAUDE_WORKING_DIR comment/example, which the hardcoded-path rule rejects.
  • @bassilkhilo-ag2 (08-03) concurs with the existing block rather than adding a third.

So: two fixes, three flags. Both are addressable in a single push.

The PR is CONFLICTING/DIRTY against main and is the largest of the stalled set (+2188/-1189), so the rebase is the real first cost here — worth doing before either fix, not after.

Ping me when pushed and I will re-review the same day.

@jsun-m

jsun-m commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

@john-the-dev — answering directly, because you asked cleanly and four weeks of silence from me is what made this cost you a sweep. Your option 3: it has been overtaken. Closing.

The capability this PR existed to add is already on main, shipped by another path.

src/sutando_config.py:555   resolve_core_runtime()  — core.runtime, _SUPPORTED_CORE_RUNTIMES
src/sutando_config.py:583   "core_config_dirs[type=claude].value (canonical — new in v0.9)"
src/sutando_config.py:606   priority-1 handling + find_core_config_dir(type_="claude")
src/agent/claude, src/agent/codex   both present

So alternative-provider support for the core exists, core_config_dirs exists as the canonical schema, and the legacy claude_sutando_config_dir is now explicitly deprecated in favour of it (py hard-fails if both are set). Rebasing +2188/-1189 across 29 files onto that would be reconstructing something already there. Not a close I would have reached without your message prompting me to actually read main — thank you for it.

Your parity objection may have outlived this PR, and I think it now describes main. Flagging rather than filing, because I have not confirmed it is a live defect:

  • sutando_config.py has the full v0.9 core_config_dirs resolver and deprecates claude_sutando_config_dir.
  • sutando_config.ts carries core_config_dirs only as a name in the known-keys set (:45). The separate set the comment calls "blocks every consumer reads with property access" (:59-67) contains claude_sutando_config_dir and not core_config_dirs. TS has CoreRuntime/core.runtime parity (:428, :434) but no type=claude resolver.

If a TS consumer resolves the claude config dir from the legacy key, then a host that migrated as Python instructs — new schema in, legacy key removed — would leave that consumer with nothing. I have not traced such a consumer, so this is a shape worth a look, not a bug I am asserting. It is your finding either way; I would rather hand it back than let it die with the PR.

@liususan091219's $HOME/.sutando/repo flag was correct too and dies with this branch.

On #1893: I am not lumping it in unexamined. It gets the same read against current main and its own answer, not an inherited one.

On the wider point: "deprioritised by a present author" was the right diagnosis and it was my ambiguity to resolve, not yours. I will not leave a standing block of yours unanswered like this again.

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

Labels

live-test-requested A change-request asks for live-test/repro evidence (voice/vision/runtime policy) request-change Has an active CHANGES_REQUESTED review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants