Introduce 'Document Ready' state in project open process

[dbarashev]

### Summary

This pull request integrates more project opening logic into the Project-Open state machine. Three new states allow for waiting for "Document Ready" state and for handling document fork and authentication needed states.

Besides important bugfixes: 
 - a websocket would not open if we just open a cloud document
- chart would freeze when opening a cloud document (related to the Decorator implementation)

[github-actions]

Test Results

351 tests  +2   351 ✅ +2   19s ⏱️ +3s
 96 suites ±0     0 💤 ±0 
 96 files   ±0     0 ❌ ±0 

Results for commit 86072d7. ± Comparison against base commit 83834f1.

♻️ This comment has been updated with latest results.

[claude]

Claude finished @dbarashev's task in 2m 56s —— View job

---

PR Review: Introduce 'Document Ready' state in project open process

• Gather context (diff + changed files)
• Review state machine changes
• Review new files (Authentication, DocumentForkDialog, ErrorHandling)
• Review cloud/websocket changes & Decorator fix
• Post review feedback

This is a solid refactoring that moves the project-open flow into an explicit state machine and removes the fragile channel-plumbing. The new transition() with built-in exception → Failed state is a nice improvement, and consolidating the error dialog into showProjectOpenErrorDialog is good. Below are the issues I found, ordered by severity.

---

🔴 Correctness

1. TwoPhaseBarrierImpl.activate() can silently drop exits registered before activation — Barriers.kt
The old isActive setter fired the exits when the barrier was activated with no pending entrances (counter == 0). The new activate() only stores the value:

fun activate(exitValue: T) { value = exitValue }

The new immediate-call path lives only in await() (if (isActive && counter.get() == 0) { code(value!!); return }), which covers await-after-activate. But the reverse ordering — await() registered first, then activate() with zero entrance activities — is no longer handled: nothing re-checks the already-queued exits, so they never fire. The two new tests both register an entrance, so this gap is untested. Whether a live call site hits it depends on every barrier always having ≥1 entrance. Recommend handling counter==0 inside activate() (call callExits()) and adding a test for "exit registered before activation, no entrances". Fix this →

2. ProjectOpenActivityFactory is now a never-cleared global singleton — ProjectOpenStateMachine.kt
Changing from a per-facade class to an object with a static builders list means every subscriber (ProjectUIFacadeImpl.init, WebSocketClient.init, LazyProjectDatabaseProxy, table components, GanttProject) appends to a list that is never cleared. The author's own // TODO: this shall be registered just once (GanttProject.java:275) flags this. Risk: in tests, multi-window, or repeated facade construction, builders accumulate and every new state machine re-runs every builder, potentially wiring initStateMachine/websocket/db transitions multiple times. Worth a de-dup or lifecycle story before this lands.

3. WebSocketClient.init references the global webSocket instead of this — GPCloudHttpImpl.kt:280

init {
  ProjectOpenActivityFactory.addBuilder { sm ->
    sm.stateCompleted.await { it.document.asOnlineDocument()?.let { webSocket.start() } }
  }
}

webSocket is the module-level val webSocket = WebSocketClient() being constructed here. It works only because the lambda runs later, after the global is assigned — fragile self-reference. Use this@WebSocketClient.start().

4. Fork-dialog CANCEL marks the activity Completed — ProjectUIFacadeImpl.kt (handleChoice)
On CANCEL the state goes DocumentForked → Completed, which then runs stateCompleted.await { undoManager.die() }. But on cancel the new document was never loaded (project.close() / openFileAsIs only happen on the DocumentReady → MainModelReady path). So we signal "completed" and kill the undo manager while the current project is still the live one. Please confirm this is intended (cancel = keep current project) — calling undoManager.die() here looks questionable.

---

🟠 Functional regression worth confirming

5. Decorator "fix" removes the offline-mode warning badge — GPCloudStatusBar.kt:283-300
The freeze is "fixed" by commenting out all Decorator.addDecoration/removeAllDecorations calls. That also removes the warning badge that indicated offline mode, and createWarningDecoration() (GPCloudStatusBar.kt:349) is now dead code. Is dropping the offline indicator acceptable, or should the warning be re-added via a non-Decorator approach (e.g. setting it directly as the label graphic)? As written this disables the feature rather than fixing the root cause.

---

🟡 Cleanup (large volume of dead/commented code)

These should be removed before merge — they make the diff hard to read and will rot:

