Fixing pull --beta and refactoring along the way by josephjclark · Pull Request #1084 · OpenFn/kit

josephjclark · 2025-10-21T17:19:22Z

Had to get an old state.json format for Elias and it turned into a nightmare

Basically pull was written before a lot of our nice Workspace APIs and stuff, and was rushed out even then

And what's happening is the pull isn't using config properly from the openfn.yaml file

This PR basically plugs in the Workspace class to make life easier.

But I hit loads of little problems on the way - the code here is super messy and we really need to tidy up a bit

Fixes #1083

Headline changes

Rename project.repo to project.config
Generally renamed the repo/config stuff do "Workspace Config", or variations of
Centralised option parsing into a single file, which sets defaults
Introduced a new project.yaml format ({ project, workspace })
Standardised and simplified how we load openfn.yaml

AI Usage

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

You can read more details in our Responsible AI Policy

josephjclark · 2025-10-21T17:20:17Z

packages/cli/src/pull/beta.ts

  const { OPENFN_API_KEY, OPENFN_ENDPOINT } = process.env;

-  const config: Partial<Config> = {
+  const cfg: Partial<Config> = {


This old config stuff was just a cheap way to use the old deploy API. We need to phase it out soon

josephjclark · 2025-10-21T17:22:17Z

packages/cli/src/pull/beta.ts

+
+  // TODO is outputRoot the right dir for this?
+  const workspace = new Workspace(outputRoot);
+  const config = workspace.getConfig();


Not a fan of the getter - I just want to use workspace.config here. Why make it private?

josephjclark · 2025-10-21T17:24:59Z

packages/cli/src/pull/beta.ts


  const project = Project.from('state', data, {
-    endpoint: config.endpoint,
+    repo: config,


The second argument here needs the same structure as new Project

So basically I'm passing state (as string, yam or json, maybe even as a file path?) and we'll load all the values from that state file. But if I want to override anything, or add context which the state file doesn't have, all that stuff just goes on the second argument.

josephjclark · 2025-10-21T17:25:41Z

packages/cli/src/pull/beta.ts

  const workflowsRoot = path.resolve(
    outputRoot,
-    project.repo?.workflowRoot ?? 'workflows'
+    project.repo.dirs?.workflows ?? 'workflows'


Shouldn't need any of this optional chaining bullshit: I want good robust config with everything defaulted

josephjclark · 2025-10-21T17:27:37Z

packages/project/src/Project.ts

  // workspace-wide configuration options
  // these should be shared across projects
  // and saved to an openfn.yaml file
+  // TODO should this just be the Workspace now?


The hestitation is that there's a bit of circularity between Project and Workspace. I should be able to have one without the other - but they're actually coupled a bit nastily through config.

Needs more thought

josephjclark · 2025-10-22T08:33:12Z

Here's the plan to tidy some of this stuff up:

openfn.yaml is the Workspace Configuration file. It contains configuration for the workspace
So the repo options are just configuration
A Project may have an associated Workspace
Project and Workspace will both default config from the same place
A Project always has config (which may be a proxy for Workspace config)

josephjclark · 2025-10-22T08:40:12Z

Oh and openfn.yaml needs to look like this:

project:
workspace:

That gives you metadata about the checked out project, and config for the workspace generally

We should support legacy and new file formats for a while, but always serialize to the new

doc-han · 2025-10-22T12:51:19Z

From reading the description. we want to use workspace class in pull command.
An edge case is that, pull command can work in any empty directory but then the workspace class only works in a directly that has an openfn project previously pulled into it. Hence, if pull depends on the current state of workspace, we might not be able to pull into a new empty directory.

maybe init is needed? so then pull becomes
pull = init? + fetch + expand to filesystem

? - run if not already run

josephjclark · 2025-10-22T13:29:57Z

Yeah, something like that.

Either the CLI can run openfn init on an empty directory. Or we can have something like Workspace.fix() which generates a workspace with default values. Or we ask the user.

pull doesn't really need to care if it runs in a valid directory or not - it should just do what it needs to do. That's a different usecase to eg openfn project, which can only run in a valid workspace

josephjclark · 2025-10-22T14:03:31Z

Really need to wrap this up so I think I'm going to pause the "a Project should reference its workspace" thing and open a new issue

I think there are big improvements here, and it seems to work, so I'm going to declare victory ASAP

josephjclark · 2025-10-22T14:50:57Z

openfn.yaml now looks like this:

project:
  uuid: d3367267-ef25-41cf-91b2-43d18f831f3c
  endpoint: https://app.staging.openfn.org
  env: project
  inserted_at: 2025-10-21T17:10:57Z
  updated_at: 2025-10-21T17:10:57Z
workspace:
  dirs:
    projects: .projects
    workflows: workflows
  formats:
    openfn: yaml
    project: json
    workflow: json

Old yaml files, pre 0.6, are still 100% compatible. Very few tests have updated to the new structure.

👨‍🍳 💋

josephjclark · 2025-10-22T14:56:56Z

.changeset/fuzzy-colts-invite.md

@@ -0,0 +1,5 @@
+---
+'@openfn/project': patch


I'll recycle all this into a better patch note on release

This PR fixes a bug where workspace config settings are not properly respected by pull. This must be known.

josephjclark · 2025-10-22T15:02:18Z

@doc-han When you get a chance I'd like you to have a close look over this please: partly for your opinions (this is the right thing to do right??) but also to keep you in-sync with the changes. There's more work I want to do and you might be "it" for the next ticket

josephjclark · 2025-10-22T15:57:53Z

packages/project/test/util/config.test.ts

+  t.is(result.content, 'x: 1');
+});
+
+test('find openfn.json', (t) => {


@doc-han thank you for the project integration tests! They caught an issue here which would have resulted in a big breakage 😬

doc-han

I don't have much issue with the code here. looks fine. my only issue has to do with some typings(Typescript) not being strict.

josephjclark · 2025-10-24T15:06:22Z

@doc-han agreed - let's try and fix those as we go. We should be striving to improve the typing and general semantics on every PR from here going forward - I welcome your contributions in cleaning this all up!

trying to enable old json formats

23e56dc

taylordowns2000 added this to Core Oct 21, 2025

github-project-automation bot moved this to New Issues in Core Oct 21, 2025

josephjclark commented Oct 21, 2025

View reviewed changes

josephjclark changed the title ~~trying to enable old json formats~~ Fixing pull --beta and refactoring along the way Oct 21, 2025

josephjclark marked this pull request as draft October 21, 2025 17:23

josephjclark commented Oct 21, 2025

View reviewed changes

josephjclark added 6 commits October 22, 2025 11:45

refactor project.repo to project.config

e4584ab

changeset

5e919d5

test and refactor in workspace

1c1ae21

make project meta public

0b43135

new helper to load workspace file

518ec0b

standardise workflow.yaml loading

2eeb12a

update new format for openfn.yaml

17fe1bc

josephjclark added 5 commits October 22, 2025 14:31

fix types

b511a8c

suppress error logging

12502a6

types

c09b04e

cli: fix config

7160782

fix tests

21ac5cc

josephjclark added 2 commits October 22, 2025 15:33

track project name in legacy config files

4f98667

track project name from app state

cf572a6

update test

b0c9090

josephjclark marked this pull request as ready for review October 22, 2025 15:01

josephjclark commented Oct 22, 2025

View reviewed changes

josephjclark requested a review from doc-han October 22, 2025 15:01

josephjclark added 2 commits October 22, 2025 16:04

fix more tests

6ea477f

fix empty workspace files

87b8d18

josephjclark commented Oct 22, 2025

View reviewed changes

josephjclark changed the base branch from main to release/next October 22, 2025 16:04

doc-han approved these changes Oct 24, 2025

View reviewed changes

josephjclark merged commit edfc759 into release/next Oct 24, 2025
6 checks passed

github-project-automation bot moved this from New Issues to Done in Core Oct 24, 2025

josephjclark deleted the use-workspace-in-pull branch October 24, 2025 15:06

Conversation

josephjclark commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Headline changes

AI Usage

Uh oh!

josephjclark Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

josephjclark Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

josephjclark Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

josephjclark Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

josephjclark Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

josephjclark commented Oct 22, 2025

Uh oh!

josephjclark commented Oct 22, 2025

Uh oh!

doc-han commented Oct 22, 2025

Uh oh!

josephjclark commented Oct 22, 2025

Uh oh!

josephjclark commented Oct 22, 2025

Uh oh!

josephjclark commented Oct 22, 2025

Uh oh!

josephjclark Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

josephjclark commented Oct 22, 2025

Uh oh!

josephjclark Oct 22, 2025

Choose a reason for hiding this comment

Uh oh!

doc-han left a comment

Choose a reason for hiding this comment

Uh oh!

josephjclark commented Oct 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

josephjclark commented Oct 21, 2025 •

edited

Loading