Skip to content

β.1: amico-run thin process orchestrator (closes #1) - #7

Merged
jack-champagne merged 12 commits into
mainfrom
rchari/beta1-amico-run
Jun 18, 2026
Merged

β.1: amico-run thin process orchestrator (closes #1)#7
jack-champagne merged 12 commits into
mainfrom
rchari/beta1-amico-run

Conversation

@Rchari1

@Rchari1 Rchari1 commented Jun 10, 2026

Copy link
Copy Markdown
Member

β.1: amico-run thin process orchestrator

Closes #1.

Implements the D9 thin orchestrator per the spec spec-20260610-101245-amico-run-thin-orchestrator (plan: plan-20260610-103608-amico-run-thin-orchestrator).

What this is

  • Repo converted to a pnpm workspace; the spike extension moved verbatim to packages/extension/ (no behavior change).
  • New packages/amico-run/ (@amicode/amico-run): TypeScript library (Executor.submit(scriptPath, opts) → RunHandle) + CLI (amico-run <script.jl> [--executor local] [--lab <id-or-path>] [--runs-root] [--julia] [--project] [--sysimage]), bundled to a single zero-dependency dist/amico-run.js with a bash launcher that resolves node or exits 64.
  • Enforces the run-dir contract from outside Julia: manifest.toml written first (atomic), runs/index appended, latest symlink swung, julia spawned with cwd = run dir, stdout/stderr relayed (AMICODE_ITER → iter events) into run.log, FINISHED{status, exit_code} written last (atomic) keyed purely off the process result. The orchestrator's verdict overwrites any FINISHED a script fakes.
  • Runs root default: ~/.amico/runs/<lab-id>/ (survives reboot, per-lab partition; resolves Q124). No AMICO_* env vars — all argv (resolves Q90, satisfies S37). Lab pointer passed through verbatim, never parsed (D10); derived lab_id recorded in the manifest.
  • Provisional schema validators for manifest.toml/FINISHED/result.toml (superseded by Phase 0' SchemaPackage; snake_case keys are the frozen contract).
  • Deletes the pre-D9 bin/amico-run bash spike.

Acceptance criteria → evidence

Issue AC Proven by
Run dir conforms: manifest first, FINISHED terminal, result.toml atomic (S7) local_executor.test.ts (manifest-before-submit-resolves, schema-validated), failure_lanes.test.ts (manifest-before-spawn fixture, tight-loop no-partial-file reader), slow tier
rc≠0 ⇒ FINISHED{failed, rc}; clean ⇒ completed; abort ⇒ aborted (S7/Q4) failure_lanes.test.ts (9 matrix rows incl. spawn-failure-127, external-signal-143, bogus-FINISHED-overwrite), abort.test.ts (SIGTERM 143 / SIGKILL-after-grace 137 / idempotency)
No flag-parsing/SolveSpec/MCP/HTTP (S31) s31.test.ts grep rule, runs in CI
Exit codes 0 / 64 / 127 / 130 / julia-rc cli.test.ts — 6 end-to-end tests spawning the real bundle
Executor.submit → RunHandle consumed by the extension packages/extension/src/executor_check.ts type-level check, compiled in CI
Validated across solves/systems against schemas (issue challenge #1) Slow tier: X-gate and H-gate solves (Piccolo 0.11, Ipopt, 150 iters, fidelity > 0.9999) through the actual CLI; emitted run dirs validate against the provisional schemas end-to-end

Fast tier: 44 tests green (pnpm --filter @amicode/amico-run test), no Julia required. Slow tier: AMICO_TEST_JULIA_PROJECT=<piccolo env> pnpm --filter @amicode/amico-run test:slow.

Notes for the β.5 implementer

  • On-disk TOML keys are snake_case (run_id, exit_code, lab_id); the TS API is camelCase. This supersedes the architecture §3 sketch (exitCode, runId).
  • sizeClass (architecture §3/Q47) is intentionally absent from the manifest — underivable under D10; the Phase 1 Scheduler must obtain it elsewhere.
  • Watchers must key on FINISHED, never result.toml. Orchestrator hard-kill (SIGKILL/power loss) leaves manifest-without-FINISHED — Phase 1 stale-run detection (Q70) handles that; β.1 doesn't recover it.
  • The slow-tier solve_common.jl doubles as the seed for the β.3 AGENTS.md template (AMICODE_ITER via DirectTrajOpt's IpoptSolverExt.Callbacks, atomic result.toml, JLD2 export).

🤖 Generated with Claude Code

Rchari1 and others added 10 commits June 10, 2026 11:22
…ension

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ract enforcement

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ash spike (S31 grep green)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ontract end-to-end

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Rchari1
Rchari1 requested a review from jack-champagne June 10, 2026 15:55
@jack-champagne

jack-champagne commented Jun 10, 2026

Copy link
Copy Markdown
Member

Quick question before I review, why are we converting to pnpm workspace? Or maybe rather, what do you mean by that?

@Rchari1

Rchari1 commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

I understand it as packaging this best we should use a monorepo type space (it would be easier to package and give to customers), see claude's argument below:

  1. amico-run has two consumers that must never drift. The extension calls Executor.submit() in-process (it imports the library), while the LLM invokes amico-run as a CLI through opencode's bash tool. Those are two surfaces over what must be one implementation of the run-dir contract. The workspace lets packages/extension declare "@amicode/amico-run": "workspace:*" and import the exact same code and types the CLI bundles — the design docs flagged this exact failure mode as Q59 ("shared TS types across the process boundary... need a published/bundled package or they drift"). Without a package boundary, the sharing is copy-paste or a fragile relative import into another module's guts.

  2. The plan requires a workspace three weeks from now anyway. Phase 0's first task (0.1 SchemaPackage) is specified as "a shared workspace package consumed by extension + CLI + CI." That seam has to exist for the phase the team has already committed to — converting now, while the repo was one small package, cost about an hour. Converting later, with more code and more branches in flight, costs more.

  3. A real boundary keeps the orchestrator honest. β.1's whole identity is "thin, boring, contract-enforcing" — there's literally a CI grep rule (S31) policing what it's allowed to contain. As its own package, amico-run is typechecked, tested, and bundled in isolation; the extension physically can't reach into its internals, and the package can't quietly grow extension dependencies. Inside src/ that discipline would be convention-only.

The alternatives were considered explicitly in the design round: a second esbuild entry inside the extension's src/ (no restructuring, but boundary-by-convention and you still do the conversion for 0.1), and a standalone repo (maximum isolation, but it reintroduces the sibling-tree path resolution that made the spike hard-fail on clean machines, and splits CI). Workspace was the middle that pays for itself.

As for pnpm specifically — no new decision there: the repo was already pnpm (pnpm-lock.yaml predates us), so its workspace feature was the zero-friction option.

@jack-champagne jack-champagne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

β.1 looks good . Pulled it locally: build + typecheck (incl. the extension type-checking against @amicode/amico-run) green, fast tier 44/44, and a real-julia run through the bundle emitted a run dir that validates against all three provisional schemas. Contract enforcement (manifest-first, FINISHED-terminal, abort lanes, the §6 matrix) is exactly right, and the tests are behavioral rather than smoke.

One correction to confirm: the Piccolo 0.11 references (slow-tier solve_common.jl header + the PR body) look mistaken, should those be Piccolo 1.19, or are they actually Piccolissimo.jl version references?

Inline notes below are all non-blocking. The only one worth a decision is the updateLatest tmp-name race (latent until β.5 can submit concurrently), but maybe that doesn't matter while we are in phase 1 trying to roll this out to schuster? I'll leave that up to your discretion

Comment thread packages/amico-run/src/run_dir.ts Outdated
}

export function updateLatest(runsRoot: string, runId: string): void {
const tmp = join(runsRoot, '.latest.tmp')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fixed tmp name → races concurrent same-lab submits; scope to runId.

Comment thread packages/amico-run/src/run_dir.ts Outdated
}

export function appendIndex(runsRoot: string, runId: string, createdAt: string, scriptPath: string): void {
appendFileSync(join(runsRoot, 'index'), `${runId}\t${createdAt}\t${scriptPath}\n`)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unescaped tab/newline in script_path corrupts index.

Comment thread packages/amico-run/src/cli.ts Outdated
return f.exitCode === 0 ? 1 : f.exitCode
}

main(process.argv.slice(2)).then(c => { process.exitCode = c })

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No top-level .catch → unexpected throw exits 1, not 64.

type RunStatus, type SubmitOpts,
} from './types.js'

const ORCHESTRATOR_VERSION = '0.1.0'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hardcoded version duplicates package.json; will drift.

expect(validateFinished(readToml(join(runDir, 'FINISHED'))).ok).toBe(true)
const result = readToml(join(runDir, 'result.toml'))
expect(validateResult(result).ok).toBe(true)
expect(result.fidelity as number).toBeGreaterThan(0.99)

@jack-champagne jack-champagne Jun 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Asserts >0.99 but PR body claims >0.9999.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am actually fine with this, don't address right now, leaving for later


# Per-iter callback machinery lives in DirectTrajOpt's internal IpoptSolverExt
# module (not re-exported by Piccolo); reach it through the solve! method table.
const IpoptSolverExt = first(filter(

@jack-champagne jack-champagne Jun 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fragile reach-through; don't canonize in β.3 template especially if we are going to end up mainly supporting MadNLP, and Altissimo!

…xpected-throw→exit 64, updateLatest per-runId temp (concurrent-submit race), version single-sourced from package.json; solve_common.jl on Piccolo 1.19 via public Piccolo.Callbacks (no reach-through)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rchari1 added a commit that referenced this pull request Jun 17, 2026
…s (no reach-through); rollout+subspace fidelity (F=0.99994, ≤1)

Re-vetted against Piccolo 1.19 (what Pkg.add installs today, per Jack's #7 note).
Kills the methods(solve!) IpoptSolverExt reach-through — Callbacks is public in 1.19.
Fidelity now = unitary_fidelity over the computational subspace from a fresh
unitary_rollout (was reading >1 from raw-propagator integrator drift).

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

Rchari1 commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

Thanks Jack — pushed fixes (force-update on the rebased stack):

  • Piccolo version: confirmed — my dev env was stale at 0.11; the latest registered / what Pkg.add Piccolo installs is 1.19.0, and β.4 provisioning will pull 1.19 onto the lab machine. So 0.11 was just wrong, not Piccolissimo. I've re-vetted on 1.19 (see β.3+β.5: AGENTS.md + vetted template + chat→solve→inspector slice (closes #3) #9) and corrected the solve_common.jl header label (0.11 → 1.19).
  • solve_common.jl reach-through (your "don't canonize" note): removed. Piccolo.Callbacks is a public module in 1.19, so the methods(solve!) introspection is gone — both here and in the β.3 template. Re-vetted on 1.19 (toy qubit X, F=0.9998).
  • index TSV corruption (run_dir.ts:78): tab/newline in the script path now sanitized to a space before the index append (manifest.toml keeps the canonical TOML-escaped path); added a unit test.
  • cli no top-level .catch (cli.ts:75): added — any unexpected throw now maps to exit 64 (orchestrator fault), not a bare 1.
  • updateLatest race (run_dir.ts:82): temp name scoped to .latest.<runId>.tmp so concurrent same-lab submits don't clobber each other's in-flight temp.
  • version drift (local_executor.ts:17): ORCHESTRATOR_VERSION now imported from package.json (esbuild inlines it) — single source.

Fast tier 45/45. The struck-through >0.99/>0.9999 note I left as-is per your call.

@jack-champagne
jack-champagne merged commit 4415f02 into main Jun 18, 2026
1 check passed
@jack-champagne
jack-champagne deleted the rchari/beta1-amico-run branch June 18, 2026 06:29
@jack-champagne
jack-champagne restored the rchari/beta1-amico-run branch June 18, 2026 06:31
jack-champagne added a commit that referenced this pull request Jun 19, 2026
#7 was squash-merged (4415f02), flattening beta1's history; beta2 already
contains beta1's changes via its original commits, so this records main as an
ancestor without altering beta2's tree (-s ours). Lets #8 reach main with a
merge commit — no history rewrite, branch stays fast-forwardable.
@kateebonner kateebonner mentioned this pull request Jul 9, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

β.1 — amico-run thin process orchestrator

2 participants