Files
wwtools/graccesscli/analysis/ide-edit-investigation/REPORT.md
T
Joseph Doherty c52d8d0171 graccesscli: correct script-edit docs to TN-537 truth (writes DO persist)
I was wrong. AVEVA Tech Note 537 ("Creating an Application Object Script
Using GRAccess", April 2008) documents the supported pattern:
ConfigurableAttributes[<script>.<field>].SetValue(MxValue) inside a
CheckOut/Save/CheckIn cycle. graccesscli's existing
FindAttributeForMutation already follows this — writes to MxCategoryPackageOnly_Lockable
script-text fields persist correctly.

The earlier "writeback gap" diagnosis was a phantom caused by a reader-side
issue. `object attribute value get` against a script body returns
"Supported: False / Attribute value is not exposed" because
MxValueDetails uses a case-sensitive `ReadProperty(attr, "Value")` lookup
plus an accessor probe (GetBoolean -> GetInteger -> GetFloat -> GetDouble
-> GetString) that can fall through silently for some MxValue shapes. The
COM-side property is exposed as `value` (lowercase), readable as
`attr.value.GetString()` -- which the live probe at
`analysis/ide-edit-investigation/probe_setvalue/` does and confirms the
post-write content matches the marker exactly.

Live verification on $TestMachine.UpdateTestChangingInt.DeclarationsText
and $DelmiaReceiver.ProcessRecipe.{ExecuteText,DeclarationsText}:

  === verdict ===
    marker landed on same-proxy   ConfigurableAttributes: True
    marker landed on same-proxy   Attributes            : True
    marker landed on fresh-proxy  ConfigurableAttributes: True
    marker landed on fresh-proxy  Attributes            : True

The probe also confirmed that two earlier graccesscli `object scripts set`
invocations (which I had wrongly believed failed) had persisted -- the
marker text I wrote previously was still on disk in
ProcessRecipe.{ExecuteText,DeclarationsText} when read directly via
attr.value.GetString(). The probe restored both fields to their original
values.

This commit:

- Updates the misleading [Command(...)] / [CommandOption(...)]
  descriptions in GRAccessSurfaceCommands.cs back to honest versions
  citing TN-537.
- Restores the --file-using examples for `object scripts set` and
  `object scripts create` across script-editing.md, llm-integration.md,
  usage.md, and zb-testmachine.md.
- Removes the test that asserted the (wrong) EnsureMutableViaSetValue
  guard. Re-aims ScriptCommandDescriptions_… at the corrected wording.
- Removes two leftover EnsureMutableViaSetValue calls in the trigger-period
  / trigger-type write paths (both targeted MxCategoryWriteable_C_Lockable
  attributes; would never have fired even if the helper still existed).
- Adds analysis/ide-edit-investigation/REPORT.md (replacing the earlier
  wrong report) plus the probe sources under probe_setvalue/.

The MxValueDetails reader gap (case-sensitive ReadProperty + accessor
probe) is a real follow-up: `object attribute value get` should
case-insensitively read `value` and try GetString first when the
underlying MxValue.DataType is MxString. Out of scope here -- that's a
separate, smaller fix.

Test count delta: 67 -> 66 (-2 wrong tests, +1 corrected description test).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-05 21:33:51 -04:00

83 lines
6.6 KiB
Markdown

# How the IDE edits scripts — investigation findings (corrected)
**Status:** my original report (and the safety-check + docs note that grew out of it) was **wrong**. AVEVA Tech Note 537 ("Creating an Application Object Script Using GRAccess", April 2008) documents the supported pattern, and a live round-trip probe confirmed it works on this Galaxy. graccesscli's existing `FindAttributeForMutation` already follows this pattern correctly; the apparent "writeback gap" was a reader-side gap in `MxValueDetails`.
This document supersedes the earlier "package-only no-op" claim. The reverts on `master`: commits `4e242ca` (safety check) and `e4e5425` (docs note) plus the corrective commit on top.
## What's actually true
Per **AVEVA Tech Note 537** (page 6-7):
> First we add a new script into the list of scripts. Then we save the object. Saving the object generates all the attributes required to configure the script the conventional way. […] Configurable attributes of the ScriptExtension primitive: ExecuteText, AliasReferences, Aliases, TriggerType, DataChangeDeadband, Expression, DeclarationsText, StartupText, ShutdownText, OnScanText, OffScanText, ExecutionError.Alarmed, TriggerPeriod, ScriptExecutionGroup, ScriptOrder, RunsAsync, State.Historized, ExecuteTimeout.Limit, TriggerOnQualityChange.
The supported edit sequence is:
```csharp
// 1. Acquire the object.
var galaxy = galaxies[GalaxyName];
galaxy.Login(user, pwd);
var obj = galaxy.QueryObjectsByName(EgObjectIsTemplateOrInstance.gObjectIsTemplate, ref names)[1];
// 2. Mutation lifecycle.
obj.CheckOut();
// 3. Add the primitive (only when creating a new script).
obj.AddExtensionPrimitive("ScriptExtension", scriptName, true);
obj.Save(); // generates the script's child attributes
// 4. Write text via ConfigurableAttributes — NOT Attributes.
IAttribute body = obj.ConfigurableAttributes[scriptName + ".ExecuteText"];
var v = new MxValueClass();
v.PutString("// new script body...");
body.SetValue(v);
// (repeat for DeclarationsText, TriggerType, Expression, etc.)
// 5. Persist + release.
obj.Save();
obj.CheckIn("");
```
graccesscli's `FindAttributeForMutation` (line 988-995) already prefers `ConfigurableAttributes` first and falls back to `Attributes`. `ObjectScriptSet` in the dispatcher writes via that helper. So `object scripts set --field <name> --file <path>` follows the TN-537 pattern.
## Live verification
The probe at `analysis/ide-edit-investigation/probe_setvalue/` exercises the TN-537 sequence directly against `$TestMachine.UpdateTestChangingInt.DeclarationsText` and against `$DelmiaReceiver.ProcessRecipe.{ExecuteText,DeclarationsText}` on the local ZB galaxy. Result of the four-way verify after a marker write:
```
=== verdict ===
marker landed on same-proxy ConfigurableAttributes: True
marker landed on same-proxy Attributes : True
marker landed on fresh-proxy ConfigurableAttributes: True
marker landed on fresh-proxy Attributes : True
```
Same probe also confirmed two earlier `graccesscli object scripts set` invocations had persisted (the `// roundtrip-test marker (graccesscli scripts set --field DeclarationsText)` text was still in `ProcessRecipe.DeclarationsText` when re-read directly via `attr.value.GetString()`).
## Why my earlier diagnosis went wrong
My earlier round-trip test wrote with graccesscli's `object scripts set`, then read back with `object attribute value get` — and the read returned `Supported: False, Value: None, Unavailable: "Attribute value is not exposed by this GRAccess attribute."` I read this as "the write didn't persist." It actually means **the reader couldn't surface the value**, while the write *had* persisted.
The reader gap is in `GRAccessCommandDispatcher.MxValueDetails` (line 1382-1417) plus `AttributeValueDetails` (line 1340-1380):
```csharp
var direct = MxValueDetails(ReadProperty(attr, "Value"));
```
`ReadProperty(attr, "Value")` uses `Type.InvokeMember(name, BindingFlags.GetProperty, ...)` which is **case-sensitive** by default. The COM-side property is exposed as `value` (lowercase) on `IAttribute` — what the probe successfully reads via `attr.value`. The capital-V `"Value"` lookup may either return `null` or hit a different sibling, and `MxValueDetails(null)` short-circuits to "not exposed." Even when `ReadProperty` does return the MxValue object, `MxValueDetails`'s scalar accessor probe (`GetBoolean``GetInteger``GetFloat``GetDouble``GetString`) can fall through silently for some types.
This is a real gap in `attribute value get` that should be fixed separately — but it's a *reader* problem, not a write-side issue. The script body content does land on disk, and `object scripts get --llm-json` (which uses the package-export fallback) reflects it correctly.
## What changed in this correction
- **Reverted** `bd95ace` (the safety check `EnsureMutableViaSetValue`) and `c12fbc5` (the wrong docs note about a "package-only writeback boundary").
- **Removed** two leftover `EnsureMutableViaSetValue` call sites the user had added in `87c0124` for the trigger-period / trigger-type write paths (both call sites also targeted `MxCategoryWriteable_C_Lockable` attributes, so they would never have fired even if the helper still existed — removing keeps the dispatcher consistent with the truth).
- **Updated** the misleading `[Command(...)]` / `[CommandOption(...)]` descriptions in `GRAccessSurfaceCommands.cs` back to honest versions citing TN-537.
- **Restored** the `--file`-using examples for `object scripts set` and `object scripts create` that `87c0124` removed across `script-editing.md`, `llm-integration.md`, `usage.md`, `zb-testmachine.md`.
- **Updated** test assertions accordingly: removed `DispatcherScriptSettings_GuardsPackageOnlyNoOpsBeforeSetValue` (asserted the wrong helper) and re-aimed `ScriptCommandDescriptions_…` at the corrected wording.
- **Rewrote** `analysis/ide-edit-investigation/REPORT.md` (this file) to document the truth.
## Open follow-ups (≤2)
1. **`MxValueDetails` reader gap.** `object attribute value get` against ScriptExtension text fields and similar `IAttribute` shapes returns `Supported: False` despite the value being set. Likely fix: case-insensitive `ReadProperty` lookup *and* try `GetString` first when the underlying `MxValue.DataType` is `MxString`. Out of scope for this corrective commit.
2. **The decompiled `IConfigurationAccess2` / `XxGalaxyPackageServer` analysis** under `cac/`, `appcfg/`, `scriptpkg/` is still useful as an *internal IDE map* — but it's not the *required* CLI write path, just an alternative one the IDE happens to use. Keep the artifacts; they document the IDE-internal channel honestly.