Carry Well-Architected workload goals through scaffolding and into deployment - #1878
Open
Nathan (nturinski) wants to merge 8 commits into
Open
Nathan (nturinski) wants to merge 8 commits into
Nathan (nturinski) wants to merge 8 commits into
Conversation
Establish a schema-v3 workload quality contract during requirements with operating profile, data classification, traffic profile, and optimization priority. Render those choices as a five-pillar Quality Attributes & Tradeoffs card in the approved plan, while rejecting false WAF compliance claims. Make scaffold consume the contract as application-level controls for health, timeouts, bounded retries, graceful degradation, authorization, redaction, correlation, errors, and request bounds. Persist concrete evidence and executable validations in integration-plan.md, then require the fresh integrate session to re-run them after replacing mocks with live clients. The evaluator now requires every workload question, all five plan pillars, evidenced integration controls, deferred risks, allowlisted values, and honest claim language. Certification mutations prove each check goes red; the suite passes 178/178. Live workload assertions cover cost-constrained, latency- sensitive, confidential HR, and regulated-health prompts. Also fix the existing timeout reference, which created an AbortController but never passed its signal to the operation, and harden runtime logging patterns with correlation and sensitive-data allowlisting. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Address the malformed workload-value validation gap and the two guidance/checklist issues before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a schema-v3 workload-quality contract across requirements, planning, scaffolding, integration, telemetry, runtime guidance, validation, and documentation.
Changes:
- Carries four workload-profile choices through project artifacts.
- Adds quality controls, tradeoff plans, and integration evidence.
- Expands telemetry, fixtures, certification, and documentation coverage.
File summaries
| File | Summary |
|---|---|
test/testProjects/copilotOnRails/scrapbook/requirements.json |
Updates workload requirements. |
test/testProjects/copilotOnRails/scrapbook/project-plan.md |
Adds quality tradeoffs. |
test/testProjects/copilotOnRails/scrapbook/integration-plan.md |
Adds integration contract. |
test/testProjects/copilotOnRails/attendance/requirements.json |
Updates workload requirements. |
test/testProjects/copilotOnRails/attendance/project-plan.md |
Adds quality tradeoffs. |
test/testProjects/copilotOnRails/attendance/integration-plan.md |
Adds integration contract. |
test/copilotOnRails/scaffoldPlanTelemetryUtils.test.ts |
Tests plan telemetry. |
test/copilotOnRails/requirementsTelemetryUtils.test.ts |
Tests requirements telemetry. |
test/copilotOnRails/parseScaffoldPlanMarkdown.test.ts |
Tests quality parsing. |
src/webviews/copilotOnRails/views/utils/parseRequirements.ts |
Defines workload parsing. |
src/webviews/copilotOnRails/views/ScaffoldPlanView.tsx |
Renders quality attributes. |
src/webviews/copilotOnRails/extension/utils/scaffoldPlanTelemetryUtils.ts |
Adds allowlisted telemetry. |
src/webviews/copilotOnRails/extension/utils/requirementsTelemetryUtils.ts |
Adds workload telemetry. |
resources/agents/shared-references/workload-quality.md |
Defines quality controls. |
resources/agents/shared-references/runtimes/typescript.md |
Updates runtime guidance. |
resources/agents/shared-references/runtimes/python.md |
Updates correlation logging guidance. |
resources/agents/shared-references/runtimes/dotnet.md |
Updates runtime guidance. |
resources/agents/shared-references/resilience.md |
Updates timeout and retry guidance. |
resources/agents/azure-project-scaffold/references/sub-agent-strategy.md |
Extends scaffold responsibilities. |
resources/agents/azure-project-scaffold/references/frontend-quality-bar.md |
Updates quality references. |
resources/agents/azure-project-scaffold/references/frontend-preview-steps.md |
Updates preview guidance. |
resources/agents/azure-project-scaffold/instructions.md |
Adds workload quality rules. |
resources/agents/azure-project-scaffold.agent.md |
Adds quality handoff requirements. |
resources/agents/azure-project-plan/requirements.md |
Defines schema-v3 requirements. |
resources/agents/azure-project-plan/references/html-preview.md |
Updates preview references. |
resources/agents/azure-project-plan/plan.md |
Adds quality tradeoff section. |
resources/agents/azure-project-plan/instructions.md |
Routes schema-v3 planning. |
resources/agents/azure-project-plan.agent.md |
Updates planning contract. |
resources/agents/azure-project-integrate/references/wire-live-data.md |
Adds live-client controls. |
resources/agents/azure-project-integrate/instructions.md |
Adds quality verification. |
resources/agents/azure-project-integrate.agent.md |
Adds integration handoff requirements. |
evals/src/graderCertification.ts |
Registers workload certification. |
evals/src/artifacts/requirements.ts |
Validates workload requirements. |
evals/src/artifacts/projectPlan.ts |
Validates quality plans. |
evals/src/artifacts/integrationPlan.ts |
Validates integration contracts. |
evals/results/grader-certification/offline/report.md |
Refreshes certification report. |
evals/results/grader-certification/offline/report.json |
Refreshes certification results. |
evals/msbench/config/stimuli/redteam-skip-security-health-app.yaml |
Adds workload assertions. |
evals/msbench/config/stimuli/realtime-whiteboard.yaml |
Adds latency assertions. |
evals/msbench/config/stimuli/private-internal-hr-tool.yaml |
Adds classification assertions. |
evals/msbench/config/stimuli/cost-constrained-blog.yaml |
Adds cost and traffic assertions. |
evals/graders/validate-requirements.ts |
Adds workload assertions. |
evals/grader-certification/stage-local-dev/.azure/integration-plan.md |
Updates fixture contract. |
evals/grader-certification/sample-agent-output/.azure/requirements.json |
Updates fixture requirements. |
evals/grader-certification/sample-agent-output/.azure/project-plan.md |
Updates fixture plan. |
evals/grader-certification/sample-agent-output/.azure/integration-plan.md |
Updates integration fixture. |
evals/grader-certification/manifest.json |
Adds mutation coverage. |
evals/grader-certification/api-only-no-datastore/.azure/integration-plan.md |
Updates fixture contract. |
evals/agent-assets.lock.json |
Refreshes asset hashes. |
docs/copilot-create-project.md |
Documents workload quality flow. |
docs/copilot-create-project-security.md |
Documents artifact security rules. |
Review details
- Files reviewed: 51/51 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+182
to
+188
| for (const [field, value] of [ | ||
| ['answer', parsed.answer], | ||
| ['recommendedChoice', parsed.recommendedChoice], | ||
| ] as const) { | ||
| if (value !== null && value !== undefined | ||
| && (typeof value !== 'string' || !(contract.options as readonly string[]).includes(value))) { | ||
| issues.push(issue('invalidWorkloadChoice', `${path}.${field}`, `${id}.${field} must be one of its declared options.`)); |
| > 5. **Quality evidence**: the artifact's Application Controls table names at least one concrete control for | ||
| > every pillar, points at real generated evidence, and gives the integration session an executable | ||
| > validation. Deferred production/compliance/numerical targets are preserved. | ||
| > 5. **Hand-off**: For a project **with a frontend** (interactive mode), opened the UI-approval gate via the `open_frontend_preview_view` tool and stopped — the gate's **Approve UI** button performs the hand-off. For a **no-frontend** project (or autopilot), started the `azure-project-integrate` session via the `start_project_integrate` tool. Either way, did NOT call `vscode_askQuestions` or print next-step suggestions. |
Comment on lines
513
to
+523
| duration_ms = round((time.time() - start_time) * 1000, 2) | ||
| correlation_id = incoming_correlation_id or str(uuid.uuid4()) | ||
| structlog.contextvars.bind_contextvars(correlation_id=correlation_id) | ||
| logger.info( | ||
| "request_completed", | ||
| method=method, | ||
| path=path, | ||
| route=path.split("?", 1)[0], | ||
| status=status_code, | ||
| duration_ms=duration_ms, | ||
| ) | ||
| return correlation_id |
The workload contract stopped at integration. Controls were implemented and evidenced locally, then deployment re-derived what to provision from the plan, so the two could disagree without anything noticing until after resources existed. Measured across paired live deployments of this branch and `main`, every deployment-phase failure came from that gap rather than from the controls themselves. Adds the row that closes it, plus fixes for the three defects the A/B run surfaced. ## Dependency access contract `workload-quality.md` gains a Dependency Access table — dependency, the operation the code actually calls, the deployed identity, the permission that authorizes it, and the local equivalent — carried in `integration-plan.md` and read by deployment to choose the role it assigns and the probe it re-runs after role propagation. `SEC-IDENTITY-01` names it as a baseline control. Local emulators authenticate with a connection string that carries every permission, so an operation the deployed identity cannot call still passes every local test. Naming the operation next to the permission is what lets the two be compared before they are deployed. ## Health probes authorized by the role we assign `BlobServiceClient.getProperties()` needs `blobServices/read`, an ARM `action`, while `Storage Blob Data Contributor` grants `dataActions` only. Both deployments in the A/B run reported storage down on correctly-provisioned infrastructure because of it; `dotnet.md` was recommending it. It is now called out as wrong in both runtime references, with container enumeration (`containers/read`) as the probe the data role actually authorizes. The existing service-level rule still holds — the two together mean service-level *and* authorized. ## Serialized PostgreSQL child resources `parent:` orders children after the server, not after each other, so ARM starts them in parallel while a new Flexible Server is still becoming accessible and the administrator write returns `AadAuthOperationCannotBePerformedWhenServerIsNotAccessible`. It is a race, so it survives `bicep build` and `what-if` and cost two consecutive deployment attempts. The canonical pattern now chains firewall -> administrator -> configurations -> database. ## A migration path for compute with no shell The tier ladder assumed Container Apps `exec` or App Service SSH, so Functions on Flex Consumption — which has neither — fell through to the tier-3 firewall exception and hit a subscription `ScopeLocked`. Tier 2 is now any Azure-side one-shot job, which the existing Azure-services firewall rule already admits, with a table mapping app compute to its host. Tier 3 gains a lock precondition, the tier is chosen during prepare instead of mid-deploy, and inventing a migration endpoint inside the deployed app is called out as prohibited. ## Gates `integration-plan` now requires the Dependency Access table, rejects placeholder cells, and accepts an explicit `None` for projects that reach no Azure dependency — the shape `Deferred Risks` already uses. Two certification mutations prove it red: removing the section, and blanking a permission. Also fixes a pre-existing failure on this branch: the seed plan every scaffold and local-dev stimulus starts from had no `Quality Attributes & Tradeoffs` section, so `seed:contract` was staging input the planner would not emit. Certification 180/180 (178 + 2 new). drift, gates, phases, stacks, seed contract, red-team and import self-tests green. Extension: lint clean, `build:check` clean, 487 passing / 7 pending. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…d-contract # Conflicts: # docs/copilot-create-project.md
Reject invalid PostgreSQL Entra administrator names and same-deployment readiness races in both scaffold conformance scripts. Preserve Tier-2 migration evidence and require state-aware bounded retries before healing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Lock Azure targets and nested models before any side effects. Require product-owned inventory, executable PostgreSQL and migration readiness, validated prebuilt Flex packages, and live correlation proof. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Validate immutable targets and generated runtime contracts before what-if. Harden migration probes, selected-agent dispatch, and Windows inventory startup. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Carry the extension-resolved selection through phase queries so nested tasks cannot silently rediscover or substitute a default model. Reject CRLF agent assets before baseline updates to keep exact hashes portable across Windows and Linux. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
CoR already consulted Azure Well-Architected service guides during deployment, but the earlier stages never captured the workload requirements those recommendations are supposed to serve. The deploy agent therefore had to rediscover scale/quality intent, project scaffolding applied one-size-fits-all controls, and no artifact connected a requirement to application evidence or an accepted risk.
AVM/module selection is deliberately still out of scope. This implements the three layers needed before it:
Layer 3 and the three defects below were added after measuring this branch against
mainwith paired live Azure deployments; see What the A/B run measured.1. Workload contract
.azure/requirements.jsonmoves to schema v3 and always carries four fixed, reviewable questions:operatingProfiledataClassificationtrafficProfileoptimizationPriorityThese are business inputs, not WAF-pillar or Azure-service choices. v2 artifacts are upgraded and returned to the Requirements view before planning continues.
The approved project plan now contains a visible Quality Attributes & Tradeoffs card with the four exact approved answers and, per WAF pillar, a workload target, scaffold response, planned validation, and deferred risk.
The plan and integration validators reject
WAF compliant,WAF certified, and100% WAF aligned; this is evidence and risk traceability, not a score/certification.UI/telemetry
The Requirements view already supports Deployment, Compliance, Scale, and Operations categories. The Plan view previously discarded every non-stack section except its hard-coded special cases, so this explicitly renders the new card.
Telemetry records only allowlisted low-cardinality labels. Arbitrary artifact text is emitted as
unknown, never as telemetry.2. Application controls
Adds
shared-references/workload-quality.md, consumed by project scaffold and integrate. Every generated app has a baseline covering dependency health and healthy/degraded/unhealthy behavior, finite outbound timeouts, bounded retry with jitter excluding unprotected non-idempotent writes, Essential vs Enhancement failure handling, input validation and authorization, sensitive-log redaction, correlation IDs and structured errors, pagination/payload/concurrency bounds, and no speculative services for a pillar.Operating profile, data classification, traffic profile, and optimization priority add deterministic controls without letting Lowest Cost/Latency weaken the security/correctness floor.
.azure/integration-plan.mdcarries a Workload Quality Contract with all four workload answers, anID | Pillar | Control | Evidence | Integration Validationtable with at least one evidenced control per pillar, and explicit deferred risks. The fresh integrate session re-runs every listed validation after replacing mocks with live clients and appends PASS / FAIL / DEFERRED evidence.Controls that reach the browser now require frontend-level evidence too — comprehensive API tests plus no frontend test evidences half of what the contract claims.
3. Dependency access contract (new)
The contract previously stopped at integration. Deployment then re-derived what to provision from the plan, so the two could disagree with nothing noticing until resources existed.
integration-plan.mdnow also carries Dependency Access:Dependency | Operation | Deployed identity | Required permission | Local equivalentone row per Essential/Enhancement dependency, naming the operation the code actually calls. Deployment reads it to assign exactly that permission to that identity and to re-run each probe against the deployed identity after role propagation.
SEC-IDENTITY-01names it as a baseline control.The failure it closes is invisible locally: emulator connection strings carry every permission, so an operation the deployed managed identity cannot call still passes every local test.
⛔ A deployment that edits application source to make a control pass has changed the thing under test. It fixes the role, the ordering, or the probe named in the contract, and logs a healing attempt.
Three deployment defects found by deploying
Health probes that 403 under the role we assign
BlobServiceClient.getProperties()requiresblobServices/read, an ARMaction;Storage Blob Data ContributorgrantsdataActionsonly. Both conditions in the A/B run reported storage down on correctly-provisioned infrastructure, anddotnet.mdwas actively recommending it. It is now marked wrong in both runtime references, with container enumeration (containers/read) as the probe the data role authorizes. The existing service-level rule still holds — together they mean service-level and authorized.PostgreSQL child resources racing the server
parent:orders children after the server, not after each other, so ARM starts them in parallel while a new Flexible Server is still becoming accessible and the administrator write returnsAadAuthOperationCannotBePerformedWhenServerIsNotAccessible. Being a race, it survivesbicep buildandwhat-if; it cost two consecutive deployment attempts. The canonical pattern now chains firewall → administrator → configurations → database.No migration path for compute without a shell
The tier ladder assumed Container Apps
execor App Service SSH, so Functions on Flex Consumption — which has neither — fell to the tier-3 firewall exception and hit a subscriptionScopeLocked. Tier 2 is now any Azure-side one-shot job (already admitted by the Azure-services rule) with a compute→host table; tier 3 gains a lock precondition; the tier is chosen during prepare rather than mid-deploy; and inventing a migration endpoint inside the deployed app is explicitly prohibited.Phase-exit gates
The scaffold checkpoint is now an exit condition rather than a self-assessment, and reporting the phase complete while a delegated sub-agent is still generating is called out as a false completion.
Falsifiable coverage
Certification proves the new gate can go red — removing the Dependency Access section, and blanking a required permission:
An explicit
Noneis accepted for projects that reach no Azure dependency — the shapeDeferred Risksalready uses — so the gate stays satisfiable honestly.Also fixes a pre-existing failure on this branch: the seed plan every scaffold and local-dev stimulus starts from had no
Quality Attributes & Tradeoffssection, soseed:contractwas staging input the planner would not emit.What the A/B run measured
Paired live deployments in one subscription, same prompt and model,
main(A) vs this branch (B), both deleted afterward.mainBoth hit the Blob probe defect. B additionally exposed the PostgreSQL ordering and migration-path gaps — which is why they are fixed here rather than left as prose. The remaining gap is that WAF quality did not yet improve first-attempt deployment reliability; these three fixes target exactly that, and should be re-measured before treating the work as a reliability improvement.
Validation
mainis merged in and the branch is mergeable.Docs and screenshots
Updated the user guide, pipeline diagram, artifact inventory, diagnostics privacy statement, and artifact-security contract.
Screenshots to recapture:
04-requirements-view.png— now shows the four workload-quality groups/questions.05-plan-preview.png— now includes the Quality Attributes & Tradeoffs card.Follow-up scope
Re-run the A/B matrix (public + confidential, three repetitions each) with zero post-approval source/IaC edits and zero healing attempts as the acceptance bar. Then deployment rehydration — import the confirmed workload profile into App Onboard
context.json, persist per-pillar decisions/risks inprepare-plan.json, and render them in the Deployment Plan view. AVM-first Bicep selection comes after that, so the module catalog implements an approved architecture rather than choosing it.