feat: detect features in CLI and pass to /deploy endpoint #5653
Conversation
| } catch (err) { | ||
| return Err(new Error('failed_to_detect_features', { cause: err })); | ||
| } | ||
| } |
There was a problem hiding this comment.
I went for traversing the code again instead of piggy backing on the compilation step because it would have required to entirely refactor how compilation outcome is handled and returned.
Since this is pure ast traversing, the performance impact is low.
There was a problem hiding this comment.
Review suggests a reliability fix to fail fast when feature detection fails to prevent misrouted syncs.
Status: Changes Suggested | Risk: Medium
Issues Identified & Suggestions
- Fail fast when feature detection fails to avoid silent misrouting:
packages/cli/lib/zeroYaml/deploy.ts
Review Details
📁 18 files reviewed | 💬 1 comments
Instruction Files
├── .claude/
│ ├── agents/
│ │ └── nango-docs-migrator.md
│ └── skills/
│ ├── agent-builder-skill/
│ │ ├── EXAMPLES.md
│ │ └── SKILL.md
│ ├── creating-integration-docs/
│ │ └── SKILL.md
│ └── creating-skills-skill/
│ └── SKILL.md
├── AGENTS.md
└── GEMINI.md
👍 / 👎 individual comments to help improve reviews for you
| @@ -203,6 +204,15 @@ async function createPackage({ | |||
| return Err(new Error(`No script files found for "${sync.name}"`)); | |||
| } | |||
|
|
|||
| const entryPoint = path.join(fullPath, providerConfigKey, 'syncs', `${sync.name}.ts`); | |||
| const features = await detectFeatures({ entryPoint }); | |||
| if (features.isErr()) { | |||
There was a problem hiding this comment.
[Reliability] When feature detection fails, the deploy continues and silently sends features: []. That will misclassify checkpointed syncs and route them incorrectly. Please return an error here (and in the action block below) so the deploy fails fast, or explicitly surface this as a hard error to the caller.
Context for Agents
When feature detection fails, the deploy continues and silently sends `features: []`. That will misclassify checkpointed syncs and route them incorrectly. Please return an error here (and in the action block below) so the deploy fails fast, or explicitly surface this as a hard error to the caller.
File: packages/cli/lib/zeroYaml/deploy.ts
Line: 209There was a problem hiding this comment.
on purpose for now. features will only be used internally and false negative are acceptable for now. It will result in syncs not being candidate for lambdas which would is already the fallback option
3f06509 to
f46a53e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| const entryPoint = path.join(fullPath, providerConfigKey, 'actions', `${action.name}.ts`); | ||
| const features = await detectFeatures({ entryPoint }); | ||
| if (features.isErr()) { | ||
| console.error( | ||
| chalk.red(`Error detecting features for "${action.name}"`), | ||
| features.error instanceof Error ? features.error.message : features.error | ||
| ); | ||
| } |
There was a problem hiding this comment.
This could also potentially be done in parseSync/parseAction in packages/cli/lib/zeroYaml/definitions.ts. "Features" would have to be added to the ParseNangoSync and ParseNangoAction types, and you would access them here with action.features.
However, it would also make it into nango.json, which might not be intentional. So no strong opinion tbh, just pointing it out
There was a problem hiding this comment.
Actually, having it in nango.json also means having it in flows.zero.yaml which we use to deploy preBuilt templates. So this might be a requirement.
There was a problem hiding this comment.
good idea. let me look into it
There was a problem hiding this comment.
moved the features detection to parsing 7bf3cbf
For now not, built-in templates aren't supported
To be used in next PR to store features used by a given function. At first we'll only track use of checkpoints in function.
For now we only detect the use of checkpoints in function code. This information will then be used to dynamically dispatch checkpointed syncs to Lambda
f46a53e to
877442a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
877442a to
7bf3cbf
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
9d22cc6 to
b3d2c6c
Compare
There was a problem hiding this comment.
Review found no issues; changes appear correct and complete.
Status: No Issues Found | Risk: Low
Review Details
📁 30 files reviewed | 💬 0 comments
Instruction Files
└── .claude/
├── agents/
│ └── nango-docs-migrator.md
└── skills/
├── agent-builder-skill/
│ ├── EXAMPLES.md
│ └── SKILL.md
├── creating-integration-docs/
│ └── SKILL.md
└── creating-skills-skill/
└── SKILL.md
For now we only detect the use of checkpoints in function code.
This information will then be used to dynamically dispatch checkpointed syncs to Lambda
Notes:
Add CLI feature detection and persist
featuresthrough deploy pipelineThis PR introduces feature detection in the CLI build pipeline (currently tracking checkpoint usage) and propagates a new
featuresarray through parsed definitions, deploy payloads, and persisted sync configs. It adds database support for storing features, updates shared types and validation to include the field, and ensures templates and YAML parsing default to empty feature lists where detection is not supported.Tests and fixtures are updated to cover feature detection and to include
featuresin expected configs, sync configs, and webhook/job contexts, ensuring the new field is consistently handled across services.This summary was automatically generated by @propel-code-bot