fix(server): compile caller fragments under restricted mode at append scope

[sagarswamirao]

The reported finding

From a multi-tenant security review, rated HIGH:

/compile and malloy_compile compile caller source unrestricted. The /query
path uses loadRestrictedQuery, which rejects .table(), .sql(), import
and given:. The compile path does not. Malloy resolves SQL/table schemas at
compile time by running a DESCRIBE against the connection, so compiling
run: duckdb.sql("SELECT * FROM read_csv('/etc/secret')") executes against the
sandbox: a file-existence / column-name oracle, plus SSRF via
read_csv('https://...'). It returns no row data, so this is disclosure and
SSRF rather than bulk exfiltration.

Reproduced

Compiled hostile text through the real Environment.compileSource at append
scope against a real DuckDB package. Three signals:

The file is read. run: duckdb.sql("SELECT * FROM read_csv('<tmp>/secret.csv')") -> { group_by: ssn }
returns problems: [] -- a clean compile, so the header resolved.

A file-existence oracle. The same text against a path that does not exist:

IO Error: No files found that match the pattern ".../nope.csv"
LINE 1: DESCRIBE SELECT * FROM (SELECT * FROM read_csv('...

The DESCRIBE is quoted back verbatim, which is also the proof that compile-time
schema resolution executes.

A schema oracle. ssn resolves against the CSV's real header; no_such_column
returns 'no_such_column' is not defined.

read_csv accepts https://, so SSRF rides the identical path.

A second vector the review did not name

URL_READER does a bare fs.readFile with no containment, so append-scope
import "../../outside.malloy" compiled cleanly: it read, compiled and made
queryable a Malloy file outside the package tree, including whatever connection
that file declares. import at append scope is an independent arbitrary-file-read
vector, and a DuckDB sandbox flag cannot close it. It has its own test.

What this changes

assertNoRestrictedConstructs() in a new compile_restriction.ts, invoked for
scope === "append" only, before the compile.

It runs a separate restricted compile and acts only on
code: 'restricted-construct-forbidden'; every other diagnostic is left to the
real compile, so positions and problem sets for legitimate text are unchanged. A
non-MalloyError throw is rethrown rather than treated as a pass, because an
infrastructure failure must not open the gate. The base model is loaded without
the caller's text, so a caller cannot widen the namespace it is judged against.

An earlier revision fell back to no base model when the named one could not
be loaded. That was a fail-open, not a safe default: name!type(...) and the
sql_* family are classified inside computeExpression(fs) and need a resolved
FieldSpace, so with no base model they are never classified and the gate returns
clean on text it exists to refuse. The gate now requires a Model rather than
accepting Model | undefined, which makes that a type error instead of a
convention the next caller can break.

Refusals raise CompileRefusedError, the existing house type, which maps to 400.

Failing to load the base model is three different answers

Requiring a base model means deciding what happens when it cannot be loaded, and
refusing uniformly would have been wrong. getModel() throws MalloyError on
any base-model problem, including failed-to-fetch-table-schema -- which is what
a warehouse being unreachable looks like. A 4xx tells a client it sent something
bad and should not retry, so an outage would have been reported as the caller's
fault, with a deliberately detail-free body that could not tell them otherwise.

Failure	Answer	Why
Schema could not be fetched	503	A dependency is unreachable. The package scope already maps its analogous failure this way.
Base model does not compile	400, with the problems	The author's to fix, and the diagnostics describe a model the caller already has, so they disclose nothing new.
Missing file, permission, anything else	400, detail-free	The caller named a model this server cannot load.

The detail stays server-side in every case: the model path is absolute inside the
container, so returning it would answer "does this file exist, and is it
readable" for any path a caller names -- the same shape of oracle this gate
exists to close.

Scope restricted, and scopes deliberately left alone

append is restricted. Its text is a fragment checked against a curated
model, inherits that model's namespace, and has no legitimate reason to declare
its own data roots.

file and package are untouched. There the source IS the model file, and
import / connection.table(...) / connection.sql(...) are how any model
declares what it reads. Restricting them would make an ordinary package
un-authorable -- the repo's own examples define sources with duckdb.sql(...) at
file root -- and would break the dashboard write path, which compiles at file
and would start refusing every dashboard containing a source definition.

Two skills instructed the refused pattern

The skills are what an agent loads, so they decide whether the workflow works,
and both pointed at the scope this now refuses.

malloy-dashboards told an agent to compile a not-yet-saved dashboard at the
default scope. A dashboard file opens with an import, so the documented happy
path failed twice: on the import, and on the model path that does not exist yet.
It now points at file scope for the new-file case, with the reason.

malloy-modeling presented append as the way to add a new definition, while
search_database_schema hands the agent a source: x is conn.table(...) line to
start from -- which append refuses. It now says so and names the scopes that
accept it.

AGENTS.md and docs/ai-agents.md were already updated for the refusal; this
carries the same correction into the two copies agents actually read.

One thing worth knowing for any future change here

runtime.loadModel(url, { restrictedMode: true }) silently drops the flag.
It destructures only refreshSchemaCache/noThrowOnError and forwards a fixed
field list; extendModel drops it too. Verified empirically: the flag has no
effect there and the hostile text compiles fine. Only loadRestrictedQuery and a
direct Malloy.compile honour it. So the obvious implementation of this fix
looks applied and does nothing.

loadRestrictedQuery itself was not usable here: it compiles a query, while the
documented append fragment forms (source: check is X extend {...},
query: check is ...) are statements with no runnable query, so routing append
through it would break the primary authoring workflow. Hence
Malloy.compile({ restrictedMode: true, model, method: "extendModel" }).

The restriction is the compiler's own classification rather than a pattern match,
for the reason authorize.ts already states about gates: a rejecter that matches
text can be walked past in a spelling it never heard of.

Testing

compile_restriction.spec.ts, 16 cases: the oracle forms above, the import
traversal, legitimate append text that must still compile, and the unrestricted
scopes.

Verified load-bearing by reverting the gate: 8 fail / 7 pass. The 7 that still
pass are exactly the legitimate-authoring and unrestricted-scope cases, which
should pass either way. Restored: 16/16.

One real regression surfaced and is fixed: compile_scopes.spec.ts submitted a
model opening with import at append scope as a control for an unrelated
"Cannot redefine" collision. Since append-scope import is precisely the
traversal vector proved above, the control drops its import line rather than the
gate making an exception -- the test's actual subject is unchanged and still
asserted.

Gates on the merged tree: build, typecheck and lint all exit 0;
test:server is 3934 pass, 3 skip, 0 fail across 179 files.

Known gaps, not addressed here

Raised in review and worth naming rather than leaving implied:

• The suite exercises 4 of the 7 restricted rules. sql_*, given: and
  ##! are untested. sql_* matters most, because it shares the
  FieldSpace-dependent classification path with the construct that motivated
  requiring a resolved base model.
• api-doc.yaml is not in this diff. The scope description still reads as
  though an invalid enum value is the only 400 it can produce, and a previously
  200 request class now returns 400. External clients generate from that spec.
• Neither refusal path increments a metric. With the 503 split above a
  dependency outage is at least no longer indistinguishable from bad input by
  status code, but a counter with a reason label would separate
  restricted_construct from base_model_load_failed in the data.
• The gate adds a second parse and translate inside the per-package mutex.
  Schema round trips are not doubled, since the schema cache lives on the shared
  connection, so this is CPU and lock-hold rather than backend load. Loading the
  base model once and reusing it for both compiles would recover most of it.
• A missing model path now returns 400 where a present one returns 200,
  which distinguishes present from absent within the package tree (bounded by
  assertSafeRelativeModelPath). Much narrower than the oracle being closed, but
  new, and not pinned by a test.

[housejester]

Read the whole change plus the six-comment thread above. The fix is well shaped: using the compiler's own restricted-construct-forbidden classification rather than a byte match, running the gate as a separate compile so ordinary diagnostics keep their positions and wording, rethrowing a non-MalloyError instead of treating it as a pass, typing model: Model rather than Model | undefined so the FieldSpace hole can't silently come back, and the revert check (8 fail / 7 pass, and the 7 are exactly the ones that should pass either way). The SCOPE, STATED HONESTLY header is the right landing for the containment question.

Everything below is about the edges of the new namespace, and about workflows the gate refuses that this repo's own skills still instruct agents to perform. I had no Node/Bun runtime available, so nothing here was executed — the two namespace items are confirmation requests with the test that settles each, not claims.

What this changes about append

append stops being "your text concatenated into the model file and compiled once" and becomes "your fragment, type-checked under restricted mode against the model's compiled published surface, then concatenated and compiled." Two namespaces where there was one, plus a precondition:

	before	after
namespace judging the caller's text	the model file's text	gate: the model's compiled Model; real compile: still the file text
legal construct set	anything Malloy accepts	minus the seven restricted constructs
modelPath that does not exist	200, empty namespace	400
modelPath that exists but does not compile	200 + the model's own diagnostics	400, reason withheld

The load-bearing invariant is no longer "a base model exists" — it is namespace parity: the gate's namespace must be a superset of what the real compile resolves.

---

Worth acting on

[HIGH — fail-open, same shape as the one fixed in 1af0b37d] The gate judges the fragment against baseModel via method: "extendModel"; the real compile judges it against modelContent + "\n" + source. Wherever the gate's namespace or compiler context is narrower, an enclosing reference fails to resolve, ExprFunc.getExpression(fs) is never reached, name!type(...) / sql_* are never classified, restrictedRejections() comes back empty, and the unrestricted compile runs the construct. Two narrowings I can point at from this repo:

• export { … }. docs/discovery-and-access.md:37 — "A model with no export { … } exports all of its locally-declared top-level sources; declaring export { customers } … keeps imported/internal helpers out." If extendModel exposes exports rather than full ModelDef.contents, then run: internal_helper -> { group_by: v is read_csv!string('/etc/passwd') } is unresolvable in the gate and resolvable in the concatenated file — your exact payload, reached through a non-exported name instead of an absent model.
• ##! compiler flags. The gate's synthetic internal:// document has no ##! line; the concatenated file does. Five bundled example models open with one (examples/storefront/givens.malloy:29, examples/governed-analytics/orders.malloy:11), so a package enabling an experiment is the normal case here rather than a corner. If flags don't carry into extendModel, a package with ##! experimental.sql_functions gets sql_number(...) rejected inside the gate as experiment-not-enabled — no restricted code, gate passes — and accepted by the real compile. That is the same backstop you identified as carrying the sql_* half, being relied on from the other side.

Could you confirm whether Malloy.compile({ model, method: "extendModel" }) carries the full ModelDef.contents and the source model's compiler flags? Either way it is worth pinning rather than reasoning about, since it is a malloy-side property this gate now depends on: two cases in compile_restriction.spec.ts — a base model with export { base_source } plus a second non-exported top-level source, and a base model opening with ##! experimental.sql_functions — each carrying a read_csv!string(...) / sql_number(...) payload asserted refused.

[MED-HIGH — two documented workflows now return 400, and the skills still teach them] skills/ is symlinked into .claude/skills/, so these are what an agent in this repo reads:

• skills/malloy-dashboards/SKILL.md:56-65 tells an agent to compile a new dashboard "at the path the file will have," and says outright: "(The third scope, append, is the default and is what a not-yet-saved file gets.)" That request now fails three independent ways — the path does not exist yet, step 4's first line is import { order_items, products } from '../storefront.malloy' (:44, :83, :133), and step 3's filter declarations are given:. All three compiled before; the test "refuses an import that would borrow another model's surface" is itself the proof the import leg used to work. The dashboard write path is correctly protected (dashboard.controller.ts:198 compiles at "file") — it is the documented pre-save check that breaks.
• skills/malloy-modeling/SKILL.md:152 — "Adding a new definition or query: the default (scope: "append")". A new source rooted in a table is the most common new definition in the modeling loop and is now refused. The escape hatch is right, but it currently exists only in the AGENTS.md / docs/ai-agents.md lines this PR adds.

Recommend updating both in this PR: dashboards step 5 to send a not-yet-saved file to "file", and malloy-modeling step 1 to carve out "a new definition that declares its own data root or import". compile_scopes.spec.ts having to drop its import line is the same signal arriving from inside the suite.

[MED — the debug loop loses its diagnostics, and the 400 says nothing] Separate from the missing-path case you weighed in the 19:41 comment and settled via option 3: environment.ts:973-996 also collapses "the file exists and does not compile" into that same reason-free refusal. That is the case append is most useful for — a package whose model is broken (get_status stale: true, the malloy-debug loop) previously got the model's compile errors back as ordinary diagnostics and now gets could not be loaded to check it against, with the cause server-side only. The withholding in f1e857be is right for a caller-named path, but a path that resolves to a real package file discloses nothing new — scope: "package" with no source already returns every file's diagnostics, including files hidden from discovery. Recommend splitting the two: file absent ⇒ refuse opaquely as now; file present but getModel() threw ⇒ return the base model's problems as diagnostics, which is what append did before. The diskTarget?.isFile() check at environment.ts:700 is the existing pattern for the distinction.

Smaller notes

[MED — the new docs under-enumerate what is refused] AGENTS.md:207, docs/ai-agents.md:75 and COMPILE_DESCRIPTION all name import, connection.table(...), connection.sql(...) and "raw-SQL function forms" — three of the seven in your own enumeration. given: and ##! are refused and documented nowhere, so an author declaring a given at append hits a refusal no doc surface predicted. Two untouched surfaces: api-doc.yaml:5624, the scope enum description that AGENTS.md §7 points unattended agents at as the complete spec, still describes append as plain concatenation with no mention of the refusals or the new 400; and docs/authorize.md:430 — "/compile still compiles unrestricted… Closing this (restricted compilation on /compile, as on /query) is tracked as a follow-up" — is the one doc stating this exact posture, and this PR is the follow-up it names. Worth updating to what actually shipped: append refused, file/package still unrestricted and caller-selectable, keep /compile behind the trusted tier.

[MED — no release note] RELEASE_NOTES.md asks for an ## [Unreleased] — title section "in the PR that changes the behaviour" for breaking changes and migration steps. A 200→400 on the default scope of a public endpoint, for text that compiled yesterday, is one. Cheaper to write here than to backfill.

[LOW] logger.error at environment.ts:983 fires on a caller typo in modelPath — an unauthenticated 400 emitting ERROR-level logs at whatever rate a caller likes; warn matches what it is.

[LOW] On the redundant base-model compile you already logged as a follow-up: one thing to fold into it when you get there — it sits inside the per-package mutex, which is mutually exclusive with installPackage's swap and deletePackage's retire, and it reintroduces the read-1/read-2 asymmetry inverted (read 1 succeeds, a transient failure on read 2 now 400s a request that would have served). Compiling from the modelContent already read at environment.ts:645 removes both along with the extra compile.

---

Top three before merge: confirm extendModel namespace/flag parity or pin it; update the two skills that currently instruct agents into a guaranteed 400; split missing-file from does-not-compile so the debug loop keeps its diagnostics.

_{Reviewed with Claude Code — session codename Bashful Vole.}

[sagarswamirao]

The append-scope gate is the change. compile_model and POST /compile default to append, and that text is a fragment against a published model, the same class of caller text loadRestrictedQuery already refuses. file and package staying on runtime.loadModel is the rest of the rule: that text is the model file. This is not a half implementation of "restrict every scope."

What is still not in the diff, and has to land before this matches the behavior:

1. api-doc.yaml (scope on the compile request, around the default: append description). It still says append is "the historical behavior" of concatenating and compiling, and that the only 400 is an invalid enum value. Append now refuses import, connection.table(...), connection.sql(...), given:, ##!, and the raw-SQL function forms with restricted-construct-forbidden, and a base model that cannot be schema-fetched is a 503. Clients generate from this spec.

2. docs/authorize.md still says /compile compiles unrestricted and that restricted compilation on /compile is a follow-up (the note near the end of the page, and the "false lock does not cover /compile" callout above it). This PR is that follow-up for append only. The page should say append is restricted, and that file and package are still unrestricted and caller-selectable.

3. RELEASE_NOTES.md. A 200 to 400 on the default scope of a public endpoint, for text that compiled before (import, connection.table(...), a new source rooted in a table), needs an Unreleased note and the migration: those constructs belong at file or package.

4. A refusal counter. Neither refusal path increments a metric, so a restricted-construct 400 is indistinguishable from a bad fragment in the data. One counter with a reason (restricted_construct versus base_model_load_failed) is enough.

The skills, the namespace and ##! parity tests, and returning a broken base model's own problems are already on the branch. The extra base-model compile inside the per-package mutex can stay a follow-up: compiling from the modelContent already read for the real compile removes it.

[housejester]

Follow-up pass at e560d55c, run this time with a runtime (bun 1.4.2): compile_restriction.spec.ts and compile_scopes.spec.ts pass 36/0 locally. Most of this morning's list is closed:

• The export and ##! parity cases are pinned. I also checked names brought in by import and by import { x } from, and both are classified and refused.
• Both skills are corrected.
• given: and ##! are named in every doc surface, and api-doc.yaml, authorize.md and the release note are in.
• The failure log is now warn, and the refusal counter is in.

Two things are left. The first reopens the namespace-parity invariant, on the parse side this time.

[HIGH, same fail-open family as 1af0b37d] The gate and the real compile parse different text

The gate compiles source on its own. The real compile runs ${modelContent}\n${source} (environment.ts:677). A fragment that is a syntax error on its own, but continues the model's last statement, gets no restricted-construct-forbidden from the gate, so the gate passes it and the unrestricted compile runs it.

Against the suite's own BASE_MODEL (which ends in ... extend { ... }), at scope: "append":

 extend { join_cross: s is duckdb.sql("SELECT * FROM read_csv('<path>')") }
run: base_source -> { group_by: s.ssn }

fragment	result
<path> exists	200, problems: []
<path> missing	200, invalid-sql-source: Invalid SQL, IO Error: No files found that match the pattern "…/nope.csv", with the DESCRIBE echoed
group_by: s.not_a_col	200, 'not_a_col' is not defined
extend { dimension: v is read_csv!string('<path>') } + a run:	200, problems: []
control: the same duckdb.sql(...) as a standalone run:	CompileRefusedError

So the file-existence and column oracles from the description come back in full at the default scope. The setup needs only a model whose last statement is a source definition, which is the usual case: most bundled models end in }, and examples/governed-analytics/internal.malloy ends in a bare duckdb.table(...).

On severity: this is no new exposure while file stays open to the caller, as the SCOPE header already says. But it breaks the premise the gate rests on, and it becomes a real hole the moment scope authorization lands and append is the only scope a read-only caller gets.

The outcome I'd want is that the gate judges the same unit that runs, and that a gate compile which stopped before classifying anything (here, a parse error) does not count as a pass. The mechanism is your call. One option: load the base model once, and run the real append compile as an unrestricted extendModel on it, offsetting diagnostic positions by the model's line count. That would also resolve the redundant-compile-inside-the-mutex follow-up. Either way, a spec case with the payload above would pin it.

[MED] The 503 split keys on a code that doesn't mean "unreachable"

In environment.ts:1024, failed-to-fetch-table-schema is also what a table that doesn't exist produces. With source: z is duckdb.table('no_such_tbl'), an append compile answers 503 "a problem reaching the data source", which tells the client to retry a permanent authoring error.

The reverse also happens: a conn.sql(...)-rooted model whose warehouse is down fails with invalid-sql-source, so that outage gets the 400.

Separately, the description says package scope "already maps its analogous failure this way", and it doesn't. Its 503 (:871) is for the compile worker pool being unavailable. At package and file scope the missing table comes back as an ordinary 200 diagnostic, which I checked.

I'd suggest one of these, and correcting that sentence either way:

• drop the 503 branch and let these fall into the "does not compile" 400, or
• classify on a real connectivity signal.

Smaller

• [LOW-MED] On the broken-base-model case from this morning: the problems now arrive inside the 400's message string rather than as the structured diagnostics (model, line, character) append used to return. The debug loop gets the text but loses the coordinates. The absolute paths in that message are not a new disclosure, since file and package scope already return the same file:///… paths.
• [LOW] The release note says a model "that does not exist, or that does not itself compile, answers 400 with the model's own problems". The does-not-exist case is deliberately opaque, so that sentence wants splitting.

_{Reviewed with Claude Code — session codename Spicy Dugong.}

[sagarswamirao]

Both findings are fixed in d3e1ca15. Thank you for the payload -- I reproduced it before changing anything, and it behaved exactly as you described: problems: [], at the default scope, against the shipped storefront package.

HIGH: the gate and the real compile parse different text

Fixed, but not by the mechanism you suggested, because that one does not work and it is worth recording why before someone else reaches for it.

Running the append compile as an unrestricted extendModel on the loaded base model -- or equivalently handing the existing gate the concatenation -- fails on the model's own declarations. extendModel judges text as an extension of a model that ALREADY holds them, so ${modelContent}\n${source} comes back as:

Cannot redefine 'customers'
Cannot redefine 'products'
Cannot redefine 'order_items'
'brand_suggest' is already defined, cannot redefine

and those abort the compile before anything in the appended fragment is classified. I tried it first, and the integration suite caught it: the continuation payload still passed, because the gate was now aborting even earlier than before. It is a second bypass wearing the shape of a fix.

So the two units stay different, and the gate instead refuses when it could not parse what it was given. Classification walks a parsed tree, so unparsed text was never judged -- and for a gate, that absence is not evidence of safety. It is keyed on Malloy's own syntax-error code (lang/parse-log.d.ts), not on message text. An ordinary semantic error still flows through as the caller's diagnostic, because there the tree WAS walked and the absence of a rejection is real evidence.

That covers the class rather than the instance: a continuation fragment is by definition a syntax error standing alone, whatever it continues and whatever it reaches for once concatenated.

One consequence you should weigh in on

This necessarily catches every non-parsing fragment, not only hostile ones. A bare view: / dimension: / measure: -- which docs/ai-agents.md already steers people away from -- now answers 400 with "send text that stands alone" instead of returning no viable alternative at input 'view:' as a diagnostic. Both rejected it before and after; the channel changed.

I think that is right, since the gate genuinely cannot check text it cannot parse, and the refusal says what to send instead. But it is a UX change beyond the security fix, so it is your call whether it wants softening. compile_fragment_techniques.spec.ts pins the new contract rather than having been deleted for being inconvenient.

MED: the 503 that meant two things

Dropped, taking your first option. You are right that it was wrong in both directions: failed-to-fetch-table-schema is also what a table that does not exist produces, so a permanent authoring error was telling the client to retry, and a conn.sql(...)-rooted model whose warehouse is genuinely down fails with invalid-sql-source and took the 400 anyway. Malloy publishes no code here that means "unreachable", so classifying on a real connectivity signal is not available today. These are now one 400 carrying the model's own problems.

You were also right that the description's claim about package scope was false -- its 503 is for the compile worker pool being unavailable, not for this. That sentence is gone rather than reworded.

LOW

The release note now splits the two cases: a model that does not COMPILE answers 400 with its own problems, and a model that does not EXIST answers 400 saying only that it could not be loaded, because naming the fault for a caller-supplied path would answer "does this file exist" for any path.

The structured-diagnostics point on the broken-base-model case is real and I have not changed it: those problems still arrive inside the 400's message rather than as model/line/character. Happy to take it in a follow-up if you want the coordinates back.

Testing

Written against the SHIPPED packages rather than a fixture, on the reasoning that whether the attack's precondition holds -- "the model's last statement can be continued" -- is a property of the models we actually ship. A fixture would be one I chose to be vulnerable. storefront.malloy ends in a query: block and internal.malloy in a bare duckdb.table(...), so both are covered and in different shapes.

compile_continuation.integration.spec.ts (new, 9 cases) loads both into a real environment and asserts:

• the continuation payload is refused on both packages;
• the present-file and missing-file refusals are byte-identical, and name neither the path nor the file's columns -- the oracle closed, not merely a refusal added;
• the standalone raw-SQL form the gate always caught is still caught (the control that shows these cases test the new behaviour);
• an ordinary fragment compiles clean and the query runs, returning real rows from DuckDB;
• a misspelled field is still a diagnostic, so the parse-failure branch does not eat the authoring loop;
• diagnostic positions still land past the model's own line count, which four doc surfaces promise.

Numbers: the four compile specs together are 53 pass / 0 fail; typecheck:server is clean. Disabling the parse-failure branch fails exactly the four continuation cases and leaves the five controls green.

On the wider suite: it sits at 82 failures here, and 79 of those are present on a clean tree in this worktree (broken test mocks -- storageManager.getDuckDbConnection is not a function and similar -- in theme, materialization and MCP). Of the three that appeared alongside my change, two (dashboard tile normalization > stays linear, a 15s wall-clock assertion, and a 4th case in an already-failing concurrency E2E) reproduce with my changes stashed, so they are not mine. The third is the bare-view: contract update above.

[housejester]

Approving at d7081098. d3e1ca15 closes both findings, and the merge after it touches nothing on the compile path. The four compile specs pass 53/0 locally at this head.

Continuation bypass. Refusing on the gate's own syntax-error covers the whole class, not just the payload I sent. A continuation has to start with a token that no top-level statement can start with, so it can't parse on its own. I checked that by running it rather than reasoning about it:

• Refused: my original payloads, plus extend {…} after a bare source, + {…} refinement after a query:, -> {…} after a query: or run:, a , s2 is duckdb.sql(…) comma continuation, and include {…}.
• Lexer-level failures (an unterminated string, stray bytes) also arrive as syntax-error, so they are refused too.
• A semantic error followed by a raw-SQL statement is still classified and refused with the raw-SQL message, so the pass-through path opens nothing.
• Ordinary fragments and names brought in by either import form are unchanged.

One correction on my side. What I suggested last round was extending the loaded model with the fragment alone, run unrestricted as the real compile, not handing extendModel the concatenation. My wording made the other reading easy, so that's on me. Nothing needs to change, because the parse-failure refusal is simpler and sufficient. The new code comments do present "the obvious alternative" as ruled out, so a later reader may take that as covering the fragment-only form too.

503. Dropping it was right. A missing table now answers 400 with the model's problems, and I confirmed that.

Softening the bare view: refusal: I'd leave it as is. It was never valid at append, the refusal says what to send instead, and docs/ai-agents.md already teaches wrapping it in query:. If it's ever worth softening, the gate could return its own parse errors as ordinary diagnostics and never run the unrestricted compile. That keeps the gate failing closed and brings back the old response shape, but the positions would need offsetting by the model's line count. That's a follow-up, not something for this PR.

Non-blocking follow-ups:

• The "must parse on its own" rule is in docs/ai-agents.md and the release note, but not in the MCP tool description (COMPILE_DESCRIPTION in compile_tool.ts) or in the scope description in api-doc.yaml. AGENTS.md:233 still tells agents to send "a view body you are drafting… on its own", which is now a 400.
• Structured diagnostics for a broken base model, which you've already offered to do.
• The redundant base-model compile inside the per-package mutex.

_{Reviewed with Claude Code — session codename Spicy Dugong.}

[sagarswamirao]

Thanks for the approval, and for running the class rather than the payload -- the refinement and comma-continuation cases are ones I had not tried.

Taking the correction: you did say the fragment alone, extended onto the loaded model, and I read it as the concatenation. That is on me as much as the wording; the two differ materially and I should have asked rather than inferred.

Two of your three follow-ups are in 141096f5, because one of them was worse than a gap.

AGENTS.md:233 was instructing agents into the 400. "Send that fragment on its own" is the sentence, in the file agents read to learn this tool. It now carries the rule and the two shapes that work -- a top-level query: for a view body, a throwaway extend for a field. COMPILE_DESCRIPTION and api-doc.yaml's scope say the same.

api-doc.yaml also still documented the 503. Not something you flagged -- I found it while adding the stand-alone rule, and it is a spec-level claim about a branch this PR deleted. It now says what the code does: a model that does not compile answers 400 with its own problems, one that does not exist answers 400 saying only that it could not be loaded.

The comment you flagged as over-claiming now separates the two readings: the concatenation is ruled out, the fragment-only extendModel is recorded as live, with its cost (offsetting positions by the model's line count) and its other payoff (dropping the second compile from inside the mutex) written next to it.

Left for follow-ups, as you suggested: structured diagnostics for a broken base model, and the mutex compile -- which the note now points at, since they are the same change.

On the bare view: refusal, agreed, leaving it. Your alternative (gate returns its own parse errors as diagnostics, never runs the unrestricted compile) is a better shape than softening the refusal, and it is the same offsetting problem as the fragment-only form, so the three follow-ups collapse into roughly one piece of work.

Verified at this head: typecheck:server clean, four compile specs 53 pass / 0 fail under the project's --timeout 100000. Note your merge landed while I was pushing; I merged rather than forced, and the release stamp put your #(access_filter) section at [0.6.0] with the append-scope note still [Unreleased] above it, which is the right split.

Versions

Both independent packages sat at exactly their published npm versions -- skills 0.1.22, create-malloy-package 0.0.18 -- and the release workflow publishes each only when the local version is AHEAD of npm, skipping quietly otherwise. This branch changes malloy-dashboards and malloy-modeling, so that content would have shipped in no release with CI green. Bumped to 0.1.23 and 0.0.19; the scaffolder moves with skills because it declares workspace:* on it. Lockstep server/sdk/app stay at 0.6.0, which the release stamps.

bunx prettier './packages/**/*.{ts,tsx}' --write reformatted the two specs this branch adds and nothing else.