Skip to content

Support new Project & CLI features for integration tests - #1057

Merged
josephjclark merged 29 commits into
mainfrom
projects-root
Oct 14, 2025
Merged

josephjclark merged 29 commits into
mainfrom
projects-root

Conversation

@josephjclark

@josephjclark josephjclark commented Oct 9, 2025 •

Copy link
Copy Markdown
Collaborator

Adding various small features for the CLI - just enough to keep me moving forward

  • In Projects, allow to load from state file directly
  • In CLI, allow me to checkout a project from a state file directly
  • Start clarifying use of name and id per What's in a name? #1065

The CLI one maybe needs a bit more thinking about because where do we checkout TO? Probably any arbitrary folder (the passed project path or the current working dir). In this mode we should probably warn when not expanding to an openfn project, but we should do it (and generate an openfn.yaml).

This lets me expand a project to a local folder for testing and debug purposes.

I suppose by the same token I should be able to do like openfn checkout staging --out-dir /tmp/staging - even if tmp/staging isn't an actual repo

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our Responsible AI Policy

Release branch checklist

Delete this section if this is not a release PR.

If this IS a release branch:

  • Run pnpm changeset version from root to bump versions
  • Run pnpm install
  • Commit the new version numbers
  • Run pnpm changeset tag to generate tags
  • Push tags git push --tags

Tags may need updating if commits come in after the tags are first generated.

@github-project-automation github-project-automation Bot moved this to New Issues in Core Oct 9, 2025
@josephjclark
josephjclark changed the base branch from main to release/next October 9, 2025 16:34
@josephjclark

Copy link
Copy Markdown
Collaborator Author

I've got a little problem here on the .only test which comes from confusion over workflow name and workflow id.

I'm too tired and frazzled to fix this now - this stuff really needs a good cleanup. I'll take a look later and see if I can fix quickly (you'd think so!) but it may require deeper surgery. Very annoying.

const { jobs, edges, triggers, name, ...remoteProps } = workflow;
const mapped: l.Workflow = {
id: slugify(workflow.name),
id: workflow.id ?? slugify(workflow.name),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not really sure about some of this actually - might have to think carefully about names and ids and ensure we have clean clear docs

}

if (!checkedProject.name) {
if (!targetProject.name) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't want to do it in this PR, but all this needs to be id focused. Name is an optional label now.

*/
export type ExecutionPlan = {
id?: UUID; // this would be the run (nee attempt) id
id?: UUID; // TODO make required

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Don't want to do it as part of this PR, but it's a good note for #1065

Comment thread packages/project/src/gen/generator.ts Outdated
name: string;
uuidSeed: number;
openfnUuid: boolean;
openfnUuid: boolean; // TODO probably need to do this by default?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@doc-han can you remember what this property was supposed to control?

I inherited it from the older workflow generator but I'm not sure the purpose

I think what we want is:

  • By default everything will have a UUID generated for it
  • All UUIDs will use the seed if set, otherwise they'll be legit UUIDs
  • If you want, you can set an option like withUUIDs to false to prevent UUIDs being generated. But this feels like a "feature request" to be added later

@josephjclark
josephjclark requested a review from doc-han October 13, 2025 13:10

@josephjclark josephjclark left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

.

if (!raw.id) {
// Workflow ID is required, so make sure it gets set
// before calling the constructor
raw.id = 'workflow';

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

An important change here:

The Workflow class now MUST have an id value. But the generator will default it, so that you don't have to add @id x to all the workflows you want to define

const project = fromAppState(state, config);

t.is(project.name, 'aaa');
t.is(project.name, 'My Workflow');

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

As part of #1065, in tests, when using the Name field I want it to look like a name, not an id. So include capital letters and spaces. This just helps explain how the different properties are used.

@josephjclark

Copy link
Copy Markdown
Collaborator Author

@doc-han would you mind having a read over this at the end of your day?

Feedback is welcome but mostly I want you to observe changes I'm making to resolve #1065. It'll help set a good example for you, but also help you keep up with what's changing

The workflow generator MUST generate a name

ALso some stuff is off in state serialization
@josephjclark

Copy link
Copy Markdown
Collaborator Author

Turns out this branch was a bit broken. Filtering through the name id stuff has been super hard and confusing. Hope its up and running now!

Comment thread packages/lexicon/core.d.ts Resolved
@josephjclark
josephjclark changed the base branch from release/next to main October 14, 2025 17:40
@josephjclark
josephjclark merged commit 6946618 into main Oct 14, 2025
6 checks passed
@josephjclark
josephjclark deleted the projects-root branch October 14, 2025 17:41
@github-project-automation github-project-automation Bot moved this from New Issues to Done in Core Oct 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants