Skip to content

fix(web): publish a workflow only after its saved content changes - #168

Open
crispinhu27 wants to merge 2 commits into
mainfrom
feature/workflow-publish-gate
Open

crispinhu27 wants to merge 2 commits into
mainfrom
feature/workflow-publish-gate

Conversation

@crispinhu27

Copy link
Copy Markdown
Collaborator

What changed

Saving a workflow that has not changed no longer creates a revision, and publishing the same saved content keeps the current version. The publish button stays disabled while a publish is in flight and after the current content is already published.

  • Switching workflows with unsaved edits asks to keep editing or discard the changes.
  • The script editor shares font, letter spacing, and tab width across the input, line numbers, and highlight. Long lines scroll horizontally, and the highlight follows the input. Python highlighting tokenizes the source once so generated markup is not rewritten.
  • Skill file trees scroll inside their own panel.

Scope

  • Platform core / API / runner
  • Public adapter / deployment
  • Classic
  • Portable
  • Shared behavior contract
  • Installer / CLI
  • Documentation only

Compatibility

No schema migration is required. Published workflows that predate the published-revision marker stay published until their saved content changes. Repeating a publish of unchanged content keeps the current version, and a client cannot mark an edit as already published.

Validation

Ran:

go test ./internal/repo/ -count=1 -timeout=180s -run 'TestWorkflowPublishOnlyAfterChanges|TestPublishGenericWorkflow|TestUpdateGenericWorkflow|TestWorkflowRevision|TestWorkflowOptimistic'
go test ./internal/httpapi/ -count=1 -timeout=240s -run 'TestPlatformAdminCanEditGenericTemplate|TestCatalogWorkflowOperations'
node --test webtest/tests/flows-editor.test.mjs webtest/tests/app-common.test.mjs
python3 scripts/check-public.py

Not run: go test -p=2 ./..., go vet ./..., bash local/scripts/validate.sh, bash local/scripts/smoke-install.sh, bash local/scripts/e2e.sh, bash scripts/scan-secrets.sh.

Safety

  • I did not include credentials, internal endpoints, production data, or organization-specific infrastructure.
  • I preserved user files and unrelated changes.
  • I updated generated Classic host bundles when applicable.
  • I documented third-party content and licensing changes when applicable.

@Robert0622

Copy link
Copy Markdown
Collaborator

LoopForge PR Review — head 57dd0e87d3d5807b5ed082a37a35d12d9674a445

Verdict: ⚠️ Verification blocked

  • Diff: 14 files, +318/-41 (internal/repo/*, internal/httpapi/catalog.go, internal/domain/workflow.go, web/app.js/web/app/flows.js/CSS, webtest/tests/*). No go.mod/go.sum changes.
  • Ran locally in an isolated sandbox (no secrets/no prod access): node --test tests/app-common.test.mjs tests/flows-editor.test.mjs under webtest/ — 10/10 pass, including the two new tests for the publish gate (publishing blocks double clicks and stays disabled until new changes are saved, switching workflows explicitly offers to discard edits or continue editing).
  • Could not run the Go suite locally — this sandbox has no Go toolchain, so go test ./internal/repo/... ./internal/httpapi/... from the PR description was not reproduced here.
  • CI on this exact head SHA: required checks core and platform-required are failing. Drilling into the core job's steps via the Actions API shows every actual test/coverage step (Non-browser Go tests, Real-browser Go tests, Coverage gate regression tests, Go statement coverage gate, Browser-free UI tests, go vet) is ✅; the only failing step is go run golang.org/x/vuln/cmd/govulncheck@v1.9.0 ./.... platform-required only fails because it gates on core.
  • This PR doesn't touch go.mod/go.sum, and main@63c9e46 (this PR's base) last ran platform fully green. From here I can't tell whether govulncheck is flagging a newly-reachable call path introduced by this diff or a vulnerability that was published to the govulncheck DB between the two runs (govulncheck fetches a live DB, so DB drift alone can flip this independently of the code change).

Missing to resolve: open the govulncheck step log on the core job (run 42978695388, job 130383729291) and check which finding fired — if it points at a path only reachable via the new internal/repo/catalog.go/generic_store.go code, that's a real blocker; if it's an unrelated existing dependency, this is likely noise unrelated to this PR's scope.

@crispinhu27 could you check that govulncheck log and confirm whether it's related to this change?

@Robert0622 Robert0622 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.

Reviewed head ac626d2. No blocking findings in the workflow publication, revision-marker, legacy-definition compatibility, and editor changes.

Validation performed in this review: targeted repo/HTTP API workflow publication, revision and optimistic-locking tests passed; node --test webtest/tests/flows-editor.test.mjs webtest/tests/app-common.test.mjs passed; go vet ./internal/repo ./internal/httpapi and the public-content check passed. Also ran TestMySQLConcurrentWorkflowPublish against a disposable MySQL 8.4 container: concurrent publication of both project and generic workflows passed without duplicate version increments. The container was removed afterwards. No paid model call is needed for these deterministic publication changes.

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