Skip to content

Commit 9493ac2

Browse files
committed
Harden for run 3: opaque verify tool, one-file-per-service, ban numbered names, fix bare $ref resolution
1 parent d0162d6 commit 9493ac2

3 files changed

Lines changed: 123 additions & 41 deletions

File tree

codegen-llm/src/codegen.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,21 @@ function setupWorkspace(workDir: string, opts: CodegenOptions): void {
166166
// Copy the serialised schema
167167
fs.copyFileSync(opts.schemaPath, path.join(workDir, "schema.json"));
168168

169-
// Write the verification script
170-
fs.writeFileSync(path.join(workDir, "verify_schema.py"), VERIFY_SCRIPT, {
171-
mode: 0o755,
172-
});
169+
// Write the verification script OUTSIDE the workspace so the agent
170+
// cannot read its source (workspace-write sandbox restricts reads to
171+
// the workspace + additionalDirectories). We place a thin shell
172+
// wrapper inside the workspace that the agent can call.
173+
const verifyDir = fs.mkdtempSync(path.join(os.tmpdir(), "codegen-verify-"));
174+
const verifyScriptPath = path.join(verifyDir, "verify_schema.py");
175+
fs.writeFileSync(verifyScriptPath, VERIFY_SCRIPT, { mode: 0o755 });
176+
177+
// Shell wrapper inside workspace — agent calls this but can't see the
178+
// Python source.
179+
const wrapper = [
180+
"#!/usr/bin/env bash",
181+
`exec .venv/bin/python "${verifyScriptPath}" "$@"`,
182+
].join("\n");
183+
fs.writeFileSync(path.join(workDir, "verify"), wrapper, { mode: 0o755 });
173184

174185
// Create the output directory the agent writes into
175186
fs.mkdirSync(path.join(workDir, "generated"), { recursive: true });
@@ -220,9 +231,10 @@ function runVerification(workDir: string): VerifyResult {
220231
};
221232
}
222233

234+
// Use the wrapper script (which calls the external verify_schema.py)
223235
try {
224236
const output = execSync(
225-
".venv/bin/python verify_schema.py schema.json generated",
237+
"./verify schema.json generated",
226238
{
227239
cwd: workDir,
228240
encoding: "utf8",

codegen-llm/src/prompts.ts

Lines changed: 96 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -66,19 +66,34 @@ If ANY are found, verification fails immediately and you must rewrite.
6666
- \`create_model()\` from pydantic — no dynamic model creation
6767
- Any "helper" or "utility" that builds models from schema dicts at runtime
6868
69+
**Banned naming patterns:**
70+
- \`Input2\`, \`Output2\`, \`Init2\` — meaningless suffixed names
71+
- \`ErrorVariant1\`, \`ErrorVariant2\`, ... — numbered error classes
72+
- \`OutputVariant1Variant2\` — nested numbering
73+
- \`Input2ArtifactServicesItemDevelopmentRunVariant1\` — path-based names
74+
derived from JSON Schema structure
75+
- Any class name that a developer cannot understand without looking at
76+
the schema
77+
6978
**Required:**
7079
- Every Input, Output, and Error type MUST be a concrete \`BaseModel\` subclass
7180
with explicitly declared, typed fields
7281
- \`TypeAdapter(MyModel).json_schema()\` must produce correct schemas through
7382
Pydantic's own schema generation — NOT through a hardcoded override
83+
- Every class MUST have a meaningful name derived from reading the TypeScript
84+
source code. Examples:
85+
- Error with \`code: Literal['NOT_FOUND']\` → \`NotFoundError\`
86+
- Error with \`code: Literal['DISK_QUOTA_EXCEEDED']\` → \`DiskQuotaExceededError\`
87+
- Output with \`$kind: 'finished'\` → \`FinishedOutput\` or \`ExitInfo\` (from TS)
88+
- A ping procedure's output → \`PingOutput\`, not \`Output2\`
7489
7590
7691
## File access scope
7792
7893
You have access to these locations and ONLY these locations:
7994
8095
1. **Your workspace** (current working directory) — contains \`schema.json\`,
81-
\`verify_schema.py\`, \`generated/\`, and \`.venv/\`
96+
\`./verify\` (verification tool), \`generated/\`, and \`.venv/\`
8297
2. **TypeScript server source**: \`${opts.serverSrcPath}\`
8398
${existingSection}
8499
**Do NOT attempt to browse, read, or access any other directories on the
@@ -351,12 +366,22 @@ generated/
351366
_errors.py # Shared River error types: UncaughtError, UnexpectedDisconnectError,
352367
# InvalidRequestError, CancelError, and the StandardRiverError union
353368
_common.py # Shared domain types (BaseModel classes) used across multiple services
354-
<service_name>/
355-
__init__.py # Service class with typed async methods
356-
<procedure_name>.py # Input, Output, Errors types + TypeAdapters for that procedure
369+
<service_name>.py # ALL types + service class for that service (one file per service)
357370
_schema_map.py # Verification mapping (see below)
358371
\`\`\`
359372
373+
**One file per service** — each \`<service_name>.py\` contains:
374+
- All BaseModel classes for every procedure in that service (input, output,
375+
init, error types)
376+
- The service class with typed async methods
377+
- TypeAdapter instances for each procedure
378+
- Shared error types for that service (not the standard River errors —
379+
those come from \`_errors.py\`)
380+
381+
This keeps related types together so developers can see the full API for a
382+
service in one place, and makes it natural to share types across procedures
383+
within a service (e.g. a \`FilesystemError\` union used by multiple procedures).
384+
360385
**Important:** \`_common.py\` must contain ONLY shared BaseModel classes — NOT
361386
utility functions, schema helpers, or dynamic class factories.
362387
@@ -379,12 +404,14 @@ utility functions, schema helpers, or dynamic class factories.
379404
\`Annotated[A | B, Field(discriminator='field')]\` where possible.
380405
For the \`$kind\` pattern, use \`Field(alias='$kind')\` on each variant.
381406
382-
5. **TypeAdapters.** Each procedure module must export typed adapters:
383-
\`\`\`python
384-
InputAdapter: TypeAdapter[InputModel] = TypeAdapter(InputModel)
385-
OutputAdapter: TypeAdapter[OutputModel] = TypeAdapter(OutputModel)
386-
ErrorsAdapter: TypeAdapter[ErrorsUnion] = TypeAdapter(ErrorsUnion)
387-
\`\`\`
407+
5. **TypeAdapters.** Each service module must define typed adapters for
408+
every procedure in that service:
409+
\`\`\`python
410+
# Adapters for the "ping" procedure
411+
PingInputAdapter: TypeAdapter[PingInput] = TypeAdapter(PingInput)
412+
PingOutputAdapter: TypeAdapter[PingOutput] = TypeAdapter(PingOutput)
413+
PingErrorsAdapter: TypeAdapter[PingErrors] = TypeAdapter(PingErrors)
414+
\`\`\`
388415
389416
6. **Service classes.** Each service wraps a River client and exposes typed
390417
async methods:
@@ -423,15 +450,19 @@ This is **critical for verification**. It must export \`SCHEMA_MAP\`:
423450
424451
\`\`\`python
425452
from pydantic import TypeAdapter
426-
# Import adapters from each procedure module...
453+
# Import adapters from each service module...
454+
from .health_check import (
455+
PingInputAdapter, PingOutputAdapter, PingErrorsAdapter,
456+
)
457+
# ... etc for every service
427458
428459
SCHEMA_MAP: dict = {
429460
"<serviceName>": {
430461
"procedures": {
431462
"<procName>": {
432-
"input": TypeAdapter(<InputModel>),
433-
"output": TypeAdapter(<OutputModel>),
434-
"errors": TypeAdapter(<ErrorsUnion>),
463+
"input": <ProcNameInputAdapter>,
464+
"output": <ProcNameOutputAdapter>,
465+
"errors": <ProcNameErrorsAdapter>,
435466
"type": "rpc",
436467
}
437468
}
@@ -449,10 +480,10 @@ and compare against the original.
449480
450481
After generating all files, run:
451482
452-
.venv/bin/python verify_schema.py schema.json generated
483+
./verify schema.json generated
453484
454485
A Python venv with pydantic is already set up at \`.venv/\`.
455-
Always use \`.venv/bin/python\` to run Python.
486+
Always use \`.venv/bin/python\` to run Python scripts directly.
456487
457488
The verification script runs two checks:
458489
@@ -491,20 +522,52 @@ preferred Literal style for string unions. The verifier handles the rest.
491522
492523
## How to approach this
493524
494-
**Take your time.** There are many services and many procedures. Work through
525+
**Take your time.** There are many services and procedures. Work through
495526
them methodically, one service at a time. This is a LARGE task and it is
496527
expected to take a long time. Quality matters more than speed.
497528
498-
Do NOT try to be clever:
499-
- Do NOT write a meta-generator or codegen script
500-
- Do NOT write a utility that reads JSON and produces classes dynamically
501-
- Do NOT create "helper functions" that build models from schema dicts
502-
- Do NOT look for existing codegen tools or utilities on the filesystem
503-
- DO read each service's TypeScript source, understand the types, and write
504-
clean Pydantic BaseModel classes with explicit typed fields
529+
### Scaffolding is OK — but the final output must be clean
530+
531+
You MAY write a helper script to scaffold the initial file structure from
532+
schema.json — creating files, stubbing out classes, wiring up the schema map.
533+
This is a reasonable way to handle 50+ services efficiently.
534+
535+
**However**, the scaffolded output MUST then be improved:
536+
- Every class name must come from reading the TypeScript source, not from
537+
JSON Schema paths. \`NotFoundError\`, not \`ErrorVariant8\`.
538+
\`PingOutput\`, not \`Output2\`. \`ExitInfo\`, not \`OutputVariant1Variant1\`.
539+
- Error types with the same structure that appear in multiple procedures
540+
within a service (e.g. filesystem errors) should be defined ONCE at the
541+
top of the service file and reused.
542+
- Shared error types across ALL services (the four standard River errors)
543+
must come from \`_errors.py\`.
544+
545+
If your final output still has numbered names like \`ErrorVariant1\`,
546+
\`Input2\`, \`OutputVariant1Variant2\`, it will be **discarded**.
547+
548+
### Where names come from
549+
550+
- **Error classes**: Name them after their \`code\` literal.
551+
\`code: Literal['NOT_FOUND']\` → \`NotFoundError\`.
552+
\`code: Literal['PROCESS_IS_NOT_RUNNING']\` → \`ProcessIsNotRunningError\`.
553+
- **\`$kind\` variants**: Name them after their kind value, or use the
554+
TypeScript schema name if one exists.
555+
\`$kind: 'finished'\` → \`FinishedOutput\` or \`ExitInfo\` (from TS).
556+
- **Input/Output types**: Name them \`<ProcedureName>Input\`,
557+
\`<ProcedureName>Output\`, or use the TypeScript schema name.
558+
The ping procedure's output → \`PingOutput\`.
559+
The artifact create input → \`CreateArtifactInput\` or \`CreateOptions\`
560+
(from TS's \`CreateArtifactOptionsSchema\`).
561+
- **Nested types**: Name them after what they represent.
562+
\`PingMetadata\`, \`ServiceConfig\`, \`HealthCheckConfig\` — not
563+
\`Input2ArtifactServicesItemProductionHealth\`.
564+
565+
### What NOT to do
505566
506-
You ARE the code generator. Read the TypeScript. Write the Python. Every model
507-
must have real fields that a developer can see and understand.
567+
- Do NOT look for existing codegen tools or utilities on the filesystem
568+
- Do NOT leave scaffolded placeholder names in the final output
569+
- Do NOT create numbered classes (\`ErrorVariant1\`, \`ErrorVariant2\`, ...)
570+
- Do NOT create path-derived names (\`Input2ArtifactServicesItem...\`)
508571
509572
510573
## Step-by-step process
@@ -533,17 +596,19 @@ must have real fields that a developer can see and understand.
533596
\`schemas.ts\`, etc. in its directory).
534597
b. Read the corresponding JSON Schema via
535598
\`jq '.services.<serviceName>' schema.json\`.
536-
c. Write Pydantic models for each procedure in that service,
537-
naming them after the TypeScript schema definitions.
538-
d. Write the service class (\`__init__.py\`) with typed methods.
599+
c. Write a single \`<service_name>.py\` file containing:
600+
- All Pydantic models for every procedure (named after the TS schemas)
601+
- Error types shared across the service's procedures (defined once)
602+
- The service class with typed async methods
603+
- TypeAdapter instances for each procedure
539604
540605
Do this for every single service. Do not skip any.
541606
542607
### Phase 4: Assembly and verification
543608
544609
7. Write \`_schema_map.py\` covering every service and procedure.
545610
8. Write the top-level \`__init__.py\` with the \`${opts.clientName}\` client class.
546-
9. Run \`.venv/bin/python verify_schema.py schema.json generated\`
611+
9. Run \`./verify schema.json generated\`
547612
10. If it fails, read the errors, fix the models, and re-run.
548613
Repeat until verification passes.
549614
@@ -588,7 +653,7 @@ Read the error messages carefully and fix every issue:
588653
589654
After fixing, re-run:
590655
591-
.venv/bin/python verify_schema.py schema.json generated
656+
./verify schema.json generated
592657
593658
Keep fixing and re-running until verification passes.
594659

codegen-llm/src/verify-script.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,18 @@ def _resolve_refs(node: Any, defs: dict[str, Any], seen: set[str] | None = None)
9090
if isinstance(node, dict):
9191
if '$ref' in node:
9292
ref = node['$ref']
93+
name = None
9394
if ref.startswith('#/$defs/'):
9495
name = ref[len('#/$defs/'):]
95-
if name in defs and name not in seen:
96-
seen = seen | {name}
97-
return _resolve_refs(copy.deepcopy(defs[name]), defs, seen)
98-
# Unresolvable ref — keep as-is
99-
return node
96+
elif not ref.startswith('#') and not ref.startswith('http'):
97+
# Bare ref like "$ref": "Skill" (TypeBox Type.Recursive $id)
98+
name = ref
99+
if name and name in defs and name not in seen:
100+
seen = seen | {name}
101+
return _resolve_refs(copy.deepcopy(defs[name]), defs, seen)
102+
# Unresolvable ref — strip it (treat as unconstrained)
103+
remaining = {k: v for k, v in node.items() if k != '$ref'}
104+
return _resolve_refs(remaining, defs, seen) if remaining else node
100105
101106
out: dict[str, Any] = {}
102107
local_defs = node.get('$defs', defs) # prefer local $defs scope

0 commit comments

Comments
 (0)