Skip to content

Commit 414589e

Browse files
authored
refactor: inline playbook session runtime, unify base-resource identity (#34)
* feat(byoc): runtime config, sessions and vaults updates Change-Id: I5d340bb58f1d0d9a68607024aa49fe0b37c1eab6 Co-developed-by: Qoder <noreply@qoder.com> * refactor(playbooks): drop unused readiness mapping readinessFromPick, PlaybookReadiness, and their tests have no production consumer: the server's agent readiness flows through the sdk's listAgentsWithReadiness instead. Remove the dead code before inlining the session runtime. Change-Id: I131ba28e7fbee9ec2aaf3a44467d27bc590dfd28 Co-developed-by: OpenCode <noreply@opencode.ai> * refactor: inline session runtime from playbooks into server The playbook session-runtime adapter layer (createPlaybookSessionRuntime, pickPlaybookAgent, PlaybookAgentIdentityMismatchError, and ~15 input/output types) lived in @openagentpack/playbooks but had exactly one production consumer: the server's playbook-session-adapter. This commit: - Moves the runtime orchestration and agent-pick logic into a new apps/server/.../playbook-session-adapter/runtime.ts with all generic type parameters instantiated to their concrete server types. - Replaces the four adapter interfaces with a single concrete PlaybookSessionRuntimeDeps type (dependency injection preserved for testability, but no cross-package abstraction). - Migrates tests to apps/server with type-safe fake constructors. - Removes session-runtime.ts from @openagentpack/playbooks and cleans up all re-exports from the package index. - Eliminates the duplicate ListPlaybookSessionsInput definition in sessions.ts (now imported from runtime.ts). Behavioral changes: none. The PlaybookAgentIdentityMismatchError still surfaces as HTTP 500 (no route-level catch); a future PR can map it to 4xx if desired. Change-Id: Ia4276ea3626877163f60529342e7da1ba3a991bd Co-developed-by: OpenCode <noreply@opencode.ai> * chore: remove playground dead dep on playbooks, update description playground's devDependency on @openagentpack/playbooks had zero imports in its source tree. Remove it. Also update the playbooks package description now that the session runtime orchestration no longer lives there. Change-Id: Ic6ac9936561f2186da6b2cdd37d8ffd0121402d3 Co-developed-by: OpenCode <noreply@opencode.ai> * fix(ci): reduce contributor friction in PR gates Change-Id: I1f4b06c06ca304d98f5a1151aad48f1bf4537cec
1 parent 9ee0f7f commit 414589e

41 files changed

Lines changed: 552 additions & 606 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/pull_request_template.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@
3636
## Behavior / risk
3737

3838
<!-- Required only when changing provider contracts, config/schema parsing, release/package files, or GitHub workflows.
39-
State the user-visible behavior or compatibility/security risk. “No user-visible behavior change” is valid for an internal refactor. -->
39+
State the user-visible behavior or compatibility/security risk. “No user-visible behavior change” is valid for an internal refactor.
40+
No special commit-message marker or history rewrite is required. -->
4041

4142
## Validation
4243

.github/workflows/ci.yml

Lines changed: 1 addition & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -50,73 +50,6 @@ jobs:
5050
BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }}
5151
run: bun scripts/verify.ts full --step "${{ matrix.step }}"
5252

