Skip to content

feat(contract-toolkit): express Automation Engine activity nodes in the manifest DAG [FND-4754] - #4793

Merged
cmgrote merged 2 commits into
mainfrom
christophergrote/fnd-4754
Nov 6, 2026
Merged

cmgrote merged 2 commits into
mainfrom
christophergrote/fnd-4754

Conversation

@cmgrote

@cmgrote cmgrote commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

Changelog

  • New AEActivityNode family for extraNodes. It calls an Automation Engine activity directly instead of starting a child workflow through execute_workflow.
    • FetchEntitiesNode (fetch_entities) and MergeNode (merge) have type-checked inputs that mirror AE's FetchEntitiesInput / MergeInput. Each must have exactly one SQL source, and extraInput / extraInputs cover fields the toolkit does not model.
    • ActivityNode is the generic form: any activityName with free-form inputs.
  • These nodes render node_type: "activity", the activity's own inputs (no workflow_type / task_queue / args envelope, no stamped app_name arg), and the same depends_on / error_handling as DAGNode. heartbeatTimeoutSeconds is allowed. errorHandling defaults to null, so AE's per-activity timeouts apply.
  • extraNodes is now typed Mapping<String, DAGNode|AEActivityNode>. Existing DAGNode output is unchanged: every example regenerates byte-identical.
  • New example examples/ae-activities/. Docs updated in README.md, docs/reference.md, AGENTS.md and CLAUDE.md.

Additional context (e.g. screenshots, logs, links)

  • Linear: FND-4754. This unblocks FND-4752, which moves an app with hand-authored fetch → merge manifests onto App.pkl.
  • I rebuilt that app's manifest DAG with these nodes and diffed it against the committed JSON. Every fetch_entities / merge node matched exactly. The execute_workflow nodes differ only in existing toolkit behavior, which this PR does not change: the toolkit omits node_type (AE defaults it to activity), omits app_id / argo_workflow_slug (AE does not read them), and stamps args.app_name (CNCT-93).
  • I checked the input field shapes against atlan-automation-engine-app main.
  • No SDK runtime change is needed. Manifest token resolution walks the DAG generically.

Checklist

  • Additional tests added (contract-toolkit/tests/ae_activity_nodes_test.pkl)
  • All CI checks passed
  • Relevant documentation updated

Local runs: pkl test tests/*.pkl passed (614 tests), as did check-invariants.sh, test-sdk-import.py, check-ruff-stable.py and pre-commit.


Copyleft License Compliance

  • Have you used any code that is subject to a Copyleft license (e.g., GPL, AGPL, LGPL)?
  • If yes, have you modified the code in the context of this project? please share additional details.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AbKqGYd63fn71uWqDDx8yF

…he manifest DAG [FND-4754]

DAGNode always renders activity_name = "execute_workflow", so a contract
could not call an AE activity such as fetch_entities or merge directly.
Apps that do (csa-uber) had to keep hand-authored manifests.

Add AEActivityNode, with typed FetchEntitiesNode and MergeNode plus a
generic ActivityNode, accepted in extraNodes alongside DAGNode. They
render node_type "activity", the activity's own inputs, and the shared
depends_on / error_handling rendering (heartbeat allowed). errorHandling
defaults to null so AE's per-activity timeouts apply. Existing DAGNode
output is byte-identical.

Claude-Session: https://claude.ai/code/session_01AbKqGYd63fn71uWqDDx8yF
@linear

linear Bot commented Nov 6, 2026

Copy link
Copy Markdown

FND-4754

@cmgrote

cmgrote commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

/lens

@atlan-app-fleet

atlan-app-fleet Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

lens summary · updated after round 2 (re-review · only commits since 35506cd)

✅ Ready to merge — every finding is resolved

Open findings: 🔴 0 critical · 🟠 0 high · 🟡 0 medium · ⚪ 0 low (nit)

lens reads this PR as — App.extraNodes only accepted DAGNode, whose renderer always produces an execute_workflow child-workflow envelope, so it could not express direct Automation Engine activity nodes such as fetch_entities and merge. How: The PR adds an AEActivityNode family for direct activity calls, with typed FetchEntities/Merge inputs and a generic free-form variant. It renders each activity's own inputs and reuses the existing dependency and error-handling renderer, while retaining the existing DAGNode rendering path.

Approach check — sound.

✔️ Resolved (1)
id severity where finding found resolved how
F-a275ad 🟡 medium contract-toolkit/examples/ae-activities/app.pkl:48 Example leaves required SQL sources empty round 1 round 2 its code was removed or rewritten
🕘 Round history (2)
round run commits new resolved blocking open cost log
1 first review bad9972..35506cd 1 0 0 $0.028 run
2 re-review · only commits since 35506cd 35506cd..d7484de 0 1 0 $0.003 run
1 file(s) not reviewed
  • contract-toolkit/examples/ae-activities/app/generated/manifest.json — excluded by pattern

round 2 · $0.030 of $1.00 · 24 model calls · 0 failed requests · 76% prompt cache hits · gpt-6-luna

…s [FND-4754]

The example set every sqlInputFile and sql to "", and nothing in it binds
a submission-time value, so a deploy of the example would fail in AE.
Use an object-store key per fetch and inline merge SQL instead. The
empty-string allowance (for apps that fill the source at submission) is
now covered by an inline unit test rather than the example.

Addresses lens finding F-a275ad.

Claude-Session: https://claude.ai/code/session_01AbKqGYd63fn71uWqDDx8yF
@cmgrote

cmgrote commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

/lens

@atlan-app-fleet

Copy link
Copy Markdown
Contributor

✅ lens · round 2 · re-review · only commits since 35506cd — ready to merge — every finding resolved · $0.03

Open findings: 🔴 0 critical · 🟠 0 high · 🟡 0 medium · ⚪ 0 low (nit)
Resolved this round: 1

Approach check — ✅ sound

  • Problem: App.extraNodes only accepted DAGNode, whose renderer always produces an execute_workflow child-workflow envelope, so it could not express direct Automation Engine activity nodes such as fetch_entities and merge.
  • How the PR solves it: The PR adds an AEActivityNode family for direct activity calls, with typed FetchEntities/Merge inputs and a generic free-form variant. It renders each activity's own inputs and reuses the existing dependency and error-handling renderer, while retaining the existing DAGNode rendering path.

$0.030 of $1.00 · 24 model calls · 0 failed requests

Full summary · Run log

@atlan-ci atlan-ci left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lens: ready to merge — every finding at every level is resolved (round 2).

@cmgrote
cmgrote added this pull request to the merge queue Nov 6, 2026
Merged via the queue into main with commit 468e2b1 Nov 6, 2026
34 checks passed
@cmgrote
cmgrote deleted the christophergrote/fnd-4754 branch November 6, 2026 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants