diff --git a/.github/skills/acumatica-silent-success-traps/SKILL.md b/.github/skills/acumatica-silent-success-traps/SKILL.md new file mode 100644 index 0000000..fe4b055 --- /dev/null +++ b/.github/skills/acumatica-silent-success-traps/SKILL.md @@ -0,0 +1,8 @@ +--- +name: acumatica-silent-success-traps +description: Detect and prevent Acumatica ERP API calls that report success while doing nothing, or return an empty result instead of an error. Use when issuing a contract-based REST write and reporting its outcome, when concluding that a query returned no matching records, when parsing a 2xx response body, when choosing between a Generic Inquiry's plain and _WithParameters forms, or when diagnosing "the API returned success but nothing changed", "the field did not save", or "the inquiry is empty but the UI shows rows". Applies when writing or reviewing code that issues these calls; not for upgrading endpoint versions, migrating authentication, or repairing legacy integration clients. +--- + +# Acumatica Silent-Success Traps + +Read and follow the canonical skill at [DEV/skills/acumatica-silent-success-traps/SKILL.md](../../../DEV/skills/acumatica-silent-success-traps/SKILL.md) in full before acting. Treat `DEV/skills/acumatica-silent-success-traps/` as the skill root and resolve all relative reference paths from that directory. diff --git a/DEV/.claude-plugin/plugin.json b/DEV/.claude-plugin/plugin.json index 58ffdaa..23e897c 100644 --- a/DEV/.claude-plugin/plugin.json +++ b/DEV/.claude-plugin/plugin.json @@ -4,5 +4,5 @@ "description": "Skills and artifacts for Acumatica ERP Developers", "author": { "name": "Acumatica" }, "license": "GPL-3.0-only", - "version": "1.0.2" + "version": "1.1.0" } diff --git a/DEV/.codex-plugin/plugin.json b/DEV/.codex-plugin/plugin.json index 314ddc1..66a2365 100644 --- a/DEV/.codex-plugin/plugin.json +++ b/DEV/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "dev-toolkit", - "version": "1.0.2", + "version": "1.1.0", "description": "Skills and artifacts for Acumatica ERP developers", "author": { "name": "Acumatica" diff --git a/DEV/.cursor-plugin/plugin.json b/DEV/.cursor-plugin/plugin.json index 868ba78..3239822 100644 --- a/DEV/.cursor-plugin/plugin.json +++ b/DEV/.cursor-plugin/plugin.json @@ -5,5 +5,5 @@ "description": "Skills and artifacts for Acumatica ERP Developers", "keywords": ["csharp", "dac", "customization", "upgrade", "publishing", "integration", "rest", "oauth", "modern-ui", "aurelia", "typescript", "custom-controls"], "license": "GPL-3.0-only", - "version": "1.0.2" + "version": "1.1.0" } diff --git a/DEV/README.md b/DEV/README.md index c285c53..895bfb1 100644 --- a/DEV/README.md +++ b/DEV/README.md @@ -7,4 +7,5 @@ Plugin with skills and artifacts for Acumatica ERP Developers. - **[acumatica-customization-update](skills/acumatica-customization-update/)** — Edit, update, upgrade, validate, and publish Acumatica customization projects and packages, including packaged DACs, graph extensions, screens, Extension Library rebuilds, and version-specific merges - **[acumatica-integration-diagnostics](skills/acumatica-integration-diagnostics/)** — Modernize, repair, and validate Acumatica REST, SOAP, OData, middleware, and diagnostic integrations, including endpoint upgrades and OAuth 2.0 authentication - **[acumatica-modern-ui-control-builder](skills/acumatica-modern-ui-control-builder/)** — Design, implement, review, and troubleshoot reusable Acumatica Modern UI controls with Aurelia, TypeScript, HTML, SCSS, screen extensions, and backend field bindings +- **[acumatica-silent-success-traps](skills/acumatica-silent-success-traps/)** — Detect and prevent Acumatica API calls that report success while doing nothing, or return an empty result instead of an error: discarded writes, body-less 200s, calculated-column filters, parameterized inquiries queried by their base name, and inquiries whose name makes them unreachable diff --git a/DEV/skills/acumatica-silent-success-traps/SKILL.md b/DEV/skills/acumatica-silent-success-traps/SKILL.md new file mode 100644 index 0000000..4b43e3e --- /dev/null +++ b/DEV/skills/acumatica-silent-success-traps/SKILL.md @@ -0,0 +1,93 @@ +--- +name: acumatica-silent-success-traps +description: Detect and prevent Acumatica ERP API calls that report success while doing nothing, or return an empty result instead of an error. Use when issuing a contract-based REST write and reporting its outcome, when concluding that a query returned no matching records, when parsing a 2xx response body, when choosing between a Generic Inquiry's plain and _WithParameters forms, or when diagnosing "the API returned success but nothing changed", "the field did not save", or "the inquiry is empty but the UI shows rows". Applies when writing or reviewing code that issues these calls; not for upgrading endpoint versions, migrating authentication, or repairing legacy integration clients. +metadata: + version: 1.0.0 +--- + +# Acumatica Silent-Success Traps + +## The rule + +For the Acumatica API, **a 2xx is not evidence of effect, and an empty result is not +evidence of absence.** A setting that reports itself as enabled is also not evidence that +the thing it enables is reachable. + +Several distinct Acumatica behaviors return success and do nothing, or return empty rather +than an error. There is no exception to catch and no error code to branch on, so an +integration reports success to its caller and an agent reports a confident wrong answer. +Verify by reading back, not by reading the status code. + +Each behavior below was observed on **Acumatica 2025 R2 SaaS**. These are platform +behaviors rather than tenant configuration, so they are expected to hold broadly — but +confirm any single one against the release you target before depending on it, since none +of them is documented as a contract. + +## Before you report a write as done + +Resolve it to a verified 2xx **and** a read-back that confirms the specific fields you +sent. Two causes discard a write with an identical HTTP 200: + +- **The principal lacks edit rights on the target form.** The PUT succeeds at the HTTP + layer and persists nothing. Confirm per-form and per-entity grants for the account you + authenticate as — a read-only role produces this exact result. +- **The field is not writable through the endpoint.** A DAC field the endpoint or screen + does not expose as settable is accepted and dropped. "The API accepted it" never proves + a field saved. + +Because both look the same, the check is the same: re-GET the record and compare the +fields you intended to change. Do not compare whole payloads — unrelated server-side +defaulting will produce false differences. + +**Never escalate an unverified write to the user as done.** If you could not read it back, +say so and name what is unconfirmed. + +## Before you report that no records matched + +Distinguish three cases that all look like "nothing found": + +- **A body-less 200.** Standard OData returns `{"value":[]}` for a genuinely empty result, + so a response with *no body* is anomalous — the query did not really run. Parsing it + throws a raw JSON parser error; do not surface that as the explanation, and do not treat + it as zero rows. The two are a byte apart and mean opposite things: + + ``` + 200 OK Content-Length: 0 <- query never ran; cause is below. NOT zero rows. + 200 OK {"value":[]} <- genuine empty result set. Zero rows. + ``` + + Branch on whether a body arrived, before parsing it. +- **A `$filter` referencing a calculated column.** Acumatica documents that fields + computed by a formula cannot be sorted or filtered, but reports the violation as an + empty-body 200 rather than a 400. Filter only on stored columns — keys, dates, statuses, + codes — and apply conditions on calculated columns to the returned rows yourself. A + column whose Results Grid row is a `=…` expression is calculated. +- **A parameterized Generic Inquiry queried by its base name.** Its declared parameters + stay NULL, the WHERE clause fails, and you get 0 rows with a 200 and no error. + Parameters bind only through the separate `_WithParameters` FunctionImport, addressed by + that distinct name with the arguments in parentheses. The `$metadata` document lists an + `EntitySet` for the plain form and a `FunctionImport` with the `_WithParameters` postfix + for the parameterized one — which is also the reliable way to detect that an inquiry + takes parameters at all. + +For an unattended consumer, and for any agent that cannot inspect the design, prefer a +parameter-free copy of the inquiry, or read the underlying contract entity. + +## Before you trust that an inquiry is exposed + +An inquiry whose **name contains a URL path separator** cannot be addressed over OData. It +is accepted by the *Expose via OData* setting and appears in `$metadata`, but requests to +it 404 and it never appears in the service document, so it is undiscoverable as well as +unqueryable. Nothing flags the name as unusable — the inquiry looks published and simply +never works. If an inquiry is configured as exposed but absent from the service document, +check its name before investigating permissions. + +## Reporting + +- State separately what you **verified** and what you **inferred**. A read-back is + verification; a 2xx is not. +- When a write is unconfirmed, say which record and which fields, and what would confirm it. +- Never report "no records exist" from a response that could be any of the empty cases + above. Say the query did not return usable results and why you cannot distinguish. +- When a filter was rejected in one of these silent ways, say the query **never executed** + — otherwise a rejected filter gets reported to the user as a real, empty answer. diff --git a/DEV/skills/acumatica-silent-success-traps/agents/openai.yaml b/DEV/skills/acumatica-silent-success-traps/agents/openai.yaml new file mode 100644 index 0000000..abec3b1 --- /dev/null +++ b/DEV/skills/acumatica-silent-success-traps/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Acumatica Silent-Success Traps" + short_description: "Catch Acumatica calls that report success but do nothing" + default_prompt: "Use $acumatica-silent-success-traps to verify that an Acumatica write or query actually did what its response claims." diff --git a/Verification/acumatica-silent-success-traps-review.md b/Verification/acumatica-silent-success-traps-review.md new file mode 100644 index 0000000..185a073 --- /dev/null +++ b/Verification/acumatica-silent-success-traps-review.md @@ -0,0 +1,71 @@ +# Review: `acumatica-silent-success-traps` Skill + +## Context + +Reviewed `DEV/skills/acumatica-silent-success-traps` on 2026-09-09, before its first submission, and re-run after the fixes recorded below. Scope was the two files in the skill folder — `SKILL.md` (87 lines, 795 words) and `agents/openai.yaml` — plus the `.github/skills/acumatica-silent-success-traps/SKILL.md` discovery adapter. The skill bundles no `references/`, `scripts/`, or `assets/`, so the cross-reference and script rules were checked and recorded as inapplicable rather than passing vacuously; it references no files on disk and no network URLs, so there was nothing to resolve. Verification covered folder and frontmatter validity, discovery metadata, description and instruction quality, workflow completeness, size and progressive disclosure, cross-reference integrity, and platform-neutral wording against Skill Authoring Best Practices, Skills Docs, the Complete Guide, and Skill Creator guidance. Frontmatter thresholds, the description truncation point, platform-specific tool names, and customer-identifier leakage were measured programmatically rather than by eye. + +Three P2 findings and three P3 findings were recorded. All three P2 items were fixed before submission and are closed below; the three P3 items remain open, one of them deliberately. + +## Verification Sources + +| Abbreviation | Source | +|---|---| +| **[BP]** | [Skill Authoring Best Practices](https://code.claude.com/docs/en/best-practices) | +| **[SD]** | [Claude Code Skills Docs](https://code.claude.com/docs/en/skills) | +| **[CG]** | [The Complete Guide to Building Skills for Claude](https://resources.anthropic.com/hubfs/The-Complete-Guide-to-Building-Skill-for-Claude.pdf) (official Anthropic guide) | +| **[SC]** | Skill Creator skill instructions | + +--- + +## Findings + +### P1 — High Severity (likely to cause functional problems) + +No P1 findings. + +The directory is kebab-case, `SKILL.md` has exact case, the frontmatter `name` (30 characters) is kebab-case and matches the directory, both required fields are present, the description is 680 characters with no angle brackets, the only custom field (`version`) is nested under `metadata`, and the body names no platform-specific tool from either the Claude Code or Cursor list — so the instructions are platform-neutral. A scan for customer identifiers (instance hostnames, branch and warehouse codes, inquiry names, document numbers) returned clean. + +### P2 — Medium Severity (reduces quality or violates best practices) + +No open P2 findings. + +*Closed before submission:* three items. + +1. The description carried no boundary against `acumatica-integration-diagnostics`, which covers Acumatica REST, OData and OAuth work for the same audience. With no negative trigger, both skills could load for one request, and a reviewer had no in-artifact answer to why this is not part of the existing skill. The description now states that it applies when writing or reviewing code that issues these calls, and not for upgrading endpoint versions, migrating authentication, or repairing legacy integration clients — phrased by behavior rather than by naming the sibling, so it does not go stale if the plugin is restructured. **[CG]** "Includes negative triggers if the skill could overlap with related skills" + +2. The skill's central discrimination is between a body-less `200` and a `200` carrying `{"value":[]}`, and it described that difference in prose without showing it. SKILL.md now places the two responses side by side with what each means, followed by the resulting instruction — branch on whether a body arrived before parsing it. **[CG]** "Examples are provided for key scenarios" + +3. Every behavior described is version-dependent ERP behavior, stated without a release scope. Naming no release leaves a reader unable to tell whether a claim still holds after an upgrade; naming one risks time-sensitive content. SKILL.md now scopes the set once, beside the rule it qualifies, and says explicitly that none of the behaviors is documented as a contract — which is why the scope matters rather than being boilerplate. **[BP]** "No time-sensitive information ... unless in a collapsible 'old patterns' section" + +### P3 — Low Severity (style/polish issues) + +#### 1. The closing section partially restates the three sections above it + +- **Location**: SKILL.md:78–87 (Reporting) +- **Problem**: Two of its four bullets echo guidance already given in place — the "no records exist" bullet repeats the empty-result section, and the unverified-write bullet repeats the write section. The duplication costs context without adding instruction, and two copies of one rule can drift apart in later edits. Left open because the section's purpose is different from the sections it echoes: the others say how to *detect* each trap, this one says how to *report* the result, and a reader who reaches only that section still gets the constraint. +- **Reference**: **[BP]** "No redundant sections that repeat information stated elsewhere in the skill", **[SC]** "Each piece of content justifies its token cost" + +#### 2. Two concepts each appear under two names + +- **Location**: SKILL.md, throughout +- **Problem**: "Generic Inquiry" and "inquiry" are used interchangeably, as are "principal" and "account" for the authenticating identity. Synonym drift makes the text harder to scan and weakens keyword matching for a reader searching the skill for a specific concept. +- **Reference**: **[BP]** "Consistent terminology throughout — one term per concept, no synonyms" + +#### 3. The description opens in the imperative rather than describing purpose + +- **Location**: SKILL.md:3 +- **Problem**: "Detect and prevent ..." reads as an instruction to the model rather than a statement of what the skill is, and the description is metadata consumed during selection rather than execution. Left open **deliberately**: all three sibling skills in this plugin open the same way ("Modernize, repair, and validate ...", "Edit, update, upgrade ...", "Design, implement, review ..."), so changing it would make this skill the only outlier in the plugin. Flag it if the convention is meant to change. +- **Reference**: **[BP]** "Does not contain behavioral/runtime instructions (those belong in the body)" + +--- + +## Summary Table + +| # | Severity | Finding | Source | +|---|----------|---------|--------| +| 1 | P2 (closed) | Description lacked a negative trigger separating it from the overlapping sibling skill | [CG] | +| 2 | P2 (closed) | No paired example of the body-less 200 vs `{"value":[]}` distinction | [CG] | +| 3 | P2 (closed) | Version-dependent platform claims stated without a release scope | [BP] | +| 4 | P3 (open) | Closing "Reporting" section partially restates earlier sections | [BP], [SC] | +| 5 | P3 (open) | "Generic Inquiry"/"inquiry" and "principal"/"account" used interchangeably | [BP] | +| 6 | P3 (open, deliberate) | Description opens in the imperative, matching the plugin's three sibling skills | [BP] |