53-
policy-surface:
54-
name: Policy surface
55-
runs-on: ubuntu-latest
56-
steps:
57-
- uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
58-
with:
59-
fetch-depth: 0
60-
- name: Require [policy] marker when control files change
61-
env:
62-
BASE_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }}
63-
HEAD_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}
64-
run: |
65-
set -euo pipefail
66-
67-
# The control plane: deterministic gates and the rules that govern them.
68-
# Editing any of these changes what every other check enforces, so the
69-
# change must be explicit — a [policy] marker in the commit range.
70-
patterns='
71-
.dependency-cruiser.cjs
72-
sgconfig.yml
73-
biome.json
74-
tools/architecture/
75-
.githooks/
76-
.claude/hooks/
77-
.github/workflows/
78-
scripts/lint-changed.sh
79-
scripts/verify.ts
80-
scripts/verify.test.ts
81-
scripts/pr-evidence.ts
82-
scripts/pr-evidence.test.ts
83-
scripts/release/
84-
scripts/setup-githooks.sh
85-
'
86-
87-
base="$BASE_SHA"
88-
# New branch / first push: no usable base; nothing to diff against.
89-
if [ -z "$base" ] || ! git cat-file -e "$base^{commit}" 2>/dev/null; then
90-
echo "No base commit to compare against; skipping policy-surface check."
91-
exit 0
92-
fi
93-
94-
changed=$(git diff --name-only "$base" "$HEAD_SHA")
95-
touched=""
96-
for p in $patterns; do
97-
hit=$(printf '%s\n' "$changed" | grep -F "$p" || true)
98-
if [ -n "$hit" ]; then
99-
touched="$touched$hit"$'\n'
100-
fi
101-
done
102-
103-
if [ -z "$touched" ]; then
104-
echo "No control-plane files changed."
105-
exit 0
106-
fi
107-
108-
echo "Control-plane files changed:"
109-
printf '%s' "$touched" | sed 's/^/ - /'
110-
111-
if git log "$base..$HEAD_SHA" --format=%B | grep -qF '[policy]'; then
112-
echo "Found [policy] marker in commit range. OK."
113-
exit 0
114-
fi
115-
116-
echo "::error::Control-plane files changed without a [policy] marker in any commit message."
117-
echo "Add '[policy]' to a commit message to acknowledge changing the verification harness itself."
118-
exit 1
119-
12053
maintainer-evidence:
12154
name: Maintainer evidence
12255
runs-on: ubuntu-latest
@@ -157,14 +90,13 @@ jobs:
15790
gate:
15891
name: Gate
15992
runs-on: ubuntu-latest
160-
needs: [prepare-verification, verify, policy-surface, maintainer-evidence, package-compatibility]
93+
needs: [prepare-verification, verify, maintainer-evidence, package-compatibility]
16194
if: always()
16295
steps:
16396
- name: Check results
16497
run: |
16598
if [ "${{ needs.prepare-verification.result }}" != "success" ] || \
16699
[ "${{ needs.verify.result }}" != "success" ] || \
167-
[ "${{ needs.policy-surface.result }}" != "success" ] || \
168100
[ "${{ needs.maintainer-evidence.result }}" != "success" ] || \
169101
[ "${{ needs.package-compatibility.result }}" != "success" ]; then
170102
echo "One or more required checks failed."

CONTRIBUTING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ Every PR must satisfy:
3939
2. At least one maintainer approval
4040
3. No unresolved review comments
4141

42-
We keep the contribution path open: documentation, examples, and ordinary small changes do not need a design document or a special PR label. Changes to provider contracts, configuration/schema parsing, release/package files, or GitHub workflows have a small additional requirement: fill in the **Behavior / risk** and **Validation** sections of the PR description. This lets maintainers review the contract and its evidence before reading an implementation diff.
42+
We keep the contribution path open: documentation, examples, and ordinary small changes do not need a design document, a special PR label, or a commit-message marker. Changes to provider contracts, configuration/schema parsing, release/package files, GitHub workflows, or verification policy have a small additional requirement: fill in the **Behavior / risk** and **Validation** sections of the PR description. This lets maintainers review the contract and its evidence before reading an implementation diff without asking contributors to rewrite commit history. A generated `bun.lock` change by itself is handled by audit and compatibility checks.
4343

4444
Maintainers should enable the repository settings that make the same policy effective at merge time: require the `Gate` and `Analyze TypeScript` checks, require one approving review, dismiss stale approvals, require approval of the latest push, and require resolved conversations. CODEOWNERS review and a merge queue are intentionally not required for routine contributions at the current project stage.
4545