• ProjectUIFacadeImpl.kt: the big commented openProject block (~lines 304+), private var cnt = 0 and the commented "Test Failure" injection inside the stateDocumentReady transition — clearly leftover test scaffolding.
• ProjectOpenStrategy.kt: open(document, docChannel) + the channel-based processFetchResult path now appear to be dead code — open() is only self-referenced (signin { open(...) } at line 161), no external caller uses it after the switch to start(). Also the commented finishChannel block in CommandLineProjectOpenStrategy and the bare //TODO near checkWellFormed().
• Dialog.kt: commented createOverlayPane(...) block in toggleProgress.
• GPCloudStatusBar.kt: commented Decorator.* lines (see Copy and Paste of supertasks with subtasks creates extra tasks #5).

Clean up dead code →

---

🟢 Minor / nits

• Dialog.toggleProgress: the show path uses FXThread.runLater but the cleanup lambda uses Platform.runLater. Pick one for consistency.
• ErrorHandling.kt: missing the GPL license header that Authentication.kt and DocumentForkDialog.kt carry.
• Missing trailing newlines: Document.kt, ErrorHandling.kt, GPCloudHttpImpl.kt, GPCloudStatusBar.kt.
• i18n verification: the generic transition() builds error keys as error.state.${toStateId} (e.g. project.open.error.state.mainModelReady). Please confirm those keys exist so failures don't render the raw key.

---

Tests pass (349 ✅) and the architecture direction is good. Items #1–#4 are the ones I'd want resolved or explicitly confirmed before merge; the rest is cleanup.
• Branch

[dbarashev]

@claude review

[claude]

Claude finished @dbarashev's task in 6s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude finished @dbarashev's task in 2m 26s —— View job

---

Re-review: Introduce 'Document Ready' state in project open process

• Gather context (diff + changed files)
• Re-review state machine & barrier changes
• Verify previously-raised issues against latest commit (86072d7)
• Post review feedback

Thanks for the iterations — several of the earlier blockers are now resolved. Here's where things stand against my previous review.

✅ Resolved since last review

• Welcome to GanttProject! #1 Barrier drops queued exits — activate() now calls callExits() when counter == 0 (Barriers.kt:90-92), and there's a dedicated test exits are called immediately upon activation (BarriersTest.kt:67-77). 👍
• Dragging a dependency from a subtask to its supertask fails  #3 WebSocketClient self-reference — now an unqualified start() (this.start()) instead of the module-level global (GPCloudHttpImpl.kt:284). 👍
• Wrong update of dependencies on subtask duration change #4 Fork-dialog CANCEL — now transitions to the new ProjectOpenActivityCancelled state (ProjectUIFacadeImpl.kt:229), and stateCancelled.await { } deliberately does not call undoManager.die(), with a comment explaining why (ProjectUIFacadeImpl.kt:259-262). Clean fix. 👍

---

🔴 Still open — confirmed

A. Missing i18n keys (was "please verify", now confirmed). The error keys built by the state machine do not exist in any .properties bundle. A grep for error.title / error.state.* definitions finds only the usages in ProjectOpenStateMachine.kt — no definitions anywhere. So any failure during open (e.g. the transition() catch at ProjectOpenStateMachine.kt:209, or error.title at ErrorHandling.kt:22) will render the raw key (project.open.error.state.mainModelReady, project.open.error.title) to the user. This needs the keys added before merge. Fix this →

B. #2 ProjectOpenActivityFactory is still a never-cleared global singleton. Unchanged — it's still an object whose builders list is appended to from ProjectUIFacadeImpl.init, WebSocketClient.init, etc., and never cleared. Your own // TODO: this shall be registered just once is still at GanttProject.java:275. Every new state machine re-invokes every accumulated builder. This is a lifecycle hazard for tests / multi-window / repeated facade construction. Worth a de-dup or registration-once story before this lands.

C. #5 Offline-mode warning badge still disabled. Unchanged — GPCloudStatusBar.kt:280,290,300 still have the commented-out Decorator.* calls, and createWarningDecoration() (GPCloudStatusBar.kt:349) is now dead code. The chart-freeze fix here removes the offline indicator rather than restoring it via a non-Decorator approach. Please confirm dropping the badge is acceptable, or re-add it (e.g. as the label graphic directly).

---

🟡 Cleanup still pending

These were flagged last time and are still in the tree:

• ProjectUIFacadeImpl.kt: private var cnt = 0 (line 205) and the commented "Test Failure" injection inside the stateDocumentReady transition (lines 243-246) — leftover test scaffolding. (The large commented openProject block from before is gone now — thanks.) Fix this →
• ProjectOpenStrategy.kt: the commented finishChannel block (lines 406-421) and bare //TODO (line 415) in CommandLineProjectOpenStrategy.doOpenStartupDocument. Also Step4.onFetchResultChange (lines 358-377) contains a debug println (line 364) and appears to be orphaned — runUiTasks returns Step4() but I find no caller of onFetchResultChange. Confirm whether Step4 is still wired up.
• Dialog.kt: commented createOverlayPane(...) block in DialogControllerFx.toggleProgress (lines 524-528, 532).
• ErrorHandling.kt: still missing the GPL license header that the other two new files (Authentication.kt, DocumentForkDialog.kt) carry.

---

🟢 Minor / nits

• ProjectOpenStateMachine.kt:182: the Completed transition still allows field is ProjectOpenActivityDocumentForked. Now that CANCEL routes to Cancelled, is the DocumentForked → Completed path still reachable, or is it leftover from before the Cancelled state existed?
• Dialog.toggleProgress (FX): show path uses FXThread.runLater, cleanup lambda uses Platform.runLater — pick one for consistency.
• Trailing newlines: still worth checking on Document.kt, ErrorHandling.kt, GPCloudHttpImpl.kt, GPCloudStatusBar.kt.

---

Net: the architecture is solid and #1/#3/#4 are properly fixed. Before merge I'd want (A) the i18n keys added (otherwise users see raw keys on any open failure), plus a decision on (B) the global factory lifecycle and (C) the offline badge. The rest is cleanup.
• Branch