fix(workflow): use technical names for field and action names#1625
Open
Scra3 wants to merge 6 commits into
Open
fix(workflow): use technical names for field and action names#1625Scra3 wants to merge 6 commits into
Scra3 wants to merge 6 commits into
Conversation
|
Coverage Impact Unable to calculate total coverage change because base branch coverage was not found. Modified Files with Diff Coverage (5) 🛟 Help
|
The orchestrator→executor preRecordedArgs referenced fields/relations/ actions by displayName — a mutable, non-unique label that breaks a pre-recorded step when an admin renames it. Reference them by their stable technical name instead: - fieldDisplayNames → fieldNames (read-record) - fieldDisplayName → fieldName (update-record) - actionDisplayName → actionName (trigger-action) - relationDisplayName → relationName (load-related-record) displayName is still persisted in the RunStore and still drives the AI tool enums/prompts — it is now always resolved from the live schema at execution time, never received as a wire reference. Fixes a latent bug where update-record and trigger-action persisted the inbound reference verbatim as displayName (resolveFieldName→resolveField, resolveActionName→resolveAction now derive it from the schema). Breaking contract change: the orchestrator must send technical names in preRecordedArgs (coordinated deployment). Excludes the renderingId schema-cache fix (dedicated ticket). fixes PRD-426 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Scra3
force-pushed
the
fix/prd-426-pre-recorded-args-technical-names
branch
5 times, most recently
from
f051b0d to
a35e26a
Compare
Scra3
commented
Jun 4, 2026
| ? { relationName: preRecordedArgs.relationDisplayName } | ||
| : await this.selectRelation(schema, step.prompt); | ||
| const target = this.buildTarget(schema, args.relationName, selectedRecordRef); | ||
| const recordedRelation = preRecordedArgs?.relationName; |
Member
Author
There was a problem hiding this comment.
preRecordedArgs will be very useful for run-to-build move, when the orchestrator wants to inject inputs instead to use ai.
Scra3
force-pushed
the
fix/prd-426-pre-recorded-args-technical-names
branch
from
a35e26a to
73ff4bc
Compare
…ayName collision) Pre-recorded references arrive as technical names, but resolution went through findField, which matches displayName first. A technical name could then resolve to a *different* field/action whose displayName collides with it — silently writing/triggering the wrong target. Add a dedicated findFieldByTechnicalName (exact fieldName, no displayName, no fuzzy) and an exact action lookup; the pre-recorded and front- confirmation paths use them, while findField stays the lenient resolver for AI-returned display names. Add a collision regression test. Also correct the CLAUDE.md error-attribution: unresolvable names throw *NotFoundError (read-record: NoResolvedFieldsError), not InvalidPreRecordedArgsError (which guards arg shape only). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Scra3
force-pushed
the
fix/prd-426-pre-recorded-args-technical-names
branch
from
73ff4bc to
4e70f12
Compare
Scra3
changed the title
nbouliol
reviewed
Jun 5, 2026
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
…cord The previous commit renamed the declaration but left three usages on the old `selected` identifier (resolvedFieldNames map, NoResolvedFieldsError, and formatFieldResults), which left it undefined and broke the build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the "AI selects by displayName; map it back to the technical name…" style comments added in this PR (selectFields, selectFieldAndValue, selectAction, selectRelation, findFieldByTechnicalName). The fuzzy resolveAiFieldName comment is kept (it documents non-obvious behavior). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
2 participants
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.
Context
fixes PRD-426
The orchestrator→executor
preRecordedArgsreferenced fields/relations/actions by their displayName — a user-customizable label that is non-unique and can change. Using it as a wire reference breaks a pre-recorded step the moment an admin renames it. This PR makes the wire reference the technical name (the stable, unique column/relation/action id) instead.Inspired by #1618 but scoped down: the
displayNameis still persisted and still drives the AI tool enums/prompts — it is just no longer used as a reference on the wire. Excludes therenderingIdschema-cache fix (dedicated ticket).Changes
Wire contract (
step-definition.tspreRecordedArgs):fieldDisplayNames→fieldNames(read-record)fieldDisplayName→fieldName(update-record)actionDisplayName→actionName(trigger-action)relationDisplayName→relationName(load-related-record)displayName is now always resolved from the live schema at execution time (still persisted in the RunStore, still shown to the AI). This also fixes a latent bug where
update-recordandtrigger-actionpersisted the inbound reference verbatim asdisplayName(resolveFieldName→resolveField,resolveActionName→resolveActionnow derive{ name, displayName }from the schema).Exact technical-name resolution (review follow-up): pre-recorded (and front-confirmation) references resolve via a dedicated
findFieldByTechnicalName/ exact action lookup —findField's displayName-first + fuzzy matching is reserved for AI-returned display names. Without this, a technical name could resolve to a different field whose displayName collides with it (silent wrong-target write). Covered by a regression test.The orchestrator (and the front, upstream) must send technical names in
preRecordedArgs. The executor's step-definition schema strips unknown keys, so an oldrelationDisplayNamewould becomeundefinedand silently fall back to the AI. This must ship in tandem with the orchestrator-side change.Verification
yarn workspace @forestadmin/workflow-executor build✅🤖 Generated with Claude Code
Note
Switch
preRecordedArgswire format to use technical names across all workflow step executorsfieldDisplayNames,fieldDisplayName,actionDisplayName,relationDisplayName) with technical name fields (fieldNames,fieldName,actionName,relationName) in thepreRecordedArgsschema for all step types.findFieldByTechnicalNameandresolveAiFieldNamehelpers toRecordStepExecutorfor exact resolution and AI display-name-to-technical-name mapping.resolveAiFieldNamebefore use; pre-recorded args now require exact technical names and throw typed errors (e.g.FieldNotFoundError,ActionNotFoundError) on mismatch.preRecordedArgswill fail schema validation.Changes since #1625 opened
ReadRecordStepExecutor.doExecuteto return structured field objects [64ce331]ReadRecordStepExecutor.doExecutemethod [64ce331]ReadRecordStepExecutor.doExecutemethod withinworkflow-executorpackage [91fddd1]RecordStepExecutor.findFieldByTechnicalNameto acceptundefinedas a parameter and returnundefinedimmediately when no name is provided, and updatedUpdateRecordStepExecutor.coerceOverrideto passundefineddirectly instead of defaulting to an empty string [53e7320]📊 Macroscope summarized 64ce331. 1 file reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.