apps/server/src/lib/build-runtime-config.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ export async function buildRuntimeConfig(): Promise<LoadedProjectConfig> {
3333
// Every provider provisions a cloud sandbox environment; bailian additionally installs
3434
// `bailian-cli` in it. The vault is provider-conditional: bailian holds its DASHSCOPE_API_KEY;
3535
// other providers currently define no vault.
36+
//
37+
// Metadata stamps (`agents.base` / `agents.vault`) let the webui identify managed base
38+
// resources via remote listing (findBaseEnvironment / findBaseVault), complementing the
39+
// state-tracked identity the plan/apply engine provides.
3640
const vaults = vault
3741
? {
3842
[vault.name]: {
@@ -45,13 +49,15 @@ export async function buildRuntimeConfig(): Promise<LoadedProjectConfig> {
4549
secret_value: requireEnv(cred.secret_name),
4650
...(cred.networking ? { networking: cred.networking } : {}),
4751
})),
52+
metadata: { "agents.vault": "true" },
4853
},
4954
}
5055
: {};
5156
const environments = {
5257
[environment.name]: {
5358
...(environment.description ? { description: environment.description } : {}),
5459
config: environment.config,
60+
metadata: { "agents.base": "true" },
5561
},
5662
};
5763

apps/server/src/lib/state-scope.ts

Lines changed: 66 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,75 @@
1-
import { resolve } from "node:path";
1+
import { copyFileSync, existsSync, mkdirSync, renameSync } from "node:fs";
2+
import { homedir } from "node:os";
3+
import { dirname, join, resolve } from "node:path";
24
import { LocalFileStateBackend, type StateScope } from "@openagentpack/sdk";
35
import { RUNTIME_PROJECT_NAME } from "@/lib/build-runtime-config";
46

5-
// Historical on-disk state anchor. Previously derived via deriveStatePath() from the
6-
// demo file examples/bailian/bailian-cli/agents.yaml → agents.state.json. We pin the default
7-
// to that exact location so previously provisioned remote_ids (agents/vault/environment)
8-
// keep resolving and are not re-created. Override with AGENTS_STATE_PATH.
9-
const DEFAULT_STATE_PATH = "examples/bailian/bailian-cli/agents.state.json";
7+
/**
8+
* Default playground state lives in the user home directory, alongside the
9+
* provider config (~/.agents/config.json). This avoids colliding with CLI
10+
* example configs that happen to live in the repo's examples/ directory.
11+
*
12+
* Override with AGENTS_STATE_PATH for custom deployment layouts.
13+
*/
14+
const DEFAULT_STATE_PATH = join(homedir(), ".agents", "playground.state.json");
15+
16+
/**
17+
* Legacy state path. Before this migration the server stored its state inside
18+
* the repo's example directory. We migrate it once so previously-provisioned
19+
* remote_ids are preserved.
20+
*/
21+
const LEGACY_STATE_PATH = "examples/bailian/bailian-cli/agents.state.json";
22+
23+
let migrationChecked = false;
24+
25+
/**
26+
* One-time migration: if the legacy state file exists and the new default does
27+
* not, copy it over and log a warning. Idempotent — subsequent calls after a
28+
* successful migration (or when no migration is needed) are no-ops. A failed
29+
* copy allows retry on the next call so transient I/O errors don't permanently
30+
* disable migration for the process lifetime.
31+
*/
32+
function ensureMigrated(newPath: string, cwd: string): void {
33+
if (migrationChecked) return;
34+
35+
if (existsSync(newPath)) {
36+
migrationChecked = true;
37+
return;
38+
}
39+
const legacyPath = resolve(cwd, LEGACY_STATE_PATH);
40+
if (!existsSync(legacyPath)) {
41+
migrationChecked = true;
42+
return;
43+
}
44+
45+
try {
46+
mkdirSync(dirname(newPath), { recursive: true });
47+
copyFileSync(legacyPath, newPath);
48+
// Rename the legacy file so a version rollback won't silently read stale state.
49+
try {
50+
renameSync(legacyPath, `${legacyPath}.migrated`);
51+
} catch {
52+
// Non-fatal: the copy succeeded, state is in the new location.
53+
}
54+
migrationChecked = true;
55+
console.warn(
56+
`[state] Migrated playground state from legacy path:\n` +
57+
` ${legacyPath}\n` +
58+
` → ${newPath}\n` +
59+
` The legacy file has been renamed to ${legacyPath}.migrated.`,
60+
);
61+
} catch (error) {
62+
// Don't set migrationChecked — allow retry on next call.
63+
console.warn(`[state] Failed to migrate legacy state file: ${error instanceof Error ? error.message : error}`);
64+
}
65+
}
1066

