feat(task): opt-in sub-agent model autorouting with generated tiers and /routing setup - #3764
feat(task): opt-in sub-agent model autorouting with generated tiers and /routing setup#3764Yeachan-Heo wants to merge 40 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0462a8ba2
ℹ️ 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".
| await this.ctx.settings.commitAtomicBatchWithCurrent(() => | ||
| buildAutoroutingSettingsBatch({ | ||
| tiers: preview.tiers, | ||
| setup: preview.setup, | ||
| provenance: preview.provenance, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
When another process edits the generated tiers after the settings snapshot was loaded—or between #assertSmartRoutingNotHandEdited and this write—the guard examines stale in-memory data, while this callback ignores the current on-disk configuration supplied by commitAtomicBatchWithCurrent. Apply therefore silently overwrites the concurrent hand edit without the required confirmation; Refresh repeats the same pattern. Re-evaluate the provenance/tier fingerprint from current inside the locked callback before returning the patches.
Useful? React with 👍 / 👎.
| const attempts: AutoroutingAttempt[] = []; | ||
| const consumed = new Set<string>(); | ||
| const skips = buildBoundedRoutingSkips(options.autoroutingSkips); | ||
| const candidates = options.autoroutingCandidates.map(boundedSelector).filter(selector => selector.length > 0); |
There was a problem hiding this comment.
When a valid custom provider/model selector exceeds 256 characters or contains NFKC-sensitive characters, this applies evidence sanitization to the selector before it is used as modelOverride. Although the original selector already matched the registry snapshot, the truncated or normalized value no longer identifies that model, so preflight can incorrectly exhaust the route. Keep the original candidates for execution and call boundedSelector only when recording routing evidence.
Useful? React with 👍 / 👎.
| if (result.routing) { | ||
| const model = result.routing.effectiveModel ?? "not-executed"; | ||
| lines.push(`${continuePrefix}${theme.fg("dim", `Routing: ${model} ${result.routing.note ?? ""}`)}`); |
There was a problem hiding this comment.
When a custom provider reports a model identifier containing tabs or ANSI/control sequences, effectiveModel is interpolated directly into the Task tool renderer, allowing malformed alignment or terminal escape injection; the new routing note is also rendered without the shared width bound. Apply replaceTabs() and truncateToWidth()/ui.truncate() to the complete routing line before passing it to the theme renderer.
AGENTS.md reference: AGENTS.md:L115-L121
Useful? React with 👍 / 👎.
Computer/task backlog ownership routingExact head Signature: GJC backlog census | PR #3764 | exact-head |
Signed task-autorouting conflict handoffOwner: existing branch/PR mutation owner A three-way
Scope boundary: resolve only this rebase and directly required generated/schema/test fallout for the task-autorouting change. Do not absorb unrelated source cleanup. Reconfirm the branch head before pushing because this handoff is signed only for the SHA pair above. No duplicate mutation owner was started, and no CI workflow was rerun or cancelled. Signature: GJC task-autorouting batch lane | owner |
Owner hostile review —
|
Maintainer triage — LEFT OPEN, blocked (not mergeable in current state)Verified against current
Path to merge: rebase onto current |
f0462a8 to
fad4ab0
Compare
There was a problem hiding this comment.
💡 Codex Review
gajae-code/packages/coding-agent/src/session/artifacts.ts
Lines 388 to 390 in fad4ab0
Track and remap IDs claimed by staged saves
When a staged candidate calls save() or allocatePath(), the claimed ID is never added to #allocatedIds, so commitAttemptStaging() builds an empty mapping and copies both the artifact and its hidden .artifact-id-* claim under the candidate-local ID. If the parent already owns that ID, publication fails with destination_conflict; otherwise transcript references are not re-keyed as promised. Record asynchronous allocations and exclude or remap staging-only claim markers during publication.
ℹ️ 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".
| @@ -1,1717 +1 @@ | |||
| # Changelog | |||
|
|
|||
There was a problem hiding this comment.
This change replaces the entire package changelog—including every released section and the current Unreleased fixes—with a blank file. That permanently removes release history and leaves future release tooling without an ## [Unreleased] insertion point; retain released sections and add this feature under the existing Unreleased section instead.
AGENTS.md reference: AGENTS.md:L178-L178
Useful? React with 👍 / 👎.
| case "record": | ||
| case "constrained-record": { |
There was a problem hiding this comment.
When a user runs gjc config set task.autorouting.setup '{"schema":1,"providers":["anthropic"]}' (or sets provenance), schemaType is optional-object, but this switch falls through to the string default because only records were added to the JSON-object branch. The command therefore persists a string that autorouting validation rejects, making these newly exposed object settings impossible to set through the generic config CLI.
Useful? React with 👍 / 👎.
| if (!autoroutingActive) return description; | ||
| return `${description}\n\n<autorouting-guidance>\nChoose a tier by agent role/type, per-call complexity, and cost intent: fast for mechanical/lookup/high-volume work where cheap tokens are the point; balanced (default) for ordinary implementation/review lanes; strong for deep design, hard debugging, or high-stakes review where the cost is justified. Provider availability/auth is enforced by deterministic code and is never an input to tier choice. Omitting tier is fine and routes as balanced.\n</autorouting-guidance>`; |
There was a problem hiding this comment.
This appends a newly constructed prompt fragment directly in TypeScript instead of placing it in the existing static task.md prompt template. Move the conditional autorouting guidance into a static .md prompt imported with { type: "text" } so prompt content remains reviewable and managed through the repository's required prompt surface.
AGENTS.md reference: AGENTS.md:L110-L110
Useful? React with 👍 / 👎.
| routing?: TaskRoutingEvidence; | ||
| /** Ordered, normalized autorouting candidates for the cross-phase preflight ledger. */ | ||
| autoroutingCandidates?: string[]; | ||
| autoroutingSkips?: Array<{ selector: string; code: import("../config/autorouting-contract").AutoroutingReasonCode }>; |
There was a problem hiding this comment.
The new executor option uses the forbidden import("...").Type form, and the new routing types repeat this pattern. Import AutoroutingReasonCode at the top level and reference the named type here so the implementation follows the repository's top-level-import-only contract.
AGENTS.md reference: AGENTS.md:L105-L105
Useful? React with 👍 / 👎.
| const cleanupStore = this.#stagingParentStore ?? this.#store; | ||
| const cleanupPath = this.#stagingParentStore ? this.#stagingRelativePath! : ""; | ||
| const parentCleanupPath = cleanupPath ? path.posix.dirname(cleanupPath) : ""; | ||
| let parentBefore: ReturnType<ManagedSessionDescendantStore["captureTree"]> | undefined; |
There was a problem hiding this comment.
The new staging cleanup declares its snapshot through the explicitly prohibited ReturnType<> utility; the same pattern was also introduced in the session-manager staging cleanup and TaskTool seam. Use the concrete native directory-tree snapshot type instead so the staging boundary has an explicit, stable contract.
AGENTS.md reference: AGENTS.md:L104-L104
Useful? React with 👍 / 👎.
경고 — 이 PR의 현재 head가 CHANGELOG 전체를 삭제한다머지하면 안 된다. 확인된 사실: 1바이트 — 개행 하나만 남았다. 원인은 내 쪽이다#3932(11:25:32Z 머지)가 그런데 그 결과 리베이스에서 CHANGELOG가 처음으로 진짜 충돌을 내기 시작했고, 그 충돌을 해소하는 과정에서 파일이 비워졌다. 시간대가 명확하다:
전환 비용을 예고하지 못한 건 내 잘못이다. 미안하다. 복구git fetch origin
git checkout origin/dev -- packages/coding-agent/CHANGELOG.md # 해당 패키지 경로로
# 그 다음 ## [Unreleased] 아래에 이 PR의 항목만 다시 추가
git add packages/coding-agent/CHANGELOG.md
git commit --amend --no-edit # 또는 새 커밋앞으로 리베이스에서 CHANGELOG 충돌이 나면 양쪽 항목을 모두 푸시 전에 다음으로 자가 점검할 수 있다: git cat-file -s HEAD:packages/coding-agent/CHANGELOG.md # 30만 바이트 근처여야 정상 |
18098e0 to
fb6b444
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb6b444b81
ℹ️ 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".
| const explicitMap = isMeaningfulTierMap(fragment.tiers); | ||
| if (explicitMap) { | ||
| return { active: true, map: resolveTierMap({ tiers: fragment.tiers }), source: "tiers" }; |
There was a problem hiding this comment.
When a hand-edited tier contains at least one valid selector alongside an invalid entry, isMeaningfulTierMap() succeeds and this branch activates the filtered subset even though validateAutoroutingLocal() marks the configuration invalid. For example, fast: ["anthropic/model", "pi/default"] yields an invalid schema report but still routes tasks to anthropic/model; fail closed on any local tier-map validation issue instead of silently executing a partial configuration.
Useful? React with 👍 / 👎.
| const ids = staging.getAllocatedIds(); | ||
| const start = this.#nextId; | ||
| this.#nextId += ids.length; | ||
| const mapping = new Map<string, string>(); | ||
| for (let index = 0; index < ids.length; index++) mapping.set(ids[index]!, String(start + index)); |
There was a problem hiding this comment.
When a staged attempt creates artifacts through the normal save() or allocatePath() APIs, those methods call #claimNextId() without adding the returned ID to #allocatedIds, so staging.getAllocatedIds() is empty here. The commit consequently builds no remapping and tries to publish the staged 0.* files—including .artifact-id-0—under their original names; if the parent already has artifact 0, accepted-session publication fails with a destination conflict instead of assigning the staged artifact the next parent ID.
Useful? React with 👍 / 👎.
| return path | ||
| .resolve(filePath) | ||
| .split(path.sep) | ||
| .some(segment => segment === SESSION_STAGING_DIRNAME); |
There was a problem hiding this comment.
When --session-dir is located at or below any directory named .staging, this absolute-path segment scan classifies every ordinary transcript there as an internal staged attempt. The new list filters then hide those sessions and SessionManager.open() refuses to resume them, even though the .staging segment belongs to the user-supplied path rather than GJC's staging child; classify relative to the known session root and expected staged filename layout instead.
Useful? React with 👍 / 👎.
fb6b444 to
05a914c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05a914cda9
ℹ️ 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".
| if (failure.kind === "local" && failure.op === "auth_resolve") | ||
| return { code: "credential_unavailable", advance: true }; |
There was a problem hiding this comment.
When model resolution or credential storage throws an untyped local error during auth_resolve—for example, a keychain/configuration I/O failure—classifyAutoroutingPreflightFailure records it as non-transient, but this branch still advances and labels it credential_unavailable. That masks a terminal local failure and may execute a lower-priority provider unexpectedly; only an explicitly discriminated missing-credential failure should advance.
Useful? React with 👍 / 👎.
| try { | ||
| if (await registry.getApiKey(model)) authenticated.push(selector); |
There was a problem hiding this comment.
When an OAuth refresh, credential broker, or keychain lookup stalls, cancelling the Task cannot interrupt this sequential prefilter because getApiKey receives no signal and the await is outside the executor's abort race. The task remains stuck before runSubprocess starts, potentially once per configured candidate; pass the task signal through the registry lookup or race this await against cancellation.
Useful? React with 👍 / 👎.
Owner adversarial review —
|
|
Signed CI census review — GJC, 2026-08-09 Disposition: PR-local test regressions plus superseded history; bounded hold. Current head Smallest repair lane: rebase |
REQUEST_CHANGES — owner-self-review constrained exact-head verdictReviewed exact head Exact-head integration, CI, and surface census
Blocking findings
The red exact-head preflight/staging suites also block: managed durable retry, post-acceptance terminalization, discard/advance liveness, and artifact sibling-residue cleanup fail. Rebase onto current dev; resolve the 15 conflicts while preserving target behavior; fix these fail-closed/security/concurrency defects and the remaining Codex contract violations; regenerate artifacts; then publish a new SHA with green exact-head required CI. Not LGTM. — |
probepark
left a comment
probepark
left a comment
There was a problem hiding this comment.
NEEDS-WORK — does not rebase onto current dev, so I could not evaluate the substance.
Rebase conflict
packages/coding-agent/src/task/index.ts:24-28:
<<<<<<< HEAD
import { resolveProfileBindings } from "../config/model-profiles";
=======
import { type RoutingOutcome, resolveTaskRouting } from "../config/autorouting";
>>>>>>> 5db2ad11b
I verified this against current dev directly: line 24 is now import { resolveProfileBindings } from "../config/model-profiles";. Profile-binding resolution landed in the same import region this PR uses for autorouting.
Both paths are wanted — please rebase and integrate them rather than taking one side. Taking the PR's version wholesale would drop profile bindings from dev; taking dev's would drop autorouting. That silent-revert shape has bitten two other PRs in this repo this week, so it is worth being deliberate here.
Cleared
packages/ai/src/models.json is not touched. That was my first check — it is generated by bun run generate-models, and a hand-edit would have been an automatic block. Clean.
What I did NOT verify
Per the stop-on-conflict rule I did not evaluate this on a stale head, so none of the following is assessed:
- what proportion of the +15823 lines is generated versus hand-written
- whether a sync gate exists for whatever is generated (a generated artifact with no check drifts silently)
- whether declared provider capabilities are validated against reality, or trusted — a provider declaring a capability it lacks would route tasks to a model that cannot serve them
- fallback behaviour for a provider/model with no declared capabilities or an unknown one, which matters because new models ship constantly
- whether this changes routing for existing users by default or is opt-in — the single highest-impact question in the PR
- tests, typecheck, neighbouring suites, mutation check
Given the size (43 files, +15823/-88) these are exactly the questions worth answering properly, and answering them against a tree that will not exist after the rebase would be misleading rather than helpful.
Re-request review after the rebase and I will run the full pass, leading with the generated-versus-authored split and the default-routing question.
05a914c to
c9501b7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9501b7a01
ℹ️ 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".
| const probe = await runSubprocessOnce({ | ||
| ...options, | ||
| autoroutingPreflight: false, | ||
| preflightProbe: true, |
There was a problem hiding this comment.
When autorouting probes a candidate, this invokes the full runSubprocessOnce setup, including extensionRunner.emit({ type: "session_start" }) at line 2118, before the probe exits at the prompt fence. Extension hooks can perform network or other external side effects that the in-memory session cannot roll back, and the accepted candidate repeats the hook during its durable run, so autorouted tasks can execute session_start twice—or execute it for rejected candidates. Keep probe validation side-effect-free or explicitly suppress extension lifecycle emission for preflightProbe.
Useful? React with 👍 / 👎.
| const start = this.#nextId; | ||
| this.#nextId += ids.length; | ||
| const mapping = new Map<string, string>(); | ||
| for (let index = 0; index < ids.length; index++) mapping.set(ids[index]!, String(start + index)); |
There was a problem hiding this comment.
When another ArtifactManager or process allocates from the same artifact directory while a staged attempt is committing, this advances only the local #nextId counter and does not publish the hidden claim files used by normal allocation. The other allocator can therefore claim one of these IDs before the staged files are published, causing the no-replace publication to fail at the provider-acceptance fence and abort an otherwise valid task. Reserve each parent ID through the atomic claim path before constructing the remapping.
Useful? React with 👍 / 👎.
| for (let index = 0; index < this.#draft.providers.length; index++) { | ||
| const provider = this.#draft.providers[index] ?? ""; | ||
| const prefix = index === this.#providerCursor ? theme.fg("accent", `${theme.nav.cursor} `) : " "; | ||
| this.addChild(new Text(`${prefix}${provider}`, 0, 0)); |
There was a problem hiding this comment.
When a custom provider name contains tabs, ANSI escapes, or other control characters, this passes it directly to Text, allowing terminal escape injection or malformed panel layout; provider validation only requires a non-empty trimmed string and does not reject these characters. Sanitize and width-bound provider names—and the analogous generated selector text—before rendering them.
AGENTS.md reference: AGENTS.md:L150-L152
Useful? React with 👍 / 👎.
| typeof modelRegistry.getApiKey === "function" | ||
| ) { | ||
| preflightOperation = "auth_resolve"; | ||
| const exactKey = await awaitAbortable(modelRegistry.getApiKey(model, options.parentSessionId)); |
There was a problem hiding this comment.
In SDK/provider-session contexts where parentCredentialSessionId differs from parentSessionId, model resolution and the eventual child session use the credential-session ID, but this additional check re-resolves credentials under the parent session ID. A different sticky or exhausted account can therefore reject a candidate whose selected credential is usable, or refresh the wrong account; pass options.parentCredentialSessionId ?? options.parentSessionId consistently here.
Useful? React with 👍 / 👎.
…t rebase Rebasing onto the current dev tip pulled in 277 new catalog keys that the autorouting tier map has never seen, so check:autorouting-map failed closed on uncurated coverage. Record them as baseline skips with an explicit rationale rather than inventing tier/rank data nobody reviewed. Lore-id: 2f6d90ae Constraint: an uncurated catalog key is a skip with a rationale, never a guessed tier Confidence: high Scope-risk: narrow Reversibility: clean Tested: bun scripts/check-autorouting-tier-map.ts (gate passed, 4220 in-scope keys); focused autorouting/task/session suite 255 pass across 14 files
Step 1-4 of the preset-source unification: the smart-routing draft now comes from the user's configured provider priority instead of raw catalog iteration, so tier generation stops depending on a hardcoded provider list. Extract projectProviderOrder as the single implementation of "explicit order then first-wins catalog order" and rewrite the policy's orderedProviders on top of it. ModelRegistry.autoroutingProviderOrder() takes no session and bypasses the policy builder entirely, so it never assembles an effectiveAuth map at all: auth-independence is structural, not a convention. Auth-aware banding stays private to rank(). Catalog-absent declarations are dropped so a dead provider cannot pollute declarationFingerprint, and returned ids keep catalog spelling because the generator matches provider prefixes case-sensitively. Refresh reseeds a recorded declaration into current priority and refuses to persist an empty result. The panel gains an advisory-only provider-order hint updated through a narrow updateProviderOrderHint: the mounted modelProviderOrder subscription must not call refreshState, which would discard an unsaved draft, allowlist buffer, cursor, and confirmation. Drift stays advisory and never reaches provenance, effective state, routing, preflight, or evidence. The preset layer is deliberately untouched here; it is removed as one atomic cutover in the next story. Lore-id: 5c30b7ea Constraint: the order projection may never acquire auth sensitivity Constraint: a background settings change may never destroy unsaved panel state Confidence: high Scope-risk: medium Reversibility: clean Tested: new autorouting-provider-order suite (8 pass) incl. auth-invariance; hint helper units in autorouting-settings-contract (19 pass); smart-routing integration (17 pass) incl. derived seed, recorded-declaration precedence, empty-catalog entry refusal and unsaved-draft survival, each verified to fail with the fix reverted; focused suite 266 pass across 14 files; check:types clean
One wording shared by every delivery surface so the interactive, print, and SDK/ACP host paths cannot drift apart. Landed separately from the cutover so the two implementation slices do not contend on this file. Lore-id: 71c4ab02 Confidence: high Scope-risk: narrow Reversibility: clean Tested: check:types (clean)
…union Preset tier maps were a second source of truth for provider priority. With the draft now seeded from the configured provider order, the hardcoded layer has no remaining job, so it is removed outright rather than left as a dead path. Removes AUTOROUTING_PRESETS, AUTOROUTING_PRESET_IDS, AutoroutingPresetId, and resolveTierMap (normalizeTierMap becomes the one normalization entry point), the task.autorouting.preset setting, and the preset branches in validation, the panel, and /routing. The source union collapses everywhere it was observable: AutoroutingEffective, RoutingOutcome, and TaskRoutingEvidence, including the durable receipt, the summary XML projection, and the terminal renderer. With one possible value left it was about to become a meaningless required field on a public contract. /routing status now reads a settings-only snapshot of effective state, raw tiers, and provenance, labelling absent, matching, mismatched, and malformed provenance distinctly. Malformed fails closed as hand-authored instead of being reported as generated. The helper still touches no registry or auth state. BREAKING: task.autorouting.preset is gone, so a preset-only configuration routes manually until tiers are generated. Public TypeScript contracts under ./config/* and ./task/* lose the symbols listed in the changelog, and receipt/summary note values change format. Lore-id: 8ad3f215 Constraint: no migration, alias, or fallback for the removed preset key Constraint: malformed provenance must never be reported as generated Confidence: high Scope-risk: wide Reversibility: migration-needed Tested: 184 pass across 12 focused autorouting/task/slash-command/tool-catalog suites; check:types clean; check:schemas regenerated and in sync Not-tested: host-computed inactive warning delivery, which lands next
Removing the preset key silently disables routing for anyone who only had a preset: load logging reports the raw schema report, the merged diagnostic needs an explicit config doctor call, and task startup just omits its guidance. The user would see nothing. The host now decides once, where it already has settings: enabled but not effectively active pushes the shared warning onto configWarnings for the interactive and print frontends, and emits the same constant as an internal autorouting notice for the agent wire. The two deliveries cover disjoint consumers, since neither frontend reads notice events and ACP cannot see configWarnings. ACP captures the startup notice onto its session record instead of rendering it early, because updates published before the client learns the session id are dropped, and republishes it exactly once during deferred bootstrap beside the existing auth-failure branch. No client-side reconstruction, no new SDK query, and no raw autorouting projection: config.list/get, getSdkConfigItems, and configValues are untouched, and emitNotice is already excluded from the public operation inventory. A session with usable tiers never creates the notice at all, so suppression is not load-bearing. Lore-id: 4e19c7d8 Constraint: the warning wording lives in exactly one constant, uninterpolated Constraint: no public raw config projection for a diagnostic Confidence: medium Scope-risk: medium Reversibility: clean Tested: acp-lazy-startup, acp-event-mapper, sdk-acp-production-path, task-autorouting and autorouting-settings-contract (75 pass); check:types clean; direct host probe with autorouting enabled and no tiers observed exactly one configWarnings entry Not-tested: AC10b end-to-end delivery over a live broker, which could not be exercised here; see the recorded review blocker
…ing notice The notice was also emitted as a session event so ACP could republish it at bootstrap. That path can never fire. AgentSession.#emit iterates #eventListenerSnapshot synchronously with no buffering, and the emit sat inside the session factory before it returns, so the listener set is still the frozen empty array and the event is dropped unconditionally. The ACP ingress capture and the bootstrap republication were therefore dead code, which is exactly what AC10b was written to catch. Keep the part that works: the host still decides once and records the warning on session.configWarnings, which interactive and print already read. ACP delivery is not implemented rather than implemented-but-unreachable; it needs the Critic-sanctioned narrow host-owned read, since the adapter reaches the host over the broker and has no access to Settings or the session object. Lore-id: b53f0a91 Constraint: never ship a delivery path that structurally cannot fire Confidence: high Scope-risk: narrow Reversibility: clean Tested: acp-lazy-startup, acp-event-mapper, sdk-acp-production-path, task-autorouting, autorouting-settings-contract and smart-routing integration (92 pass); check:types clean
…play ring The previous attempt emitted the notice from the session factory before any subscriber existed, so it was dropped and had to be removed. The fix is the delivery channel, not the determination: SessionEventStream retains a 256-frame ring and replays retained current-generation events, so a notice published at host start reaches an ACP client that attaches later. The session factory still decides once and keeps configWarnings for interactive and print. It now threads only an internal boolean through the host runtime seam, and the host publishes the existing notice event into the ring right after events.restart() during start(). ACP captures the replayed notice, clears it before publishing so a bootstrap can emit at most once, and republishes beside the auth-failure branch. No new public query, no config projection, no new event kind: the ACP mapper already renders notice as an agent_thought_chunk. Lore-id: 9f2c61d4 Constraint: startup diagnostics must survive late subscriber attach Constraint: no public SDK surface growth for an internal diagnostic Confidence: medium Scope-risk: medium Reversibility: clean Tested: 92 pass across sdk-acp-production-path, acp-lazy-startup, acp-event-mapper, task-autorouting, autorouting-settings-contract and smart-routing integration; check:types clean Not-tested: AC10b/AC10c themselves — no test yet asserts the single replayed chunk or the zero-notice case; tracked as the remaining G004 work
AC10b is the criterion that caught the earlier dead-code delivery, so it needs a real lifecycle assertion rather than a synthetic adapter fixture. Drives a real AcpAgent through newSession against a fixture broker and asserts the client receives exactly one agent_thought_chunk carrying the shared constant, which only holds because the host publishes into the replay ring before ACP attaches. The companion case proves an active or disabled configuration produces none. Lore-id: 0c7d38be Constraint: AC10b must exercise late-attach replay, not a hand-built adapter Confidence: high Scope-risk: narrow Reversibility: clean Tested: acp-autorouting-notice 2 pass; verified to fail when the host emission is disabled, while the zero-notice case correctly still passes; product source unchanged
session-runtime only threads the internal boolean; the host owns the emission, so importing the constant here was dead and failed check:tools. Lore-id: e6b104f7 Confidence: high Scope-risk: narrow Reversibility: clean Tested: biome check across 3653 files exits 0; check:types clean
The bootstrap cleared record.routingInactiveNotice before awaiting the publish, which broke two things the auth-failure precedent gets right: a later loadSession or resumeSession legitimately re-announces the condition and would now stay silent, and a rejected publish lost the warning outright because the enclosing bootstrap task swallows failures. Read without consuming, exactly like authFailure; one emission per bootstrap already follows from bootstrap running once per attach. Also covers the accessor contract directly rather than only its pure projection: catalog spelling is preserved for the case-sensitive generator, catalog-absent declarations are dropped, configured priority leads, and duplicate spellings collapse to the first occurrence. Lore-id: 7b90e2ac Constraint: a diagnostic must survive a failed publish and re-announce on resume Confidence: high Scope-risk: narrow Reversibility: clean Tested: acp-autorouting-notice and acp-lazy-startup 3 pass; autorouting-provider-order 12 pass; check:types clean
sdk/index.ts re-exports the host namespace and ./sdk is a published entry, so adding autoroutingInactive to SessionSdkHostOptions let any consumer inject the internal diagnostic through a public API. The approved plan forbids growing the public surface for this warning. The state now travels through a package-private module keyed by the object the session factory already owns, and that module is explicitly mapped to null in the package exports so it cannot be imported from outside. The field and its layer-by-layer threading are gone from the host, runtime, and bus option types; behaviour is unchanged. Lore-id: c082da4b Constraint: an internal diagnostic must not be settable through a published type Confidence: high Scope-risk: narrow Reversibility: clean Tested: 44 pass across acp-autorouting-notice, acp-lazy-startup, acp-event-mapper and sdk-acp-production-path; AC10b still fails when the host emission is disabled, proving behaviour survived the seam change; check:types clean
Removing the autoroutingInactive threading changed createNotificationsExtension, so its declaration digest no longer matched the recorded manifest. Regenerated via --write-manifest; the diff is exactly that one digest. Lore-id: 3ad51c9e Confidence: high Scope-risk: narrow Reversibility: clean Tested: telegram-daemon-generation-guard --validate-current-tree
The flag was publicly settable once; nothing stopped it regressing. Asserts the two null export-map entries that block the internal module, that the host barrel does not re-export it, and that no published option type declares the field. Lore-id: 1d4f6b70 Confidence: high Scope-risk: narrow Reversibility: clean Tested: 4 pass; removing the export-map null entry fails the guard
AC13/D7 asked for a golden that actually runs the provider-order derivation. The four existing fixtures hand the generator an already-sorted setup, so they only ever proved that declaration order dominates tier order; none of them touch the projection. This one starts from configured order plus catalog, runs the real projection, and pins the resulting bytes. It also pins the two behaviours that motivated the accessor: a configured provider missing from the catalog is dropped before it can reach setup.providers and pollute declarationFingerprint, and catalog order supplies the remainder. Lore-id: 5ec1f0d3 Confidence: high Scope-risk: narrow Reversibility: clean Tested: autorouting-generator 8 pass; removing the catalog-append branch from projectProviderOrder fails this fixture
Three gaps, all real. The selector check claimed to validate "every generated tier selector" but tested three hardcoded strings and never touched CURATED_TIER_MAP or the generator, so deleting the preset exhaustive loop silently lost that coverage. It now walks every curated key and every selector the generator actually emits from that catalog, with a negative control for unfit selectors and an explicit note that a colon is legal inside a model id. The accessor tests reimplemented the accessor body, so they could not catch a regression inside it. The spelling-restore logic moved into projectCatalogProviderOrder, which autoroutingProviderOrder now simply calls, and the tests exercise that function directly. Real-instance tests remain for the properties observable without global settings: no parameters, catalog-only output, first-wins order, credential invariance, determinism. Also proved the model-registry baseline claim instead of inheriting it: the same four failures appear at HEAD and at pristine dev 178fc26, so they are pre-existing and unrelated to autorouting. Lore-id: 4f8ba7c1 Constraint: a test must fail when the behaviour it names is removed Confidence: high Scope-risk: narrow Reversibility: clean Tested: 57 pass across autorouting-provider-order, task-autorouting-redteam, autorouting-generator and smart-routing integration; removing the spelling restore fails 4 of them; model-registry failures diffed identical against pristine dev
The rebased feature must retain current dev's semantic manifest contract and current SDK digests.\n\nTested: bun scripts/telegram-daemon-generation-guard.ts --write-manifest
Credential selection and staged publication now stay fail-closed across preflight and uncertain native outcomes.\n\nTested: bun --cwd=packages/coding-agent run check; focused autorouting and routing command tests
Credential prefiltering now retains the model selected by full-snapshot literal-first routing.\n\nTested: bun --cwd=packages/coding-agent run check; focused task autorouting tests
The direct staged path must preserve published artifact references after an unproven native rename.\n\nTested: bun --cwd=packages/coding-agent run check; bun test packages/coding-agent/test/task-autorouting-preflight.test.ts
Generation, eager credential filtering, schema, and SDK patches now share autorouting selector semantics.\n\nTested: package check; focused autorouting settings, generator, and task tests
Credential prefiltering now uses the exact model resolved by the pinned selector.\n\nTested: package check; focused task autorouting tests
The current source digest must match the guarded semantic manifest.\n\nTested: bun scripts/telegram-daemon-generation-guard.ts --write-manifest
Autorouting changes the protected notifications extension and requires a fresh Telegram generation.\n\nTested: bun scripts/telegram-daemon-generation-guard.ts --validate-current-tree
The protected lifecycle generation test now matches the required autorouting fence.\n\nTested: notifications topic registry; daemon manifest validation; package check
Current dev already owns generation 163, so autorouting advances the protected Telegram lifecycle to 164.\n\nTested: daemon manifest validation; notifications topic registry test
7997341 to
8209169
Compare
|
Rebased PR #3764 onto current dev and replaced stale evidence.
Evidence: local commands executed at exact head/base on 2026-08-13; fresh Dev CI is attached to this new head and prior CI/reviews are stale. gaebal-gajae |
|
Terminal rebase evidence — fresh review is CLEAR, but exact-head Dev CI is not green, so this PR remains draft and unmerged.
Evidence captured 2026-08-13. No merge performed while CI is red. gaebal-gajae |
|
Closing PR #3764 without merge: exact-head Dev CI gaebal-gajae |
3 participants
What
Opt-in sub-agent model autorouting for the Task tool, plus a first-class setup surface.
task.autorouting.enabled(defaultfalse) activates a fixedfast/balanced/strongtier vocabulary supplied bytask.autorouting.tiersor one of the lockedtask.autorouting.presetvalues (anthropic,openai-codex,google,xai). Explicit tiers win over a preset; an omittedtieron a Task item routes asbalanced; an autorouting pin overrides the manual model chain.provider/modelIdstrings with an optional thinking suffix. Globs, bare model ids, andpi/<role>aliases are rejected by the generated config schema.Setup surface
/routingopens the smart-routing panel directly.The panel originally lived only as a row in the
/modelpreset landing, andModelSelectorComponentskips that landing entirely when the session has zero model profiles — with no way back to it. Autorouting setup was therefore unreachable for exactly the users who had not configured presets yet. The standalone entry mounts the panel itself and closes the selector on cancel, while the landing-launched path still returns to the landing./routing— open the panel (declare providers in priority order,aapply,rrefresh,cclear,ttoggle enabled)/routing on|off— fliptask.autorouting.enabled/routing status— print effective tier chainsRebase onto current dev (2026-08-13)
Rebased onto
origin/dev(12e1df1), integrating:reusableParentMcpManager) — both task spawn paths now filter exact-config tools-only managers while preserving plugin MCP inheritance#storageProfile/#storageProfileCustomreferences from the PR's#writeTerminalBreadcrumbwrapper and the unused#sidecarCacheRootDirmethod/themeslash command from dev — both/theme(immediate switch) and/routing(autorouting) entries coexist in the builtin registryreadonlysettings mode fromgjc customize doctorThe
resolveProfileBindingsimport from../config/model-profiles(flagged by @probepark in the rebase conflict) is preserved alongside the PR'sresolveTaskRoutingimport. Both paths are integrated, no silent reverts.Verification
bun --cwd=packages/coding-agent run check:types(tsc): cleanbun testautorouting + task-autorouting + model-selector + slash-command + staging + mcp-inheritance + acp-builtin suites: 165 pass, 0 failbun run check:schemas,bun --cwd=packages/coding-agent run check:autorouting-map(4220 in-scope keys): passbun scripts/check-visible-definitions.ts,bun scripts/rebrand-inventory.ts --strict: passAlso fixes three gen8 red-team tests that only passed on Linux: they built managed-store subpaths from a raw
mkdtemppath whilemanagedDirectoryRoot()canonicalizes it, so macOS's symlinked/vartmpdir tripped the root-escape guard.Note
bun scripts/verify-g002-gates.tsfails onpackage version/binary allowlist, MCP quarantine/no default discoverable MCP. This is pre-existing ondev: the gate grepspackages/coding-agent/src/tools/index.tsforread:-style registry entries, that file no longer has them after the provider-agnostic preset refactor, and this branch has zero diff againstdevfor it. Not addressed here.The
Telegram daemon generation guardCI check fails on fork PRs becauseGITHUB_BASE_SHAis not set in the fork PR event context — pre-existing infrastructure limitation. Theroot-checkbiome error (bin["가재씨"]useLiteralKeys ingajaessi-launcher-alias.test.ts) is pre-existing ondevwith zero diff in this PR.@probepark — re-review requested. The rebase is clean against current dev and both profile bindings + autorouting paths are integrated.