mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Add mcp knowledge category with 5 rules
Rules derived from BC MCP API page development experience: - api-page-flowfields-must-be-calcfields: FlowFields return empty on API pages unless explicitly CalcFields'd in OnAfterGetRecord - stored-derived-fields-must-not-be-exposed-directly: Stored fields updated only via OnValidate triggers can be stale; recalculate live in OnAfterGetRecord - api-page-key-fields-must-be-editable-on-insert: ODataKeyFields with Editable=false are rejected as unknown properties on POST - api-page-least-privilege-write-access: Create dedicated minimal pages per write concern rather than widening general-purpose pages - agent-must-not-write-business-process-status: Agents must only write developer-tracking fields; business status fields affect invoicing/time registration Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
c2aaf92748
commit
935d756f05
5 changed files with 190 additions and 0 deletions
|
|
@ -0,0 +1,32 @@
|
||||||
|
# CURABIS MCP: Agents Must Not Write Business Process Status Fields
|
||||||
|
|
||||||
|
## Core Principle
|
||||||
|
|
||||||
|
MCP agents must only write developer-managed tracking fields — never fields that drive business process workflows such as invoicing, approval, or time registration. Writing a business status field from an agent can block downstream operations for users working in Business Central.
|
||||||
|
|
||||||
|
## The Distinction
|
||||||
|
|
||||||
|
| Field type | Examples | Agent may write |
|
||||||
|
|---|---|---|
|
||||||
|
| Developer tracking | `gitHubDevStatus`, `gitHubBranch` | Yes |
|
||||||
|
| Business process status | Task `Status` (Accepted, In progress, Done) | Never |
|
||||||
|
|
||||||
|
Developer tracking fields are independent of BC workflow. Business process status fields control what users can do — for example, a task marked as ready for invoicing cannot receive new time entries.
|
||||||
|
|
||||||
|
## Requirements
|
||||||
|
|
||||||
|
- API pages exposed to MCP agents must mark business status fields as `Editable = false`
|
||||||
|
- Agent instructions (`.agent.md` files) must explicitly list which fields the agent may write
|
||||||
|
- Any field that affects time registration, posting, approval, or invoicing is a business process field and must be read-only for agents
|
||||||
|
- Developer-managed fields (GitHub dev status, branch, comments) are the only writable surface
|
||||||
|
|
||||||
|
## Example Agent Instruction
|
||||||
|
|
||||||
|
```
|
||||||
|
Write only gitHubDevStatus and gitHubBranch on tasks.
|
||||||
|
Never write Status — it controls the invoicing workflow.
|
||||||
|
```
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
For each API page with write access, verify that fields controlling BC workflow transitions carry `Editable = false`. Review the agent instruction file to confirm it names the allowed writable fields explicitly and prohibits status fields.
|
||||||
|
|
@ -0,0 +1,32 @@
|
||||||
|
# CURABIS MCP: FlowFields on API Pages Must Be CalcFields'd
|
||||||
|
|
||||||
|
## Core Principle
|
||||||
|
|
||||||
|
FlowFields on API pages return empty or zero unless explicitly calculated. Every FlowField exposed on a `PageType = API` page must be called via `CalcFields` in the `OnAfterGetRecord` trigger — otherwise the OData response will contain empty values regardless of what the underlying data contains.
|
||||||
|
|
||||||
|
## Why This Happens
|
||||||
|
|
||||||
|
FlowFields are not stored in the database. Business Central only calculates them on demand. Regular pages trigger calculation automatically as part of the page rendering pipeline. API pages do not — the agent or external consumer receives the raw stored (empty) value.
|
||||||
|
|
||||||
|
## Requirements
|
||||||
|
|
||||||
|
- All FlowFields exposed in the `layout` section of an API page must be listed in a `CalcFields()` call in `OnAfterGetRecord`
|
||||||
|
- If multiple FlowFields are needed, they can be combined in a single call: `Rec.CalcFields(Field1, Field2)`
|
||||||
|
- Stored fields (non-FlowField) do not need CalcFields
|
||||||
|
|
||||||
|
## Example
|
||||||
|
|
||||||
|
```al
|
||||||
|
trigger OnAfterGetRecord()
|
||||||
|
begin
|
||||||
|
Rec.CalcFields("Elapsed time (Chargeable)", "Customer Name");
|
||||||
|
end;
|
||||||
|
```
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
When reviewing an API page, identify every field bound to a FlowField source expression. Confirm each appears in the `OnAfterGetRecord` CalcFields call. Any FlowField missing from CalcFields is a defect — it will silently return empty to the MCP consumer.
|
||||||
|
|
||||||
|
## Related Rule
|
||||||
|
|
||||||
|
CURABIS-MCP-002 — Stored derived fields must be recalculated in OnAfterGetRecord, not exposed directly.
|
||||||
|
|
@ -0,0 +1,40 @@
|
||||||
|
# CURABIS MCP: ODataKeyFields Must Be Editable for Create Operations
|
||||||
|
|
||||||
|
## Core Principle
|
||||||
|
|
||||||
|
Fields declared in `ODataKeyFields` that identify the record must not have `Editable = false` when the API page allows insert. If they are read-only, the OData API rejects them as unknown properties on POST — the create operation fails and the caller receives a `BadRequest` error.
|
||||||
|
|
||||||
|
## Why This Happens
|
||||||
|
|
||||||
|
`Editable = false` on a page field removes the field from the OData write schema entirely. When a consumer POSTs a new record and includes the key field in the body, BC cannot match it to any writable property and rejects the request.
|
||||||
|
|
||||||
|
## Pattern to Avoid
|
||||||
|
|
||||||
|
```al
|
||||||
|
// WRONG: Key field marked Editable = false — cannot be set on create
|
||||||
|
field(projectNo; Rec."Project No.")
|
||||||
|
{
|
||||||
|
Caption = 'projectNo';
|
||||||
|
Editable = false; // blocks insert via API
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## Correct Pattern
|
||||||
|
|
||||||
|
```al
|
||||||
|
// CORRECT: No Editable = false — BC controls mutability after insert via ODataKeyFields
|
||||||
|
field(projectNo; Rec."Project No.")
|
||||||
|
{
|
||||||
|
Caption = 'projectNo';
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## Requirements
|
||||||
|
|
||||||
|
- Fields listed in `ODataKeyFields` must not carry `Editable = false` on pages where `InsertAllowed = true`
|
||||||
|
- Fields that should be read-only after creation but writable on insert need no special property — OData key semantics handle immutability after the record exists
|
||||||
|
- Non-key fields that are genuinely read-only may still use `Editable = false`
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
On any API page with `InsertAllowed = true`, confirm that every field referenced in `ODataKeyFields` does not have `Editable = false` in its field definition. A create test via the OData endpoint is the definitive check.
|
||||||
|
|
@ -0,0 +1,42 @@
|
||||||
|
# CURABIS MCP: API Pages Must Use Least-Privilege Write Access
|
||||||
|
|
||||||
|
## Core Principle
|
||||||
|
|
||||||
|
A general-purpose API page that exposes many fields should not be widened to allow writes on a single additional field. Instead, create a dedicated minimal API page that exposes only the fields the consumer needs to read and write. This limits the blast radius of any agent or integration mistake.
|
||||||
|
|
||||||
|
## Why This Matters
|
||||||
|
|
||||||
|
An MCP agent operates with the permissions of its service identity, not an individual user. A page that allows writing to many fields gives the agent broad power that is hard to audit and easy to misuse. A dedicated page with one writable field makes the intent explicit and the surface area auditable.
|
||||||
|
|
||||||
|
## Pattern to Avoid
|
||||||
|
|
||||||
|
```al
|
||||||
|
// WRONG: General page widened with write access to one field
|
||||||
|
// Now the agent can accidentally (or intentionally) write to all other fields too
|
||||||
|
field(status; Rec.Status) { } // should be read-only
|
||||||
|
field(gitHubRepository; Rec."GitHub Repository") { } // the one field we want writable
|
||||||
|
field(estimatedHours; Rec."Estimated Hours") { } // should be read-only
|
||||||
|
```
|
||||||
|
|
||||||
|
## Correct Pattern
|
||||||
|
|
||||||
|
Create a separate, minimal API page:
|
||||||
|
|
||||||
|
```al
|
||||||
|
page 6102904 "CUR MCP Project Repository"
|
||||||
|
{
|
||||||
|
// Only two fields: the key and the one writable field
|
||||||
|
field(no; Rec."No.") { Editable = false; }
|
||||||
|
field(gitHubRepository; Rec."GitHub Repository") { }
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
## Requirements
|
||||||
|
|
||||||
|
- Each distinct write concern (e.g., setting a GitHub repo, updating a dev status) should have its own API page or be deliberately grouped only with closely related fields
|
||||||
|
- Read-only fields on write-enabled pages must carry `Editable = false`
|
||||||
|
- The page description must document which fields are writable and why
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
For each API page where `ModifyAllowed = true` (or default), list all fields without `Editable = false`. Confirm that every writable field is intentionally writable for the same consumer use case. If unrelated fields are writable on the same page, split the page.
|
||||||
|
|
@ -0,0 +1,44 @@
|
||||||
|
# CURABIS MCP: Stored Derived Fields Must Be Recalculated in OnAfterGetRecord
|
||||||
|
|
||||||
|
## Core Principle
|
||||||
|
|
||||||
|
A stored field whose value is derived from other fields via `OnValidate` triggers can be stale. When the source data changes (e.g., new time entries posted), the stored derived field is not updated automatically — it only recalculates when a specific trigger fires. Exposing such a field directly via an API page returns a value that may be hours, days, or weeks out of date.
|
||||||
|
|
||||||
|
## Pattern to Avoid
|
||||||
|
|
||||||
|
```al
|
||||||
|
// WRONG: Exposes the stored snapshot — may be stale
|
||||||
|
field(timeLeft; Rec."Time left") { }
|
||||||
|
```
|
||||||
|
|
||||||
|
`"Time left"` is recalculated only when `"Estimated time"` is validated. If new time entries are posted, the stored value does not update.
|
||||||
|
|
||||||
|
## Correct Pattern
|
||||||
|
|
||||||
|
Recalculate in `OnAfterGetRecord` using a page variable:
|
||||||
|
|
||||||
|
```al
|
||||||
|
trigger OnAfterGetRecord()
|
||||||
|
begin
|
||||||
|
Rec.CalcFields("Elapsed time (Chargeable)");
|
||||||
|
TimeLeftCalc := Rec."Estimated time" - Rec."Elapsed time (Chargeable)";
|
||||||
|
end;
|
||||||
|
|
||||||
|
var
|
||||||
|
TimeLeftCalc: Decimal;
|
||||||
|
|
||||||
|
// In layout:
|
||||||
|
field(timeLeft; TimeLeftCalc) { } // live value
|
||||||
|
field(elapsedTime; Rec."Elapsed time (Chargeable)") { } // source FlowField
|
||||||
|
```
|
||||||
|
|
||||||
|
## Requirements
|
||||||
|
|
||||||
|
- Identify stored fields whose value is computed from other fields via triggers
|
||||||
|
- Do not expose them directly in API pages
|
||||||
|
- Recalculate from the authoritative source (FlowField or live query) in `OnAfterGetRecord`
|
||||||
|
- Expose both the recalculated result and the source FlowField so the consumer can verify
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
Inspect the source table for any field with `FieldClass = Normal` whose value is set inside an `OnValidate` trigger on another field. If that field is exposed on an API page, verify it is recalculated in `OnAfterGetRecord` rather than read from `Rec` directly.
|
||||||
Loading…
Add table
Add a link
Reference in a new issue