chore: structural cleanup — dead dirs, moves, gitignore - #3
Merged
Conversation
Pure mechanical split, no behavior changes. Pulls the 70+ tool handlers out of one monolith into api.ts (PLATFORM_URL + apiCall) plus 12 tools/*.ts files grouped by domain (workspaces, agents, secrets, files, memory, plugins, channels, delegation, schedules, approvals, discovery, remote_agents). Each module exports its handlers and a registerXxxTools(srv) function; createServer() wires them up. index.ts drops from 1697 → 89 lines. Largest new file is 183 lines. All handlers still re-exported from index.ts so existing tests that import them via "../index.js" keep working. Build clean; jest results unchanged from pre-refactor baseline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Delete empty platform/plugins/ (dead remnant; plugins/ at repo root is the real registry; router.go comment updated) - Gitignore local dev cruft: platform/workspace-configs-templates/, .agents/ (codex/gemini skill cache), backups/ - Untrack .agents/skills/ (keep local, stop tracking) - Move examples/remote-agent/ → sdk/python/examples/remote-agent/ (co-locate with the SDK it exercises); update refs in molecule_agent README + __init__ + PLAN.md + the demo's own README - Move docs/superpowers/plans/ → plugins/superpowers/plans/ (plans were written by the superpowers plugin's writing-plans subskill; belong with the plugin, not under docs) - Add tests/README.md explaining the unit-tests-per-package + root-E2E split so new contributors don't ask - Add docs/README.md explaining why site tooling lives under docs/ rather than a separate docs-site/ (VitePress ergonomics) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The get_remote_agent_setup_command handler emitted \`python3 -m examples.remote-agent.run\` — an invalid Python module path (dashes not allowed in module names), so the command never actually worked. Replace with a direct \`python3 -c "..."\` snippet that imports from \`molecule_agent\` (the real SDK module) and points to the demo script for reference. Fixes the pre-existing jest failure in \`handleGetRemoteAgentSetupCommand emits bash for external workspace\` that was flagged against PR #2. Updates test expectation to \`molecule_agent\` (the actual importable module name) from the never-valid \`molecule-agent\`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
added a commit
that referenced
this pull request
Apr 16, 2026
Every workspace now commits under its own name. Step 3 of the three- step agent-separation plan (platform-level git identity today; GitHub App migration follows as Option 1). ## Problem All 20+ agents in the molecule-dev template (PM, Dev Lead, Research Lead, FE, BE, DevOps, Security, QA, UIUX, Marketing roles, etc.) share a single GITHUB_TOKEN — specifically the CEO's personal PAT. So every commit, PR, and issue across the live repos ends up attributed to HongmingWang-Rabbit. `git log` can't distinguish "which agent wrote this code" from "did the CEO write it"; neither can the authority- verification rule in triage-operator/philosophy.md (rule #3). ## Fix When the provisioner starts a workspace container, it now sets: GIT_AUTHOR_NAME = "Molecule AI <Workspace Name>" GIT_AUTHOR_EMAIL = <slug>@agents.moleculesai.app GIT_COMMITTER_NAME = (same) GIT_COMMITTER_EMAIL = (same) Git prefers these env vars over `git config user.name` / `user.email`, so no per-container git-config step is needed; every commit automatically carries the right authorship. Examples (20 agents, 20 distinct identities): Frontend Engineer → frontend-engineer@agents.moleculesai.app Backend Engineer → backend-engineer@agents.moleculesai.app Product Marketing Manager → product-marketing-manager@agents.moleculesai.app UIUX Designer → uiux-designer@agents.moleculesai.app Domain `agents.moleculesai.app` is deliberate: marks the email as a bot address without resembling a real inbox. ## Operator override preserved `applyAgentGitIdentity` runs AFTER the secret-load loops in `provisionWorkspaceOpts`, but uses `setIfEmpty` so any workspace_secret with the same key wins. Teams that want custom authorship (shared org signing identity, a person-on-the-loop owner) can still set `GIT_AUTHOR_NAME` via /workspaces/:id/secrets and get their value through to git. ## What this does NOT solve (yet) - PR / issue authorship is still whoever owns GITHUB_TOKEN (the shared PAT). That needs the GitHub App migration (Option 1, next PR). The commit-level split shipped here is the prerequisite: the App path will keep these env vars and just swap the PAT for a short-lived installation token. - Existing containers continue with their pre-fix env (git env vars are baked in at container-create time). Applying is one plain `POST /workspaces/:id/restart` per agent after this merges + deploys — the restart goes through provisionWorkspace which picks up the new injection. ## Tests `agent_git_identity_test.go` — 4 behavior tests + a 10-row slug test: - fills all 4 env vars from a workspace name - operator override via pre-set env is preserved (setIfEmpty semantics) - empty / whitespace workspace name is a no-op (no `unknown@...` emails) - nil map doesn't panic (defensive) - slugify handles spaces / punctuation / edge hyphens / em-dashes All 15 cases pass; platform build clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
added a commit
that referenced
this pull request
Apr 16, 2026
Every workspace now commits under its own name. Step 3 of the three- step agent-separation plan (platform-level git identity today; GitHub App migration follows as Option 1). ## Problem All 20+ agents in the molecule-dev template (PM, Dev Lead, Research Lead, FE, BE, DevOps, Security, QA, UIUX, Marketing roles, etc.) share a single GITHUB_TOKEN — specifically the CEO's personal PAT. So every commit, PR, and issue across the live repos ends up attributed to HongmingWang-Rabbit. `git log` can't distinguish "which agent wrote this code" from "did the CEO write it"; neither can the authority- verification rule in triage-operator/philosophy.md (rule #3). ## Fix When the provisioner starts a workspace container, it now sets: GIT_AUTHOR_NAME = "Molecule AI <Workspace Name>" GIT_AUTHOR_EMAIL = <slug>@agents.moleculesai.app GIT_COMMITTER_NAME = (same) GIT_COMMITTER_EMAIL = (same) Git prefers these env vars over `git config user.name` / `user.email`, so no per-container git-config step is needed; every commit automatically carries the right authorship. Examples (20 agents, 20 distinct identities): Frontend Engineer → frontend-engineer@agents.moleculesai.app Backend Engineer → backend-engineer@agents.moleculesai.app Product Marketing Manager → product-marketing-manager@agents.moleculesai.app UIUX Designer → uiux-designer@agents.moleculesai.app Domain `agents.moleculesai.app` is deliberate: marks the email as a bot address without resembling a real inbox. ## Operator override preserved `applyAgentGitIdentity` runs AFTER the secret-load loops in `provisionWorkspaceOpts`, but uses `setIfEmpty` so any workspace_secret with the same key wins. Teams that want custom authorship (shared org signing identity, a person-on-the-loop owner) can still set `GIT_AUTHOR_NAME` via /workspaces/:id/secrets and get their value through to git. ## What this does NOT solve (yet) - PR / issue authorship is still whoever owns GITHUB_TOKEN (the shared PAT). That needs the GitHub App migration (Option 1, next PR). The commit-level split shipped here is the prerequisite: the App path will keep these env vars and just swap the PAT for a short-lived installation token. - Existing containers continue with their pre-fix env (git env vars are baked in at container-create time). Applying is one plain `POST /workspaces/:id/restart` per agent after this merges + deploys — the restart goes through provisionWorkspace which picks up the new injection. ## Tests `agent_git_identity_test.go` — 4 behavior tests + a 10-row slug test: - fills all 4 env vars from a workspace name - operator override via pre-set env is preserved (setIfEmpty semantics) - empty / whitespace workspace name is a no-op (no `unknown@...` emails) - nil map doesn't panic (defensive) - slugify handles spaces / punctuation / edge hyphens / em-dashes All 15 cases pass; platform build clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
added a commit
that referenced
this pull request
Apr 16, 2026
Every workspace now commits under its own name. Step 3 of the three- step agent-separation plan (platform-level git identity today; GitHub App migration follows as Option 1). ## Problem All 20+ agents in the molecule-dev template (PM, Dev Lead, Research Lead, FE, BE, DevOps, Security, QA, UIUX, Marketing roles, etc.) share a single GITHUB_TOKEN — specifically the CEO's personal PAT. So every commit, PR, and issue across the live repos ends up attributed to HongmingWang-Rabbit. `git log` can't distinguish "which agent wrote this code" from "did the CEO write it"; neither can the authority- verification rule in triage-operator/philosophy.md (rule #3). ## Fix When the provisioner starts a workspace container, it now sets: GIT_AUTHOR_NAME = "Molecule AI <Workspace Name>" GIT_AUTHOR_EMAIL = <slug>@agents.moleculesai.app GIT_COMMITTER_NAME = (same) GIT_COMMITTER_EMAIL = (same) Git prefers these env vars over `git config user.name` / `user.email`, so no per-container git-config step is needed; every commit automatically carries the right authorship. Examples (20 agents, 20 distinct identities): Frontend Engineer → frontend-engineer@agents.moleculesai.app Backend Engineer → backend-engineer@agents.moleculesai.app Product Marketing Manager → product-marketing-manager@agents.moleculesai.app UIUX Designer → uiux-designer@agents.moleculesai.app Domain `agents.moleculesai.app` is deliberate: marks the email as a bot address without resembling a real inbox. ## Operator override preserved `applyAgentGitIdentity` runs AFTER the secret-load loops in `provisionWorkspaceOpts`, but uses `setIfEmpty` so any workspace_secret with the same key wins. Teams that want custom authorship (shared org signing identity, a person-on-the-loop owner) can still set `GIT_AUTHOR_NAME` via /workspaces/:id/secrets and get their value through to git. ## What this does NOT solve (yet) - PR / issue authorship is still whoever owns GITHUB_TOKEN (the shared PAT). That needs the GitHub App migration (Option 1, next PR). The commit-level split shipped here is the prerequisite: the App path will keep these env vars and just swap the PAT for a short-lived installation token. - Existing containers continue with their pre-fix env (git env vars are baked in at container-create time). Applying is one plain `POST /workspaces/:id/restart` per agent after this merges + deploys — the restart goes through provisionWorkspace which picks up the new injection. ## Tests `agent_git_identity_test.go` — 4 behavior tests + a 10-row slug test: - fills all 4 env vars from a workspace name - operator override via pre-set env is preserved (setIfEmpty semantics) - empty / whitespace workspace name is a no-op (no `unknown@...` emails) - nil map doesn't panic (defensive) - slugify handles spaces / punctuation / edge hyphens / em-dashes All 15 cases pass; platform build clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
added a commit
that referenced
this pull request
Apr 16, 2026
Every workspace now commits under its own name. Step 3 of the three- step agent-separation plan (platform-level git identity today; GitHub App migration follows as Option 1). ## Problem All 20+ agents in the molecule-dev template (PM, Dev Lead, Research Lead, FE, BE, DevOps, Security, QA, UIUX, Marketing roles, etc.) share a single GITHUB_TOKEN — specifically the CEO's personal PAT. So every commit, PR, and issue across the live repos ends up attributed to HongmingWang-Rabbit. `git log` can't distinguish "which agent wrote this code" from "did the CEO write it"; neither can the authority- verification rule in triage-operator/philosophy.md (rule #3). ## Fix When the provisioner starts a workspace container, it now sets: GIT_AUTHOR_NAME = "Molecule AI <Workspace Name>" GIT_AUTHOR_EMAIL = <slug>@agents.moleculesai.app GIT_COMMITTER_NAME = (same) GIT_COMMITTER_EMAIL = (same) Git prefers these env vars over `git config user.name` / `user.email`, so no per-container git-config step is needed; every commit automatically carries the right authorship. Examples (20 agents, 20 distinct identities): Frontend Engineer → frontend-engineer@agents.moleculesai.app Backend Engineer → backend-engineer@agents.moleculesai.app Product Marketing Manager → product-marketing-manager@agents.moleculesai.app UIUX Designer → uiux-designer@agents.moleculesai.app Domain `agents.moleculesai.app` is deliberate: marks the email as a bot address without resembling a real inbox. ## Operator override preserved `applyAgentGitIdentity` runs AFTER the secret-load loops in `provisionWorkspaceOpts`, but uses `setIfEmpty` so any workspace_secret with the same key wins. Teams that want custom authorship (shared org signing identity, a person-on-the-loop owner) can still set `GIT_AUTHOR_NAME` via /workspaces/:id/secrets and get their value through to git. ## What this does NOT solve (yet) - PR / issue authorship is still whoever owns GITHUB_TOKEN (the shared PAT). That needs the GitHub App migration (Option 1, next PR). The commit-level split shipped here is the prerequisite: the App path will keep these env vars and just swap the PAT for a short-lived installation token. - Existing containers continue with their pre-fix env (git env vars are baked in at container-create time). Applying is one plain `POST /workspaces/:id/restart` per agent after this merges + deploys — the restart goes through provisionWorkspace which picks up the new injection. ## Tests `agent_git_identity_test.go` — 4 behavior tests + a 10-row slug test: - fills all 4 env vars from a workspace name - operator override via pre-set env is preserved (setIfEmpty semantics) - empty / whitespace workspace name is a no-op (no `unknown@...` emails) - nil map doesn't panic (defensive) - slugify handles spaces / punctuation / edge hyphens / em-dashes All 15 cases pass; platform build clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Apr 16, 2026
Code review fixes: - 🟡 #1: Replace python3 with jq in Dockerfile template stages (~50MB → ~2MB) - 🟡 #2: Add clone count verification to scripts/clone-manifest.sh (set -e + expected vs actual count check — fails build if any clone fails) - 🟡 #3: Drop 'unsafe-eval' from CSP (not needed for Next.js production standalone builds, only dev mode). Updated test assertion. - 🟡 #4: Remove broken pyproject.toml from workspace-template/ (it claimed to package as molecule-ai-workspace-runtime but the directory structure didn't match — the real package ships from the standalone repo) - 🔵 #1: Add version-pinning TODO comment to manifest.json - 🔵 #3: Add full repo URLs + test counts for SDK/MCP/CLI/runtime in CLAUDE.md Security (GitGuardian alert): - Removed Telegram bot token (8633739353:AA...) from template-molecule-dev pm/.env — replaced with ${TELEGRAM_BOT_TOKEN} placeholder - Removed Claude OAuth token (sk-ant-oat01-...) from template-molecule-dev root .env — replaced with ${CLAUDE_CODE_OAUTH_TOKEN} placeholder - Both tokens need immediate rotation by the operator Tests: Platform middleware tests updated + all pass.
This was referenced Apr 17, 2026
3 tasks
molecule-ai Bot
pushed a commit
that referenced
this pull request
Apr 21, 2026
chore: structural cleanup — dead dirs, moves, gitignore
molecule-ai Bot
pushed a commit
that referenced
this pull request
Apr 21, 2026
Every workspace now commits under its own name. Step 3 of the three- step agent-separation plan (platform-level git identity today; GitHub App migration follows as Option 1). ## Problem All 20+ agents in the molecule-dev template (PM, Dev Lead, Research Lead, FE, BE, DevOps, Security, QA, UIUX, Marketing roles, etc.) share a single GITHUB_TOKEN — specifically the CEO's personal PAT. So every commit, PR, and issue across the live repos ends up attributed to HongmingWang-Rabbit. `git log` can't distinguish "which agent wrote this code" from "did the CEO write it"; neither can the authority- verification rule in triage-operator/philosophy.md (rule #3). ## Fix When the provisioner starts a workspace container, it now sets: GIT_AUTHOR_NAME = "Molecule AI <Workspace Name>" GIT_AUTHOR_EMAIL = <slug>@agents.moleculesai.app GIT_COMMITTER_NAME = (same) GIT_COMMITTER_EMAIL = (same) Git prefers these env vars over `git config user.name` / `user.email`, so no per-container git-config step is needed; every commit automatically carries the right authorship. Examples (20 agents, 20 distinct identities): Frontend Engineer → frontend-engineer@agents.moleculesai.app Backend Engineer → backend-engineer@agents.moleculesai.app Product Marketing Manager → product-marketing-manager@agents.moleculesai.app UIUX Designer → uiux-designer@agents.moleculesai.app Domain `agents.moleculesai.app` is deliberate: marks the email as a bot address without resembling a real inbox. ## Operator override preserved `applyAgentGitIdentity` runs AFTER the secret-load loops in `provisionWorkspaceOpts`, but uses `setIfEmpty` so any workspace_secret with the same key wins. Teams that want custom authorship (shared org signing identity, a person-on-the-loop owner) can still set `GIT_AUTHOR_NAME` via /workspaces/:id/secrets and get their value through to git. ## What this does NOT solve (yet) - PR / issue authorship is still whoever owns GITHUB_TOKEN (the shared PAT). That needs the GitHub App migration (Option 1, next PR). The commit-level split shipped here is the prerequisite: the App path will keep these env vars and just swap the PAT for a short-lived installation token. - Existing containers continue with their pre-fix env (git env vars are baked in at container-create time). Applying is one plain `POST /workspaces/:id/restart` per agent after this merges + deploys — the restart goes through provisionWorkspace which picks up the new injection. ## Tests `agent_git_identity_test.go` — 4 behavior tests + a 10-row slug test: - fills all 4 env vars from a workspace name - operator override via pre-set env is preserved (setIfEmpty semantics) - empty / whitespace workspace name is a no-op (no `unknown@...` emails) - nil map doesn't panic (defensive) - slugify handles spaces / punctuation / edge hyphens / em-dashes All 15 cases pass; platform build clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
molecule-ai Bot
pushed a commit
that referenced
this pull request
Apr 21, 2026
Code review fixes: - 🟡 #1: Replace python3 with jq in Dockerfile template stages (~50MB → ~2MB) - 🟡 #2: Add clone count verification to scripts/clone-manifest.sh (set -e + expected vs actual count check — fails build if any clone fails) - 🟡 #3: Drop 'unsafe-eval' from CSP (not needed for Next.js production standalone builds, only dev mode). Updated test assertion. - 🟡 #4: Remove broken pyproject.toml from workspace-template/ (it claimed to package as molecule-ai-workspace-runtime but the directory structure didn't match — the real package ships from the standalone repo) - 🔵 #1: Add version-pinning TODO comment to manifest.json - 🔵 #3: Add full repo URLs + test counts for SDK/MCP/CLI/runtime in CLAUDE.md Security (GitGuardian alert): - Removed Telegram bot token (8633739353:AA...) from template-molecule-dev pm/.env — replaced with ${TELEGRAM_BOT_TOKEN} placeholder - Removed Claude OAuth token (sk-ant-oat01-...) from template-molecule-dev root .env — replaced with ${CLAUDE_CODE_OAUTH_TOKEN} placeholder - Both tokens need immediate rotation by the operator Tests: Platform middleware tests updated + all pass.
molecule-ai Bot
pushed a commit
that referenced
this pull request
Apr 22, 2026
…ession flagged - PR #1582 (staging-to-main-p0-fix): mergeable=True, 1155 commits, correct exec-form ✅ — add as item #12, recommend as primary path to main - PR #1583: REQUEST_CHANGES review posted — Cmd regression to string concat, same as F1502. Add as item #11. - Update item #3 (superseded by #1582) and #4 (merged) - Update branch HEAD to 034601c - Add PRs row to Affected Systems Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
6 tasks
4 tasks
3 tasks
molecule-ai Bot
pushed a commit
that referenced
this pull request
Apr 24, 2026
…rift-risk-3)
### Repro
On Canvas: create a workspace named "Hermes Agent" (runtime=langgraph,
model=langgraph default). Open the Config tab, switch the model to a
Minimax provider + Minimax token, hit Save and Restart. The model
reverts to the default on every restart.
### Root cause
`workspace_restart.go` called `findTemplateByName(configsDir, wsName)`
unconditionally when the request body had no explicit `template`:
template := body.Template
if template == "" {
template = findTemplateByName(h.configsDir, wsName)
}
`findTemplateByName` normalises the name ("Hermes Agent" → "hermes-agent")
and ALSO scans every template's `config.yaml` for a matching `name:`
field — a two-layer match that returns non-empty for any workspace whose
name coincides with a template dir OR any template whose config.yaml
claims the same display name.
When the match returned non-empty, the restart handler set
`templatePath = <template>` and the provisioner rewrote the workspace's
config volume from the template on `Start`. The Canvas Save+Restart
flow's `PUT /workspaces/:id/files/config.yaml` had already written the
user's edits to the volume — those got clobbered.
The comment immediately below (line 187) ALREADY said:
// Apply runtime-default template ONLY when explicitly requested
// via "apply_template": true. Use case: runtime was changed via
// Config tab — need new runtime's base files. Normal restarts
// preserve existing config volume (user's model, skills, prompts).
The code contradicted the comment. The design intent was right; the
implementation short-circuited it. Matches drift-risk #3 in #1822's
Docker-vs-EC2 parity tracker ("Config-tab save must flush to DB before
kicking off restart, not deferred").
### Fix
Extracted the template-resolution chain into a pure function
`resolveRestartTemplate(configsDir, wsName, dbRuntime, body)` in a new
`restart_template.go`. Gated the name-based auto-match on
`body.ApplyTemplate`:
1. Explicit `body.Template` → always honoured (caller consent).
2. `ApplyTemplate=true` → name-based auto-match (prior behaviour).
3. `RebuildConfig=true` → org-templates recovery fallback (#239).
4. `ApplyTemplate=true` + dbRuntime → `<runtime>-default/`.
5. Fall through → empty path + "existing-volume" label. Provisioner
reuses the volume. This is the path Canvas Save+Restart now hits.
The handler now calls this helper and uses the returned path directly.
Duplicate rebuild_config blocks at lines 167-186 were consolidated into
the helper's single tier-3 case in passing.
### Abstraction win
`resolveRestartTemplate` is a pure function — no gin context, no DB, no
network. Takes a struct input, returns two strings. The whole priority
chain is unit-testable in a temp dir, which is exactly what
`restart_template_test.go` does.
### Tests
`restart_template_test.go` — 8 table-style unit tests covering every
branch of the priority chain:
- DefaultRestart_PreservesVolume — the regression. Even when a
template's config.yaml `name:` field matches the workspace name
exactly (worst case), a default restart MUST return empty path.
- ExplicitTemplate_AlwaysHonoured — caller-by-name, any mode.
- ApplyTemplate_NameMatch — opt-in restores the auto-match.
- ApplyTemplate_RuntimeDefault — runtime-change flow still works.
- ApplyTemplate_NoMatch_NoRuntime — fallback to existing-volume.
- InvalidExplicitTemplate_ProceedsWithout — traversal attempt stays
inside root, falls through cleanly.
- NonExistentExplicitTemplate — deleted/missing template falls through.
- Priority_ExplicitBeatsApplyTemplate — explicit Template wins over
name-match when both fire.
Full handlers race suite (`go test -race ./internal/handlers/`) still
passes — existing Restart-handler tests unchanged.
### Blast radius
Any restart caller that omitted `apply_template: true` and relied on
name-matching auto-applying a template is now a behaviour change.
Identified call sites in this repo:
- Canvas Save+Restart button (store/canvas.ts) — explicitly the
flow this commit fixes, definitely wanted the fix.
- Canvas Restart button (same file) — same semantics; user expects
a restart, not a template reset.
- Auto-restart sweeper (#1858) — never passes apply_template and
depends on the existing volume having valid config. Separately,
`workspace_provision.go`'s #1858 recovery path detects empty
volumes and auto-applies `<runtime>-default` without going
through findTemplateByName, so recovery is unaffected.
- RestartByID — internal callers; audited, all intended "restart
as-is", none relied on auto-template-match.
No SaaS parity impact — this is a handler behaviour fix that applies
equally to Docker and EC2 backends (both use the same Restart handler
before dispatching to their respective provisioners).
Refs #1822 drift-risk-3.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 29, 2026
Open
4 tasks
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
Addresses both review subagents' REQUEST_CHANGES verdicts on PR #1929: Code review (correctness) - #1: Move schedule seeding to AFTER provisionWorkspaceAuto succeeds so the scheduler never fires cron rows against a workspace whose backend never wired. Failed-backend workspaces no longer end up with orphan template_schedules rows. - #2: seedTemplateSchedules now returns (seeded, skipped int) so the caller can observe partial-seed states; workspace.go Create logs the (seeded, skipped) pair when skipped > 0, surfacing silent partial-loss that the prior (int) return masked. Security review (hostile-template defenses) - #3 / #4: parseTemplateSchedules reads config.yaml through an io.LimitReader bounded by maxTemplateConfigYAMLBytes (1 MiB) and rejects files over the cap before yaml.Unmarshal runs. Defends against billion-laughs / anchor-explosion DoS. - #3: schedules slice length capped at maxTemplateSchedules (100, 10x the largest current production grid). Hostile template with 50k schedules now rejected at parse time, not after 50k inserts. - #3: cron_expr length capped at maxScheduleCronExprLen (128) per schedule; resolved prompt body capped at maxSchedulePromptBytes (16 KiB) per schedule. Oversized entries are skipped (counted as `skipped`) so one bad row doesn't break the rest. - #3: Seed loop honours ctx.Err() so an aborted Create request stops further inserts rather than running to completion on a dead goroutine. - #8: Schedule names quoted via %q in all log lines so CRLF in a hostile name can't injection-pollute stdout/Loki. Tests - TestParseTemplateSchedules_RejectsOversizeFile — gate against the LimitReader cap (1 MiB + 1 byte of '#'). - TestParseTemplateSchedules_RejectsTooManySchedules — gate against the schedule-count cap (maxTemplateSchedules + 1 minimal entries). - Full handlers test suite still green (17.4s). Non-fix surface - Code-review #3 (runtime-default fallback also seeds): runtime- default templates do not currently ship a schedules: block so this is benign in practice; documented behavior in the comment. - Code-review #4 (files_dir in workspace-template config.yaml): not part of the current template_registry schema; flagged for follow-up if templates start declaring files_dir. - Security-review #7 (cron prompt as agent self-message escalation vector): out of scope per security reviewer's own note; tracked separately. Will file an issue. Verified locally: go vet ./... → clean go build ./... → clean gofmt -d <changed files> → clean go test ./internal/handlers/ → PASS (7 unit tests for parser, full suite 17.4s) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…riveProviderFromModelSlug
The provider-SSOT closure: with the registry-derived provider model
(P0-P4) flowing through every decision point — proxy (P1), billing
(P2-B), templates (P3 PR-A/B), provisioner (P3 PR-C) — the
LLM_PROVIDER workspace_secret has no reader left on core. This PR
retires:
- WorkspaceHandler.Create's setProviderSecret writes (the
payload.LLMProvider and deriveProviderFromModelSlug-derived
write paths). payload.LLMProvider is preserved on the request
struct for backwards-compat with older canvases that still send
it; the value is intentionally ignored. Coverage moved to
TestWorkspaceCreate_FirstDeploy_OnlyPersistsMODEL (asserts only
the MODEL secret is written, even on a slug-prefixed model that
pre-P4 would have triggered an LLM_PROVIDER write).
- SecretsHandler.SetProvider / GetProvider gin handlers + the
setProviderSecret helper. Both route registrations now point at
handlers.ProviderEndpointGone, which returns 410 Gone with a
structured PROVIDER_ENDPOINT_RETIRED body so older canvases that
still call PUT /provider on Save fail loud rather than silently
writing into a vanished row. Coverage: TestPutProvider_410Gone +
TestGetProvider_410Gone + TestProviderEndpointGone_BodyShape.
- deriveProviderFromModelSlug (retire-list #3) — the hand-rolled
35-arm slug-prefix→provider switch in workspace_provision.go.
Its only caller was Create's setProviderSecret write; the
derivation now flows through providers.Manifest.DeriveProvider
against the registry SSOT at every decision point. The drift
test (derive_provider_drift_test.go) that pinned parity with the
hermes template's derive-provider.sh is deleted with it. The
shell script remains the in-container fallback; its byte-identity
with the registry view of hermes is a P4 follow-up gated on
registry data growth (see codegen of hermes config.yaml from the
registry).
- loadWorkspaceSecrets LLM_PROVIDER drop (defence-in-depth):
any straggler workspace_secrets or global_secrets row keyed
LLM_PROVIDER is filtered out before envVars is built, so a
rolling deploy (new code, old DB) cannot re-emit the retired key
into the CP-side provisioner env.
- Canvas: ConfigTab.tsx no longer GETs or PUTs
/workspaces/:id/provider, and the provider→billing-mode linkage
(internal#703 Gap 2) is retired together — P2-B moved the
platform-vs-byok decision to ResolveLLMBillingModeDerived, which
derives the provider from (runtime, model). The provider
dropdown still renders for display so users can preview the
derived value locally. The two retired vitest suites
(ConfigTab.provider, ConfigTab.billingMode) are replaced with
documentation files pointing at the new coverage.
- Migration 20260528000000_drop_llm_provider_workspace_secret
removes any straggler rows from workspace_secrets. Idempotent:
a fresh tenant with zero LLM_PROVIDER rows produces a 0-row
delete. The .down.sql is a documented no-op (the rows cannot
be reconstituted from a soft-delete, and the writers are gone).
Behavior delta — explicitly tested:
- Registered (runtime, model) workspace → 201, provider derived,
no LLM_PROVIDER stored. UNCHANGED for the runtime-visible
`provider:` in /configs/config.yaml (CP-side commit derives it
from the same registry).
- PUT /workspaces/:id/provider → 410 Gone {code:
PROVIDER_ENDPOINT_RETIRED, error, issue: internal#718}. Was 200
with a workspace_secrets write.
- GET /workspaces/:id/provider → 410 Gone. Was 200 + {provider,
source}.
- WorkspaceHandler.Create with a slug-prefixed model (e.g.
minimax/MiniMax-M2.7) + an explicit llm_provider in the payload
→ only the MODEL workspace_secret is written. Pre-P4 both rows
were written.
- Existing workspace with an LLM_PROVIDER row → migration drops
it at next deploy; loadWorkspaceSecrets filters it defensively
in the interim.
Five-Axis review notes:
- Correctness: the four readers of stored LLM_PROVIDER (core
GetProvider, core loadWorkspaceSecrets, CP resolveModelAndProvider,
CP ValidateProviderEnv) are all migrated in this PR + the
CP-side commit. Audit query trail in the brief; the empirical
finding is that no fifth reader exists (verified across both
repos via grep of LLM_PROVIDER, setProviderSecret, SetProvider,
GetProvider, llm_provider).
- Tests: TDD red→green for the 410 Gone shape; SQL-mock for the
"no LLM_PROVIDER write on Create" contract; existing P2-B
billing tests confirm the derived-provider billing path is
untouched (the regression risk this PR could have created).
- Backward-compat: payload.LLMProvider preserved on the
CreateWorkspacePayload struct; the canvas still sends it; the
server ignores it. Older canvases that PUT /provider get a loud
410 with a recognizable code so they can stop calling.
- Rollback: revert the migration + revert this commit; the
LLM_PROVIDER workspace_secret writers stay gone (the PUT route
has no handler symbol to wire back without a separate revert).
- Observability: provider derivation is logged in
applyPlatformManagedLLMEnv (existing P2-B emission); no new
structured-event surface added — the retirement is silent at
the request boundary and the 410 Gone surface is the
operator-facing signal.
cp#362 anthropic passthrough untouched. P1 proxy ResolveUpstream
untouched. P2-B billing derives via DeriveProvider — still reads
the same derivation, never the stored LLM_PROVIDER. P3 PR-A
templates-from-registry + P3 PR-C ValidateProviderEnv-from-registry
untouched. P4 PR-2 hard-reject 422 untouched.
NOT MERGED.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
added a commit
that referenced
this pull request
Jun 12, 2026
…st, retire hardcoded provider vocab (#4/#5) P3 item 2. The canvas Provider/Model selector + Config-tab billing-mode now consume the registry-served GET /templates fields (registry_backed / registry_providers / registry_models from PR-A) instead of re-deriving provider knowledge client-side. Retires the hardcoded vocabularies as the PRIMARY path: - ProviderModelSelector (#4): new buildProviderCatalogFromRegistry(providers, models) builds the dropdown catalog from the registry payload — provider label = registry display_name, bucket = DERIVED provider, billing + auth_env from the registry — instead of inferVendor / VENDOR_LABELS / BARE_VENDOR_PATTERNS. The selector takes an optional pre-built `catalog` prop and uses it verbatim when supplied. inferVendor/buildProviderCatalog remain ONLY as the fallback for non-registry runtimes / older backends. - ConfigTab (#5): when the selected runtime is registry-backed, the provider catalog + selector models come from registry_providers/registry_models, and billingModeForSelectedProvider(provider, catalog) reads the DERIVED provider's billing_mode off the registry catalog. The hardcoded billingModeForProvider ('' | 'platform' → platform_managed else byok) stays as the fallback only. So the billing-mode the UI shows/sends reflects the DERIVED provider (folds in the closed #1931's canvas intent). Federation/back-compat preserved: a non-registry runtime (external/mock/kimi/ future third-party) or an older backend that doesn't serve the registry fields yields registry_backed=false → the canvas keeps the template-served models + its heuristic, unchanged. NO hard-reject (the canvas just can't render an option the registry didn't serve for registry-backed runtimes). Out of scope (per brief): the manifest runtime allowlist (SUPPORTED_RUNTIME_VALUES / FALLBACK_RUNTIME_OPTIONS) is NOT a provider vocabulary and is untouched; PUT /workspaces/:id/provider is NOT retired (that CTO #3 follow-through is a later phase). Stacked on PR-A (workspace-server registry-served /templates); re-target to main after PR-A merges. TDD: ProviderModelSelector.registry.test.tsx (catalog bucketed by derived provider, labelled from display_name, carries billing_mode + auth_env, no empty buckets), ConfigTab.registryBilling.test.tsx (billing reads registry catalog; falls back to the legacy rule with no catalog / unknown provider). Full canvas suite green (3380 passed / 1 skipped), tsc clean for touched files, eslint 0. internal#718 P3 — not merged; CTO merge-go after Five-Axis (UI-affecting). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #2. Tackles the structural review punch list:
Deletions / gitignore
Moves
New READMEs
Decisions left as-is (from the review)
Test plan
🤖 Generated with Claude Code