app-modeling: replace the modeling prose with a check on the compiled output - #32
app-modeling: replace the modeling prose with a check on the compiled output#32AzureMike wants to merge 19 commits into
Conversation
df1428f to
c75db1f
Compare
… output The skill carried 1,122 lines of rules across SKILL.md and eight reference files, and most of them described resource shape: property names, which fields are maps, what is required. The Bicep compiler already knows all of that and reports it with a line number, so restating it made the guidance long without making the output better. What the compiler doesn't know is what a Recipe returns. A generated eShop model compiled to 24 resources and could not run, because it read db.properties.port on postgreSqlDatabases in five connection strings and the deployed Azure Recipe never sets that property. The type schema really does declare port, which is why it compiled. That same model emitted the literal string 'managedSecret:connectionString' as workload configuration in seven places, and a literal like that reaches the container verbatim. check.py reads the compiled ARM JSON rather than the Bicep text and reports ALLOW, WARN, or DENY with a stable signature, so a repair that produces the same signature twice means the fix isn't converging and the loop should stop. It uses no regular expressions, since ARM expressions are a small formal language and scan cleanly by hand. recipe-outputs.json records what each pinned Recipe actually sets, with provenance, and authoring.md keeps one worked example and the eight rules a clean compile can't establish. Run against eleven models, nine of them already generated, the checks found a Recipe output that is never set, a managed secret passed as a literal string, a secure parameter passed in argv where the pod spec exposes it, a workload with no configuration at all, and a config override that nothing expands at runtime. Three of those models had already shipped. Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
check.py's contract is now: DENY is a theorem, provable from the compiled ARM plus recipe-outputs.json. The verdict is binary (ALLOW/DENY) and the heuristic WARN tier is gone; judgment calls belong to the model and the prose in authoring.md. Fixes three false-positive DENYs that trapped the repair loop: reading back a property the template itself sets is now allowed; secret key names are no longer substring-matched against unrelated env literals; and the documented config-file-in-secret pattern is expressible again via a sanctioned #disable-next-line for non-credential content. Per radius-project/radius#12450, every compiler diagnostic in the SARIF denies — errors, warnings, and linter rules alike — so the compile must be warning-free. secret-composed-in-template is promoted from WARN to DENY (materializing a secure parameter into deployment state is provable, and rule 5 is categorical). missing-connection now also covers generated Radius.Resources/* custom types. A hard compile failure reports a structured compile-failed finding instead of a traceback. authoring.md restores the build/delivery rules the prose rewrite lost: explicit build.platforms, //<subdir> and build.dockerfile sources, BUILDKIT_CONTEXT_KEEP_GIT_DIR, config-file delivery by mounted secret, URL-encoding and $(VAR) ordering for runtime composition, and no commentary in generated Bicep. Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
Make checker failures explicit and provable, handle malformed inputs safely, and preserve compiler diagnostics. Tighten Recipe and secret validation while leaving unprovable expression forms to authoring guidance. Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
fcfc9a7 to
c78ec41
Compare
Twenty matched runs on getting-started-todo-app produced first-try ALLOW models in all ten PR-skill runs, but the traces exposed four policy gaps. Two field bugs in radius-project/ai-extensions#128 and #131 exposed two more. - Add reserved-property-value using exact values and prefixes stored with the Recipe contract. Two runs copied MySQL root, while PostgreSQL defaults such as postgres and pg_ names fail for the same reason. Malformed constraint data now makes the checker unusable instead of silently skipping the rule. - Choose unsupported service versions deterministically. Use the greatest supported version that is not newer than the source, and use the lowest newer version only when the repository proves protocol, TLS, and authentication compatibility. Otherwise generate a custom type. - Give backing resources application-scoped names because several Azure Recipes pass the Radius name into globally scoped cloud resources. - Bind published connection-string secrets instead of composing incomplete values from host and port. - Keep the modeling agent from reading checker source or retrying a rejected push, which consumed turns without improving the model in six of ten traces. Replaying the twenty generated models makes the two root-username models DENY on reserved-property-value; the other eighteen still ALLOW. Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
The Shape example named its resources with bare service names, which rule 24 now forbids, and its app name was adoptable verbatim by the most common demo-app category — an example a model can mistake for an answer. Rename everything in it to overt placeholders (example-app, example-app-mysql) so the only thing the example can teach is shape. Two rules learned from cross-app evaluation, stated generically: - Rule 5: Kubernetes $(VAR) expansion happens before the process starts, so expanding a secret-valued variable into command or args prints the secret in the pod process list. Prefer env-native settings. This stays prose rather than a check.py rule: for an application whose only configuration surface is a flag or URL, every alternative wiring is also denied, and an unfixable DENY is worse than a documented trade-off. - Rule 17: an image that ships an auth gate or first-run wizard needs that surface configured — disabled or provisioned — so the first request lands in the working application, not on a setup screen. Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
Resolve the app-modeling documentation conflicts against upstream main. Keep the upstream SKILL.md and custom-resource-types.md structure, then point the consolidated rules at authoring.md and retain the compiled checker, scoped naming, connection, route, and generated-artifact requirements. Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
…LL.md Auditing ff5a333 against the pre-merge branch showed the merge kept the checker pipeline but dropped the reasoning that makes it load-bearing, and left three cross-document contradictions: - Compile and check again explains why exit codes cannot be trusted (bicep exits 0 on warnings; unknown types and credential leaks are warnings), why --diagnostics is required, that non-SARIF output fails, and that the signature hashes findings rather than wording. - authoring.md pointed at "step 5 of SKILL.md#workflow", which the new structure made a different step; it now points at the Compile and check section. - authoring.md's "Touch no other file" contradicted the custom-type artifact flow the merge made prominent; the boundary is now "no file outside .radius/". - The naming tables reintroduced todo-list-app example names and prescribed symbolic names and connection keys that disagreed with the Shape example. Examples are placeholders again, the Shape example now follows the tables, and the version row links rule 22 for unsupported source versions. - The reply spec regained the judgment-call and unsupported-component items the merge dropped. Signed-off-by: AzureMike <127820851+AzureMike@users.noreply.github.com>
Signed-off-by: AzureMike <127820851+AzureMike@users.noreply.github.com>
Signed-off-by: AzureMike <127820851+AzureMike@users.noreply.github.com>
Keep SKILL.md focused on the workflow and make authoring.md the detailed policy. Add an ALLOW-checked example, a bounded repository exploration procedure, and exact schema and Recipe lookup sources. Signed-off-by: AzureMike <127820851+AzureMike@users.noreply.github.com>
aae0fe3 to
7b325db
Compare
Signed-off-by: AzureMike <127820851+AzureMike@users.noreply.github.com>
Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
Signed-off-by: Mike Azure <127820851+AzureMike@users.noreply.github.com>
kachawla
left a comment
There was a problem hiding this comment.
I see that the old reference files have mostly been combined into authoring.md, while rules that can be checked automatically were moved into check.mjs. Could we still keep some separation by topic? Both files are now quite large, and smaller focused files would be easier to understand, test, and update.
It also looks like some useful guidance was removed, including architecture patterns, component detection clues, and information about services without predefined Radius types. These help the skill discover dependencies that the checker cannot catch later.
Thinking about this further.. could we split this work into smaller PRs and move one area at a time? This PR changes a lot at once, which makes it easier to miss behavior and guidance that we have added to the skill over time.
| } | ||
|
|
||
| export async function readRecipeContract() { | ||
| return compileRecipePack(await fetchRecipePack()); |
There was a problem hiding this comment.
Since this Recipe Pack is pinned to a specific commit, do we need to download and compile it every time? A temporary GitHub, network, or Bicep issue could make the check fail even when nothing has changed. Could we cache it using the commit SHA?
| # Radius Application Modeling | ||
|
|
||
| Use this skill to generate a Radius application definition (`app.bicep`) from a source code repository. | ||
| Generate `.radius/app.bicep` from source evidence, then compile and check the |
There was a problem hiding this comment.
Why does this need to be updated?
| 2. Commit the generated files and push when a remote is configured. If push | ||
| authentication or authorization fails, report the committed branch and stop. | ||
| Do not set up credentials. | ||
| 3. Start the reply with a one-line introduction naming the application, then |
There was a problem hiding this comment.
Same question here, was the previous format causing issue with the new script?
| - `app.bicep` compiles with both extensions and the | ||
| [compile-and-check loop](../SKILL.md#compile-and-check) returns `ALLOW`. Every | ||
| custom resource is consumed by a workload, and that workload declares a | ||
| connection to it. |
There was a problem hiding this comment.
Not sure if I'm following the updated wording here.
The app-modeling skill reads a repository and writes a Radius
app.bicep. All of its guidance used to be prose, spread across eight reference documents and another 255 lines inSKILL.md. This change cuts the general guidance to one document, keeps the custom-resource-type guide separate, and adds a script that inspects the file the skill just generated.The old design
Every rule lived in a document the model was expected to have read and kept in mind.
bicep-structure-rules.mdwas 278 lines of property names, required fields, and which things are written as objects rather than arrays. Seven more covered naming, secrets, connections, the runtime contract, architecture patterns, a catalog of components, and a worked example.When a generated model came out wrong, the obvious fix was another rule, and that works for a while. Each rule gets written after watching one repository fail, so it covers that repository. But past a certain point the model can no longer hold all of the rules in mind while reading unfamiliar source code, and a rule that only ever mattered for one repository is still sitting there being read on every other one.
Why more words weren't the answer
Take a real rule: an
app.bicepmust resolve the Radius extension. If it doesn't, Bicep can leave the resource types unresolved instead of compiling the model the skill meant to write.This is a plain fact about the finished file. A script can settle it directly instead of asking the model to carry the rule in mind through every source file it reads.
The same is true of rules such as declaring exactly one application, using valid resource shapes, reading only Recipe outputs that exist, binding only published secret keys, connecting resources the workload consumes, and pinning source builds to an immutable revision.
Most of the old prose was like this: claims about the finished file that a script can check.
What the model is better at
A script can only check the file that was written. It cannot read the source repository and work out which of several ways to run an application is the one worth modeling. It also cannot decide whether the source needs TLS, session state, bootstrap users, writable storage, or a setting whose default breaks the main operation.
The model still does this part. It follows one real operation from input, through the workloads and backing services, to an observable result. After the checker passes, it compares that same source trace with the final model so a checker repair does not quietly remove something the application needs.
The new design
The skill now runs in nine steps.
1. Check the repository. Confirm that the repository and required Dockerfile are present before writing anything.
2. Resolve the target. Read the exact Radius schemas and Azure Recipe Pack the model will use.
3. Read the source. Start with Dockerfiles, manifests, startup scripts, and the configuration the application actually reads.
4. Pick one way to run it. Follow one complete operation and list the workloads and backing services that operation needs.
5. Prove the profile is complete. Check the listener, protocol, TLS, authentication, startup, migrations, persistence, image user, and provider requirements. If a required fact is missing, stop instead of guessing.
6. Resolve every Radius field. Write
.radius/bicepconfig.json, then check each type, property, output, and secret key against the selected schema and Recipe.7. Check the image. Build from a clean checkout, or pull the exact digest for an allowed release image, then inspect the image's user, command, working directory, platform, and volumes.
8. Write the model. Generate
.radius/app.bicepwith the workloads, resources, connections, secrets, mounts, and lifecycle settings the selected profile needs.9. Compile and check. Fix every proven checker finding, then compare the final model with the source trace again.
When no predefined Radius type fits a required Azure service, the existing custom-resource-type guide still describes the generated schema, extension, Recipe, and Recipe Pack.
Compile and check
The skill compiles in a fresh temporary directory and runs the checker even when Bicep reports an error:
The checker is
scripts/check.mjs. It reads the compiled ARM, the compiler diagnostics, and the Azure Recipe Pack pinned to an immutableresource-types-contribcommit.Compiled ARM can still contain expressions, so the checker resolves only the forms it can prove. It rejects known compiler, ARM, schema, and Recipe-contract defects, but leaves uncertain source and runtime questions to the skill. This is deliberate: letting a bad model pass is safer than rejecting a correct one.
The pinned Recipe contract also supplies provider constraints that compiled ARM can prove, including safe forms for globally scoped names and required managed connection secrets.
An
ALLOWresult means the checks the script can prove passed. It does not mean the application was deployed or that its main operation worked.Tests
tests/app-modeling/test_check.mjscontains 107 Node tests. They cover each checker rule, valid counterexamples, malformed input, Recipe contracts, finding deduplication, stable signatures, and expressions the checker cannot safely classify.