Fix/misc bug1#704
Open
linonetwo wants to merge 362 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces workspace grouping (UI + persistence) and expands workspace import/config behavior, alongside a few reliability and UI fixes in Preferences and services.
Changes:
- Add workspace groups (persisted in settings, draggable grouping/reordering in sidebar, management section in Preferences, context-menu actions).
- Add “local-only” workspace config mode via
useTidgiConfigSync, plus optional selective import of values fromtidgi.config.json. - Improve robustness/UX: restart flow cleanup, TimePicker rendering/timezone handling, preferences window sizing/scrolling, and Git repo detection + SearchGithubRepo behavior.
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/windows/Preferences/sections/Sync.tsx | Fix TimePicker time extraction using UTC + clock view renderer. |
| src/windows/Preferences/sections/Notifications.tsx | Use clock renderer for notification schedule pickers. |
| src/windows/Preferences/registerCustomSections.tsx | Register new workspace group management custom item. |
| src/windows/Preferences/customItems/WorkspaceGroupsItem.tsx | New UI to create/rename/delete groups and manage membership. |
| src/windows/Preferences/customItems/NotificationScheduleItem.tsx | Use clock renderer for schedule pickers. |
| src/windows/Preferences/SectionsSideBar.tsx | Make sidebar full-height and scrollable. |
| src/windows/Preferences/SearchBar.tsx | Add test id for preferences search input. |
| src/windows/Preferences/SchemaRenderer.tsx | Render custom components in search results when registered. |
| src/windows/AddWorkspace/useForm.ts | Allow merging selected imported config into workspace config. |
| src/windows/AddWorkspace/useExistedWiki.ts | Pass selected import config into workspace creation path. |
| src/windows/AddWorkspace/useCloneWiki.ts | Pass selected import config into clone creation path. |
| src/windows/AddWorkspace/index.tsx | Add import-config dialog + state, pass selected config to done buttons. |
| src/windows/AddWorkspace/ImportConfigDialog.tsx | New dialog to select which tidgi.config keys to import. |
| src/windows/AddWorkspace/ExistedWikiDoneButton.tsx | Wire selected import config into existed-wiki submit flow. |
| src/windows/AddWorkspace/CloneWikiDoneButton.tsx | Wire selected import config into clone submit flow. |
| src/services/workspacesView/index.ts | Ensure restart resets isRestarting via try/finally and logs failures. |
| src/services/workspaces/interface.ts | Add groupId, useTidgiConfigSync, group types, and group API surface. |
| src/services/workspaces/index.ts | Implement group persistence/APIs + gate tidgi.config syncing by useTidgiConfigSync. |
| src/services/workspaces/hooks.ts | Add hooks for workspace group observables (map/sorted list). |
| src/services/workspaces/getWorkspaceMenuTemplate.ts | Add context-menu actions for creating/moving/removing groups. |
| src/services/workspaces/tests/useTidgiConfigSync.test.ts | Unit tests for useTidgiConfigSync behavior (read/write gating). |
| src/services/windows/WindowProperties.ts | Increase Preferences window dimensions. |
| src/services/wikiGitWorkspace/index.ts | Simplify removeWiki calls and update error message. |
| src/services/wikiEmbedding/tests/index.test.ts | Update test workspace defaults for new useTidgiConfigSync field. |
| src/services/wiki/wikiWorker/startNodeJSWiki.ts | Delay stdout/stderr intercept setup to avoid startup race. |
| src/services/wiki/interface.ts | Update removeWiki signature (remove legacy params). |
| src/services/wiki/index.ts | Await notifyServicesReady and simplify removeWiki implementation. |
| src/services/sync/index.ts | Remove redundant webContents reload after restart (restart now handles it). |
| src/services/preferences/interface.ts | Add workspaceGroups preferences section enum entry. |
| src/services/preferences/definitions/workspaceGroups.ts | Define new Preferences section for group management. |
| src/services/preferences/definitions/registry.ts | Register workspace groups section in Preferences section list. |
| src/services/git/tests/gitSyncRepoDetection.test.ts | Add regression test for git-sync-js repo detection compatibility. |
| src/services/database/interface.ts | Persist workspaceGroups in settings file shape. |
| src/pages/Main/WorkspaceIconAndSelector/WorkspaceSelectorBase.tsx | Add drag-intent styling + attributes for grouping UX/testing. |
| src/pages/Main/WorkspaceIconAndSelector/SortableWorkspaceSelectorList.tsx | Major update: group headers, grouping/reordering drag logic, overlay previews. |
| src/pages/Main/WorkspaceIconAndSelector/SortableWorkspaceSelectorButton.tsx | Add drop-zone anchors + drag intent propagation to base component. |
| src/pages/Main/Sidebar.tsx | Adjust sidebar bottom button structure (preferences/update). |
| src/components/TokenForm/index.tsx | Default token-provider tab based on existing logged-in user tokens. |
| src/components/StorageService/tests/SearchGithubRepo.test.tsx | Add tests ensuring no auto-select and correct click selection. |
| src/components/StorageService/SearchGithubRepo.tsx | Remove auto-selection of first repo result. |
| src/tests/mocks/services-container.ts | Update mocked workspace defaults for useTidgiConfigSync. |
| pnpm-lock.yaml | Bump git-sync-js and lockfile adjustments. |
| package.json | Bump git-sync-js dependency to ^2.3.3. |
| localization/locales/zh-Hans/translation.json | Add strings for import-config dialog + workspace group UI. |
| localization/locales/en/translation.json | Add strings for import-config dialog + workspace group UI. |
| features/workspaceGroup.feature | New E2E coverage for grouping UX and preferences search. |
| features/workspaceConfig.feature | New E2E scenarios for non-synced config isolation and restart persistence. |
| features/stepDefinitions/workspaceGroup.ts | Step definitions to drive group creation/drag/reorder assertions. |
| features/stepDefinitions/wiki.ts | Add step to assert JSON path/value is NOT present in a file. |
| features/stepDefinitions/ui.ts | Add generic steps to type into focused input and press a key. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…p into fix/misc-bug1
…ration measurement All timeout values (PLAYWRIGHT_TIMEOUT, PLAYWRIGHT_SHORT_TIMEOUT, LOG_MARKER_WAIT_TIMEOUT, UI_RETRY_ATTEMPTS) now come exclusively from calibration JSON. Preflight tracks element step timings (click/type/check) and stores elementMs. Preflight mode uses Number.MAX_SAFE_INTEGER instead of a hardcoded ceiling. Callers of electron.launch/waitForLoadState/click use HEAVY_PLAYWRIGHT_TIMEOUT or PLAYWRIGHT_SHORT_TIMEOUT instead of hardcoded values. No hardcoded timeout values anywhere in the E2E code.
- Shorten calibration log messages to avoid long line formatting - Fix console.warn parameter formatting in ip.ts ip.ts dprint issue was introduced by earlier commit 4a1a130 Update ip.ts
…issing calibration file Number.MAX_SAFE_INTEGER overflows Node.js setTimeout (32-bit signed int limit), setting timeout to 1ms and causing all steps to fail immediately. Use 2^31-1 (max safe) instead. Also add conservative fallbacks when calibration file is missing (e.g., first CI run without prior calibration).
Calibration profile was only running smoke.feature, measuring a worst-step of 4278ms. Full suite has much heavier scenarios (AI, sync, hibernation) that need higher step timeout. Run the full feature suite for calibration, with 2 runs to keep CI reasonable.
…ype timeouts measured The step timeout catches completely stuck steps, not per-operation budgets. Smoke-test calibration (30s run) cannot predict worst-case steps from complex scenarios (AI, sync, hibernation). Use the generous fallback (120s) for the step timeout while keeping per-type timeouts (launch, element, wait) tight from measurement.
… timeouts tight Log-marker waits involve background processes (sync, git, SSE) that can take longer than simple measured element waits. Use the generous step timeout budget for these operations while keeping element-level timeouts tight and measured.
…ion file requirement - Remove FALLBACK_STEP_MS/LAUNCH_MS/WAIT_MS/ELEMENT_MS — all timeouts now come exclusively from calibration measurements. - requireRecord() throws with clear instructions when calibration file is missing instead of silently using fallback values. - getMeasuredStepTimeoutMs() uses measured stepMs from calibration. - Re-import getMeasuredWaitTimeoutMs for LOG_MARKER_WAIT_TIMEOUT. - Add FORCE_JAVASCRIPT_ACTIONS_TO_NODE24=true to all CI workflows to fix Node.js 20 actions deprecation warnings.
A single complex step can involve launch + wait + element operations. Using the sum of per-type maxima (all measured) gives a realistic composite budget without any hardcoded values.
…easured Smoke-test calibration cannot predict worst-case step durations for complex scenarios (background sync, git, AI mock calls). The cucumber step timeout is a safety net for completely stuck steps, not a per-operation budget. Keep per-type timeouts (launch, element, wait) tight and measured from calibration.
…ight for background ops Log-marker waits involve background processes (sync, git, SSE) that can take much longer than measured element-level waits. Use the generous step timeout budget for these waits, while keeping element and launch timeouts tight and measured.
…IPT_ACTIONS_TO_NODE24 - Move calibration preflight from test:e2e to test:prepare-e2e so calibration runs during preparation, not during tests. - Remove rimraf/developmentMkdir duplication from test:e2e. - Increase Prepare E2E timeout to 15min to accommodate calibration. - Remove FORCE_JAVASCRIPT_ACTIONS_TO_NODE24 workaround; actions will update to Node.js 24 naturally before September 2026 deprecation.
The calibration preflight launches Electron for smoke tests, which requires an X display. xvfb-run provides the virtual display.
…perations Using the full cucumber step budget for log marker waits leaves no time for the enclosing step's other operations (evaluate, restart). Reserve PLAYWRIGHT_TIMEOUT + HEAVY_PLAYWRIGHT_TIMEOUT as headroom, computed from measured values.
Moving calibration to test:prepare-e2e introduced double rimraf and changed the execution order. The original structure (rimraf + mkdir + calibration + cucumber in a single test:e2e script) was proven to work. Revert to that while keeping other improvements (measured per-type timeouts, generous step timeout, NO_TIMEOUT fix).
…ready true When enableFileSystemWatch is already true in the runtime workspace (e.g., set by a previous scenario or Background), restarting the wiki is unnecessary and causes a race condition in FileSystemWatcher initialization. Only restart when the setting value actually changes.
…nd include Linux in E2E paintable windows
… offscreen logic with previousCustomBounds check
… window visibility
… during navigation
- crossWindowSync: replace watch-fs marker wait with file content polling step to correctly verify browser-originated syncer writes - workspace DnD: hybrid rect (dnd-kit top + live DOM height) prevents stale cached heights from misclassifying center drops as reorder-after - agent cleanup: mergeWithDefaultAgent on read, partial-update undefined filter, deleteAgentDef temp-only guard with AgentInstanceService lifecycle cleanup, ref-based unmount-only cleanup in CreateNewAgentContent - add unit tests for default merge semantics and deleteAgentDef temp guard
1. template/wiki submodule: patch mobile-sync plugin to use functional diff_match_patch API (patchMake/patchApply) instead of new diff_match_patch() constructor, which is not exported by TiddlyWiki 5.4 bundled module. 2. NewProviderForm.tsx: add data-testid attributes to MUI <Select> components for reliable Playwright selectors. 3. preference.feature: use [data-testid='new-provider- preset-select'] instead of generic div[role='combobox'] to avoid ambiguity when multiple selects are visible.
Submodule fix:
- Update template/wiki pointer from unreachable af495b9 to reachable 076d188
(rebased mobile-sync fix onto latest origin/master)
Code fixes:
- mergeUtilities: normalize CRLF in git ls-files -u output; trim stage tokens
- deepLink: sanitizer no longer strips valid TW title chars ([], {}, <>), only removes HTML tags
- workspace selector: use nanoid() instead of Date.now() for drag-created group IDs
- preferences SchemaRenderer: preserve fallback rendering for unregistered custom items in search mode
- WorkspaceSchemaRenderer: txEn null safety (?? '')
- analytics: default analyticsEnabled=false; add disclosure gate blocking pre-consent tracking;
allow E2E/test bypass via TIDGI_ANALYTICS_HOST env var
- mcpServer: refuse startup when requireToken=true and token is empty/whitespace;
trim token in extraction and startup; register transport cleanup before handleRequest
Tests:
- Add analytics service tests (disclosure, enabled state, tracking no-op)
- Add MCP HTTP server tests (empty/missing/invalid token → 401, valid → 200)
- Add deepLink preferences routing tests
- Add preferences AllSectionsRendering tests (section coverage + search mode)
All unit tests, lint, and typecheck pass.
… clean The calibration file was stored in test-artifacts/.calibration.json, but both pnpm run clean and pnpm test:e2e delete the entire test-artifacts directory. This forced a full recalibration on every test run, adding significant delay and breaking standalone scenario execution after est:prepare-e2e. Move the file to the repo root (.calibration.json) and gitignore it so calibration survives clean/test cycles.
…indows After taskkill /T /F terminates the Electron process tree, calling this.app.context() in Playwright hangs indefinitely because the CDP pipe is dead. The Promise.race timeout cannot rescue this since .context() is evaluated synchronously before entering the race. Skip context().close() on Windows after taskkill — the process tree is already terminated and there is nothing left to close.
After taskkill /T /F terminates the Electron process tree, calling this.app.context() in Playwright hangs indefinitely because the CDP pipe is dead. The Promise.race timeout cannot rescue this since .context() is evaluated synchronously before entering the race. Fixed in both: - cleanup.ts After hook (scenario-level cleanup) - application.ts closeTidGiApplication (explicit close step) On Windows, taskkill already terminates the entire process tree, so there is nothing left for Playwright to close gracefully.
…on CI; add @calibrate to preference AI scenario
…on data structure
- workspaceGroup.ts: remove verbose zone coordinate diagnostic log and viewport warning - wiki.ts: remove [test] debug logs inside executeJavaScript and git init skipped message - timeouts.ts: remove [Timeout] startup log and unused isCI variable These logs polluted test output with noise on every run. Error-level diagnostics (RENDERER CONSOLE ERROR, assertion context) are kept.
…enarios fail When a calibration run has non-zero exit (some @calibrate scenarios failed), cucumber still writes JSON results for completed scenarios. Previously the entire run was discarded, which meant if both runs had at least one failure each, calibration would be empty and the FATAL error would kill the e2e step. Now we always attempt to extract step timings from the JSON output regardless of cucumber's exit code. Partial data is still valid for deriving timeout budgets — the worst-case measurement among completed scenarios is used. This follows the no-hardcoded-timeouts principle: all timeout values are still derived from actual measurements, never from magic numbers.
The MUI Select renders MenuItem elements as li[role='option'] in a portal.
Using bare li:has-text('comfyui') was unreliable because:
1. It could match li elements outside the dropdown portal
2. MUI Select portal animation may not have completed when checked
3. Other MUI Select selectors in the same file already use li[role='option']
…gress output
The page.on('console') listener was printing all renderer console.error
messages (401, 404, GraphQL errors) to stdout, mixing them into Cucumber's
progress dots. These errors are expected in the test environment.
Replace the complex deriveMeasuredTimeoutBudget formula with a simpler approach: use the spread (difference) between two calibration runs as the error margin, with a minimum 10% safety cushion. This directly reflects CI jitter and machine variance. Before: max(spread, maxDeviation, observedMax/sampleCount) — the scarcity margin dominated (~1.5x max) even when runs were consistent. After: max(spread, ceil(observedMax * 0.1)) — spread is the primary signal, 10% floor handles near-identical runs.
1. wiki.ts: stop wiki before restart to prevent 'started same wiki twice'
error when update-workspace-settings already triggered an auto-restart.
2. agentTool.feature: add :has-text('approach') to ask-question-container
selector so it matches only the first question, not stale answered ones.
The wiki restart step takes ~35s, but the 10% margin only gave ~34.7s cucumber step timeout (observed max 31573 * 1.1). Increasing to 20% gives ~37.9s, providing enough headroom for restart operations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.