1167
export function resolveStatePath(env: NodeJS.ProcessEnv = process.env, cwd: string = process.cwd()): string {
1268
const configured = env.AGENTS_STATE_PATH?.trim();
13-
return configured ? resolve(cwd, configured) : resolve(cwd, DEFAULT_STATE_PATH);
69+
if (configured) return resolve(cwd, configured);
70+
71+
ensureMigrated(DEFAULT_STATE_PATH, cwd);
72+
return DEFAULT_STATE_PATH;
1473
}
1574

1675
export function deriveWebUiStateScope(): StateScope {

apps/server/src/routes/vaults.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ vaultsRoute.openapi(createVaultRoute, async (c) => {
6868
const secretValue = key ?? (primarySecret ? requireEnv(primarySecret) : "");
6969
const vault = await withAgentRuntime(DEFAULT_AGENT_ID, (ctx) =>
7070
createCloudVault(ctx, name, {
71-
// display_name carries the base-vault identity (Agents/secrets) — a vault has no
72-
// separate `name` field, so findBaseVault nets it by display_name + the stamp.
71+
// display_name is used as the vault's human-readable label on the provider.
72+
// findBaseVault identifies the managed vault by its metadata stamp (agents.vault).
7373
display_name: name,
7474
metadata,
7575
credentials: structure.credentials.map((cred) => ({

apps/server/src/schemas/sessions.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export const SessionDeleteResponseSchema = z
3838
})
3939
.openapi("SessionDeleteResponse");
4040

41-
// Mode A REST request ergonomics (server-owned input, not the shared DTO).
41+
// REST request ergonomics (server-owned input, not the shared DTO).
4242
export const SessionsQuerySchema = z.object({
4343
// Non-numeric values resolve to `undefined` so the handler clamps to a
4444
// default instead of the request being rejected with a 400.
@@ -71,8 +71,7 @@ export const SessionParamsSchema = z.object({
7171
export const CreateSessionBodySchema = z.object({
7272
agentId: z.string(),
7373
prompt: z.string().min(1),
74-
// Required: a session must be pinned to a cloud environment (sandbox). Both transports
75-
// enforce this so Mode A (REST/OpenAPI) and Mode B (console) reject env-less creates.
74+
// Required: a session must be pinned to a cloud environment (sandbox).
7675
environmentId: z.string().min(1),
7776
// Optional: cloud vault ids to bind a user-supplied credential so the sandbox receives it.
7877
// Top-level binding shape matches the console createSession.

apps/server/src/schemas/vaults.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,8 @@ export const CloudVaultsResponseSchema = z.object({
1010
export const CreateVaultBodySchema = z.object({
1111
name: z.string().min(1),
1212
metadata: z.record(z.string(), z.string()).optional(),
13-
// The DASHSCOPE_API_KEY stored as the vault's credential secret value. Optional: Mode B
14-
// supplies the user's key; Mode A (local) omits it and the server injects it from its own
15-
// DASHSCOPE_API_KEY env. (Mode B never reaches this REST route — it goes via console RPC.)
13+
// The DASHSCOPE_API_KEY stored as the vault's credential secret value. Optional:
14+
// when omitted the server injects it from its own DASHSCOPE_API_KEY env.
1615
key: z.string().min(1).optional(),
1716
});
1817

apps/server/src/services/agents/catalog.ts

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
DEFAULT_PLAYBOOK_PROVIDER,
44
getDefaultPlaybook,
55
getPlaybook,
6+
getVaultProfile,
67
type PlaybookTemplate,
78
type ResolvedPlaybook,
89
resolvePlaybookModel,
@@ -80,9 +81,10 @@ export function compileAgentRuntime(
8081
const config = cloneConfig(baseConfig);
8182
const resolved = resolveSeedPlaybook(effectiveId, provider);
8283
const runtimeAgentId = resolved.agent.name;
83-
const built = buildAgentDecl(undefined, toAgentBuildInput(resolved, provider, modelOverride));
84-
// The agent declares neither environment nor vault: base resources are provisioned eagerly
85-
// and bound per-session via explicit environment_id + vault_ids.
84+
const built = buildAgentDecl(undefined, toAgentBuildInput(resolved, provider, baseConfig, modelOverride));
85+
// The agent declares environment and vault so syncAgentResources manages them
86+
// through the plan/apply engine — giving base resources state tracking, drift
87+
// detection, and the same content-hash identity as agent/skill resources.
8688

8789
config.agents = {
8890
...(config.agents ?? {}),
@@ -154,12 +156,45 @@ export function computeAgentConfigHash(config: ResolvedProjectConfig, agentId: s
154156
return createHash("sha256").update(stableStringify({ agentId, config })).digest("hex").slice(0, 16);
155157
}
156158

157-
function toAgentBuildInput(resolved: ResolvedPlaybook, provider: string, modelOverride?: string): AgentBuildInput {
159+
function toAgentBuildInput(
160+
resolved: ResolvedPlaybook,
161+
provider: string,
162+
baseConfig: ResolvedProjectConfig,
163+
modelOverride?: string,
164+
): AgentBuildInput {
158165
const model = resolvePlaybookModel(resolved, provider, modelOverride);
166+
// Resolve environment and vault names from the assembled config so the compiled
167+
// agent declares them. This lets syncAgentResources manage base resources
168+
// through the plan/apply engine, giving them state tracking and drift detection.
169+
// Invariant: buildRuntimeConfig produces exactly one environment (and at most one
170+
// vault for providers that need credentials). If this assumption breaks, the agent
171+
// would silently bind the wrong resource — fail fast instead.
172+
const envKeys = Object.keys(baseConfig.environments ?? {});
173+
if (envKeys.length !== 1) {
174+
throw new Error(
175+
`Expected exactly 1 environment in runtime config, got ${envKeys.length}. ` +
176+
`The agent compile path assumes a single base environment per provider.`,
177+
);
178+
}
179+
const environmentName = envKeys[0]!;
180+
const vaultProfile = getVaultProfile(provider);
181+
let vaultName: string | undefined;
182+
if (vaultProfile) {
183+
const vaultKeys = Object.keys(baseConfig.vaults ?? {});
184+
if (vaultKeys.length !== 1) {
185+
throw new Error(
186+
`Expected exactly 1 vault in runtime config for provider '${provider}', got ${vaultKeys.length}. ` +
187+
`The agent compile path assumes a single base vault per provider.`,
188+
);
189+
}
190+
vaultName = vaultKeys[0]!;
191+
}
159192
return {
160193
description: resolved.agent.description,
161194
model,
162195
instructions: resolved.agent.system,
196+
environment: environmentName,
197+
vault: vaultName,
163198
provider,
164199
builtinTools: resolved.agent.builtinTools,
165200
skills: resolved.agent.skills.map((skill) =>

apps/server/src/services/files/upload.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,7 @@ export async function listUserFiles(): Promise<ProviderFileInfo[]> {
4747

4848
/**
4949
* Resolve a short-lived presigned download URL for a file (e.g. an agent-delivered artifact).
50-
* Throws when the resolved provider has no download endpoint (bailian/claude); the webui's Mode A
51-
* transport surfaces that as an error, while Mode B never calls this route.
50+
* Throws when the resolved provider has no download endpoint (bailian/claude).
5251
*/
5352
export async function getUserFileDownloadUrl(id: string): Promise<{ url: string; expires_at?: string }> {
5453
return withAgentRuntime(DEFAULT_AGENT_ID, async (ctx, compiled) => {

0 commit comments

Comments
 (0)