mirror of
https://github.com/mvschwarz/openrig-studio.git
synced 2026-11-07 05:56:21 -10:00
Advertise focus and drive, so feature detection stops lying
An independent cold build of v0.7.0 — fresh clone, documentation only, no contact with me — found three things. The first is the one that matters and it is the worst class this repository deals in. FOCUS AND DRIVE ANSWERED WITHOUT BEING ADVERTISED. Both verbs worked, both were documented, and neither namespace appeared in /api/contract's capabilities. But contract-meta.md tells every consumer that each namespace it uses must appear there and to CHECK BEFORE USE — so an agent doing exactly what the contract instructs concluded the features were absent. A verb that answers without advertising is worse than a missing verb: the documented path leads to the wrong answer, and the consumer has no reason to doubt it. This was mine, and it was pattern-matching rather than reasoning. When drive shipped I checked whether focus had declared a capability, found none, and made drive match it for consistency. Consistency with an omission. Read and write are separate entries because they are separately true, which is what files.read has always meant, and it lets a runtime offer read-only focus honestly instead of claiming a namespace it half-implements. TWO GUARDS HAD TO BE LOOSENED, AND THAT IS THE INTERESTING PART. Both asserted the capability set was EXACTLY equal to a baseline. True when written, wrong as a rule: capabilities is the feature-detection list, so it must grow when a namespace ships or it can never advertise anything. Equality there would have blocked this very fix — a guard that reads as strictness and is actually a rule that no feature may ever be declared. Both now assert the additive property that was meant: capabilities MAY GAIN, NEVER LOSE, since a consumer gates on them. Verified by planting a removal and watching both fail. One of those loops was written backwards first — iterating the current runtime and requiring the baseline to contain each, which reads identically and asserts the opposite. Caught because the suite failed, not because it looked wrong. A guard now compares the documented capability list against the served one, and a second asserts every namespace with a served route is advertised. Neither existed: measured first, the parity suite passed at 4/4 with a capability deleted from the documented example, because the comparator descends objects and capabilities is an array. Capabilities are shape, not data — the rest of the arrays in that response are counts and errors and differ by design — so they get a named check rather than a general array descent. Also: POST /api/focus's response shape is now documented rather than inferred from a readback, and the generated project's run snippet no longer omits --surfaces, without which the new surface never reaches the rail and looks like a broken row. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
1 parent
534e08ef85
commit
9b322f6ae2
10 files changed
+133
-9
No files matched your search
@@ -73,13 +73,26 @@ watch a live-updating surface work.
|
||||
|
||||
## Status
|
||||
|
||||
Package 0.7.0. Contract v0.1 — unchanged, because everything below is additive
|
||||
Package 0.7.1. Contract v0.1 — unchanged, because everything below is additive
|
||||
or a behaviour fix, and the contract version only moves on a breaking change.
|
||||
Pre-release.
|
||||
|
||||
**All three agent primitives are now in the SDK.** An agent can see what changed,
|
||||
see what the user is looking at, and operate the surface they have open.
|
||||
|
||||
### What is new in 0.7.1
|
||||
|
||||
**`GET /api/contract` now advertises focus and drive.** Both verbs worked and
|
||||
neither appeared in `capabilities` — so an agent doing feature detection the way
|
||||
`contract-meta.md` instructs concluded the features were absent. A verb that
|
||||
answers without advertising is worse than a missing one: the consumer follows the
|
||||
documented path and is steered to the wrong answer. Found by an independent cold
|
||||
build, which is the only way this class gets found.
|
||||
|
||||
`POST /api/focus`'s response shape is now documented, and the generated project's
|
||||
final run snippet no longer omits `--surfaces` — without it the new surface never
|
||||
reaches the rail, which looks like a broken row rather than a missing flag.
|
||||
|
||||
### What is new in 0.7.0
|
||||
|
||||
Someone built a small app against 0.6.0 from the documentation alone, and fixed
|
||||
|
||||
+13
-1
@@ -67,7 +67,19 @@ const CONTRACT_VERSION = "0.1";
|
||||
// immediately reload itself, and a restart that straddles a consumer's startup is
|
||||
// still seen.
|
||||
const BOOT_ID = `${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 10)}`;
|
||||
const CAPABILITIES = ["contract.meta", "observe.factory-state", "stream.events", "files.read", "shell.protocol"];
|
||||
// FEATURE DETECTION IS THE POINT. contract-meta.md tells a consumer that every
|
||||
// namespace it uses must appear here and to CHECK BEFORE USE — so a verb this
|
||||
// runtime answers but does not advertise is worse than a missing verb: the
|
||||
// consumer does exactly what the contract instructs and concludes the feature is
|
||||
// absent. focus and drive shipped answering, undeclared, and an independent cold
|
||||
// build found it by following the documented detection path.
|
||||
//
|
||||
// Read and write are separate entries because they are separately true. That is
|
||||
// what "files.read" has always meant — the files namespace advertises read
|
||||
// because there are no write verbs — and it lets a runtime offer read-only focus
|
||||
// honestly rather than claiming a namespace it half-implements.
|
||||
const CAPABILITIES = ["contract.meta", "observe.factory-state", "stream.events", "files.read",
|
||||
"shell.protocol", "focus.read", "focus.write", "drive.read", "drive.write"];
|
||||
|
||||
const TYPES = { ".html": "text/html; charset=utf-8", ".json": "application/json", ".css": "text/css",
|
||||
".js": "text/javascript", ".mjs": "text/javascript", ".svg": "image/svg+xml", ".png": "image/png",
|
||||
|
||||
@@ -51,7 +51,8 @@ Two schemas sit beside them: `surface-row.schema.json` for a manifest row, and
|
||||
{
|
||||
"contractVersion": "0.1",
|
||||
"runtime": { "name": "openrig-studio", "flavor": "reference-fixture", "boot": "<opaque id, stable for this process>" },
|
||||
"capabilities": ["contract.meta", "observe.factory-state", "stream.events", "files.read", "shell.protocol"],
|
||||
"capabilities": ["contract.meta", "observe.factory-state", "stream.events", "files.read",
|
||||
"shell.protocol", "focus.read", "focus.write", "drive.read", "drive.write"],
|
||||
"manifest": {
|
||||
"ok": true, "errors": [], "warnings": [], "surfaces": 1,
|
||||
"seats": { "count": 1, "source": "package", "attachable": false },
|
||||
|
||||
@@ -145,6 +145,7 @@ feels driven and one that feels haunted.
|
||||
| specified | shipped |
|
||||
|---|---|
|
||||
| `POST` / `GET /api/drive`, opaque ops, server-assigned `gen` | **yes**, in the reference runtime |
|
||||
| the namespace advertised at `GET /api/contract` -> `capabilities` (`drive.read`, `drive.write`) | **yes** |
|
||||
| the change-signal `?since=` shape on the read | **yes** |
|
||||
| `driveSurface` dropping superseded ops | **yes**, with a control that plants a queue and fails |
|
||||
| not replaying ops that predate the surface, and `resumeLatestOnLoad` opting back in | **yes**, with controls on both directions — including that a post-load op still applies, so the baseline cannot be satisfied by a surface that stopped listening |
|
||||
|
||||
@@ -87,6 +87,19 @@ and the agent is told nothing at all.
|
||||
|
||||
**`POST /api/focus` — a write updates the fields it names and leaves the rest.**
|
||||
|
||||
The body is a JSON object naming the fields to change. A write answers with the
|
||||
record as it now stands, and with the marker that write produced:
|
||||
|
||||
```json
|
||||
{ "ok": true, "marker": "<opaque>", "focus": { "...": "the record after the write" } }
|
||||
```
|
||||
|
||||
**Returning the record means a writer does not need a follow-up read** to know
|
||||
what it produced, and the marker it gets back is the one a subsequent `?since=`
|
||||
compares against. A write that names no known field is refused with
|
||||
`{ "ok": false, "error": "<what> — <what to do>" }` and HTTP 400 rather than
|
||||
silently doing nothing.
|
||||
|
||||
That is the whole rule, and it exists because whole-record replacement is a
|
||||
measured defect: a second verb overwrote the record with `view` blanked and asset
|
||||
paths reduced to basenames, so **pinning something destroyed the view context the
|
||||
@@ -161,6 +174,8 @@ nothing. A surface adopting this reads the address before its first render.
|
||||
| `GET /api/focus` answering `{changed, marker, focus}` under `?since=` | **yes**, in the reference runtime |
|
||||
| field-scoped writes that cannot blank each other | **yes**, with controls |
|
||||
| `at` server-set on every write | **yes** |
|
||||
| `POST` answering `{ok, marker, focus}` so a writer needs no follow-up read | **yes** |
|
||||
| the namespace advertised at `GET /api/contract` -> `capabilities` (`focus.read`, `focus.write`) | **yes** — and it was NOT, until an independent cold build followed the documented feature-detection path and concluded the verb was absent |
|
||||
| `by` overridden from a real caller identity | **no, and it cannot be here.** The reference runtime is single-process and loopback and has no identity for its caller, so `by` is what the caller declared. A runtime that HAS one must override it |
|
||||
| a provider declaring the verb in `signals.verbs` | **specified**; the reference runtime is single-process and declares nothing |
|
||||
| `readAddress` / `writeAddress`, hash-owned and query-untouched | **yes**, with controls |
|
||||
|
||||
@@ -60,12 +60,16 @@ manifest live — check `GET /api/contract` to see whether your row was accepted
|
||||
|
||||
## Run it
|
||||
|
||||
Start the studio runtime (from the studio directory):
|
||||
Start the studio runtime, pointed at this directory:
|
||||
|
||||
```sh
|
||||
node app/serve-studio.mjs
|
||||
node <studio>/app/serve-studio.mjs --surfaces .
|
||||
```
|
||||
|
||||
**`--surfaces` is not optional.** Without it the runtime serves only its own
|
||||
surfaces and `{{LABEL}}` never appears on the rail — which looks like a broken
|
||||
row rather than a missing flag, and is the wrong thing to go debugging.
|
||||
|
||||
Open <http://127.0.0.1:8890/> and pick **{{LABEL}}** from the rail.
|
||||
|
||||
If your row does not appear, the reason is spelled out in
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@openrig/studio",
|
||||
"version": "0.7.0",
|
||||
"version": "0.7.1",
|
||||
"description": "SDK for building agent-driven mini-apps: thin surfaces paired with agents, running in a shared shell.",
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
|
||||
@@ -140,6 +140,62 @@ test("overlay-configured: the documented consumer example agrees with the served
|
||||
);
|
||||
});
|
||||
|
||||
test("the documented capabilities are exactly the ones served", async (t) => {
|
||||
// CAPABILITIES ARE SHAPE, NOT DATA, WHICH IS WHY THEY GET A NAMED CHECK HERE
|
||||
// RATHER THAN A GENERAL ARRAY DESCENT. The comparator above deliberately does
|
||||
// not compare array contents — `errors`, `warnings` and `queue` are data and
|
||||
// differ by design. `capabilities` is the opposite: it is a closed contract
|
||||
// enumeration, and contract-meta.md instructs every consumer to check it before
|
||||
// using a namespace.
|
||||
//
|
||||
// So drift here is uniquely damaging: a runtime that answers a verb it does not
|
||||
// advertise sends a consumer doing exactly what the contract says down the path
|
||||
// of concluding the feature is absent. That shipped — focus and drive both
|
||||
// answered while undeclared, and an independent cold build found it by following
|
||||
// the documented detection path rather than by reading the source.
|
||||
//
|
||||
// Measured before this was written: dropping a capability from the documented
|
||||
// example left the parity tests above at pass 4 / fail 0, so nothing could see it.
|
||||
const s = await start(false);
|
||||
t.after(() => s.stop());
|
||||
|
||||
const documented = example("`GET /api/contract` returns:").capabilities;
|
||||
assert.ok(Array.isArray(documented) && documented.length,
|
||||
"positive control: contract-meta.md's example carries no capabilities to compare");
|
||||
|
||||
assert.deepEqual([...s.response.capabilities].sort(), [...documented].sort(),
|
||||
"the capabilities the runtime serves and the ones contract-meta.md documents have diverged — " +
|
||||
"feature detection follows the DOCUMENT, so this gap is what a consumer acts on");
|
||||
});
|
||||
|
||||
test("every namespace with a served verb is advertised", async (t) => {
|
||||
// The other direction, and the one that actually bit. The check above keeps the
|
||||
// doc and the runtime in step with each other; both could still agree while
|
||||
// omitting a verb the runtime answers.
|
||||
//
|
||||
// Keyed on the ROUTES THE RUNTIME ACTUALLY HANDLES, read out of its source, so a
|
||||
// verb added without a capability fails here rather than being found by whoever
|
||||
// tries to use it.
|
||||
const s = await start(false);
|
||||
t.after(() => s.stop());
|
||||
|
||||
const src = fs.readFileSync(path.join(REPO, "app", "serve-studio.mjs"), "utf8");
|
||||
const routes = [...src.matchAll(/u\.pathname === "(\/api\/[a-z/-]+)"/g)].map((m) => m[1]);
|
||||
assert.ok(routes.length >= 4, `positive control: parsed ${routes.length} routes from the runtime`);
|
||||
|
||||
// namespace -> the capability prefix that advertises it
|
||||
const NAMESPACE = { "/api/contract": "contract", "/api/factory/state": "observe",
|
||||
"/api/events": "stream", "/api/focus": "focus", "/api/drive": "drive" };
|
||||
const caps = s.response.capabilities;
|
||||
const unadvertised = [...new Set(routes)]
|
||||
.filter((r) => NAMESPACE[r])
|
||||
.filter((r) => !caps.some((c) => c === NAMESPACE[r] || c.startsWith(NAMESPACE[r] + ".")));
|
||||
|
||||
assert.deepEqual(unadvertised, [],
|
||||
`the runtime answers these verbs but advertises no capability for them, so a consumer ` +
|
||||
`following contract-meta.md's "check before use" concludes they are absent: ${unadvertised.join(", ")}`);
|
||||
});
|
||||
|
||||
test("positive control: this suite is capable of failing", () => {
|
||||
assert.throws(() => assert.equal(1, 2));
|
||||
});
|
||||
@@ -401,8 +401,19 @@ test("contract v0.1 is unchanged: same version, same existing fields, additive o
|
||||
const body = await r.json();
|
||||
|
||||
assert.equal(body.contractVersion, "0.1", "adding fields must not move the contract version");
|
||||
assert.deepEqual(body.capabilities,
|
||||
["contract.meta", "observe.factory-state", "stream.events", "files.read", "shell.protocol"]);
|
||||
// CAPABILITIES MAY GAIN, NEVER LOSE. This asserted exact equality, which was
|
||||
// true when written and wrong as a rule: `capabilities` is the feature-detection
|
||||
// list, so it MUST grow when a namespace ships or it can never advertise
|
||||
// anything. Equality here would have blocked declaring focus and drive — the
|
||||
// very omission an independent cold build reported.
|
||||
//
|
||||
// Removing one IS breaking, because a consumer gates on it, so that is what is
|
||||
// still checked.
|
||||
for (const cap of ["contract.meta", "observe.factory-state", "stream.events", "files.read",
|
||||
"shell.protocol"]) {
|
||||
assert.ok(body.capabilities.includes(cap),
|
||||
`capability ${cap} disappeared — removing one breaks every consumer gating on it`);
|
||||
}
|
||||
for (const key of ["ok", "errors", "warnings", "surfaces"]) {
|
||||
assert.ok(key in body.manifest, `pre-existing manifest field ${key} disappeared`);
|
||||
}
|
||||
|
||||
@@ -141,7 +141,18 @@ test("zero-config: the contract answer is additive — one field gained, none lo
|
||||
"the /api/contract response gained or lost a TOP-LEVEL field");
|
||||
|
||||
assert.equal(a.contractVersion, b.contractVersion, "contractVersion moved for an additive change");
|
||||
assert.deepEqual(a.capabilities, b.capabilities, "the capability set changed");
|
||||
// GAINING a capability is additive and is the whole point of the field; LOSING
|
||||
// one breaks every consumer that gates on it. This compared them for equality,
|
||||
// which reads as strictness and is actually a rule that no feature may ever be
|
||||
// advertised — it would have blocked declaring focus and drive.
|
||||
// Direction matters and is easy to get backwards: iterate the BASELINE (`b`) and
|
||||
// require the CURRENT runtime (`a`) to still carry each. The reverse reads
|
||||
// identically and asserts that no capability may ever be ADDED, which is the
|
||||
// rule this loop exists to remove.
|
||||
for (const cap of b.capabilities) {
|
||||
assert.ok(a.capabilities.includes(cap),
|
||||
`zero-config LOST capability ${cap} — consumers gate on these, so removal is breaking`);
|
||||
}
|
||||
assert.deepEqual(Object.keys(a.manifest).sort(), Object.keys(b.manifest).sort(),
|
||||
"the manifest report gained or lost a field");
|
||||
|
||||
|
||||
Reference in new issue
Block a user