Refactor: Move shared hooks from ui/hooks to shared/hooks#729
Merged
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis PR updates import paths to consolidate hooks (useToast, useSmartPolling) under features/shared, switching many relative imports to shared or alias-based paths. Tests update mock paths accordingly. Minor internal path tweak in useToast.ts for createOptimisticId. No runtime logic, signatures, or behavior changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (34)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reorganize hook structure to follow vertical slice architecture: - Move useSmartPolling, useThemeAware, useToast to features/shared/hooks - Update 38+ import statements across codebase - Update test file mocks to reference new locations - Remove old ui/hooks directory This change aligns shared utilities with the architectural pattern where truly shared code resides in the shared directory. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Wirasm
force-pushed
the
refactor/move-hooks-to-shared
branch
from
8f908b7 to
73042cb
Compare
This was referenced Oct 9, 2025
Merged
leonj1
pushed a commit
to leonj1/Archon
that referenced
this pull request
Oct 13, 2025
Reorganize hook structure to follow vertical slice architecture: - Move useSmartPolling, useThemeAware, useToast to features/shared/hooks - Update 38+ import statements across codebase - Update test file mocks to reference new locations - Remove old ui/hooks directory This change aligns shared utilities with the architectural pattern where truly shared code resides in the shared directory. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Claude <noreply@anthropic.com>
coleam00
added a commit
that referenced
this pull request
Apr 7, 2026
…based retries (#729) * feat: safe session continuity across workflow steps with forkSession-based retries (#691) Retries previously discarded all prior conversation context by passing resumeSessionId=undefined, because naive session resume would append to the original session file and corrupt it on a crash. This adds forkSession:true support so the SDK copies the prior session history into a new file before appending — leaving the source untouched — making it safe to always pass the prior session ID on retry. Changes: - Add forkSession?: boolean to WorkflowAssistantOptions (deps.ts) and AssistantRequestOptions (types/index.ts) - Pass forkSession to SDK Options in ClaudeClient with enhanced resuming_session log - Sequential executor: set shouldForkSession=true when resuming; inject into stepOptions; fix retry to always pass resumeSessionId - DAG executor: set shouldForkSession=true when resuming in executeNodeInternal; fix retry to always pass resumeSessionId; update isFresh comment - Fix resolvedOptions to explicitly set persistSession:false for Claude workflows (was silently defaulting to SDK's true) - Add context:'shared' union to DagNodeBase.context type; update clearContext docstring Fixes #691 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove persistSession:false from workflow resolvedOptions to preserve cross-step session continuity SDK docs confirm that persistSession:false prevents session files from being written to disk, making resume impossible for subsequent steps. Removing this option restores the SDK default (true), so step 1's session ID can be used by step 2 to resume. Also clarify context:'shared' docstring to note it has no effect on parallel layer nodes, and add unit tests asserting that retries pass forkSession:true when a prior session exists. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: add persistSession: false to Claude resolvedOptions Implements the missing persistSession fix claimed in the PR description. Without this, the SDK defaults to true and writes session transcripts to disk. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(loader): preserve context: 'shared' in DAG node YAML parsing The PR added 'shared' to DagNodeBase.context type but the loader only preserved 'fresh', silently dropping 'shared' during YAML parsing. This meant context: shared in workflow YAML was ignored at load time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
coleam00
pushed a commit
that referenced
this pull request
Apr 7, 2026
…ole format (#805) * refactor(workflows)!: remove sequential execution mode, DAG becomes sole format Remove the steps-based (sequential) workflow execution mode entirely. All workflows now use the nodes-based (DAG) format exclusively. - Convert 8 sequential default workflows to DAG format - Delete archon-fix-github-issue sequential (DAG version absorbs triggers) - Remove SingleStep, ParallelBlock, StepWorkflow types and guards - Gut executor.ts from ~2200 to ~730 lines (remove sequential loop) - Remove step_started/completed/failed and parallel_agent_* events - Remove logStepStart/Complete and logParallelBlockStart/Complete - Delete SequentialEditor, StepProgress, ParallelBlockView components - Remove sequential mode from workflow builder and execution views - Delete executor.test.ts (4395 lines), update ~45 test fixtures - Update CLAUDE.md and docs to reflect DAG-only format BREAKING CHANGE: Workflows using `steps:` format are no longer supported. Convert to `nodes:` (DAG) format. The loader provides a clear error message directing users to the migration guide. * fix: address review findings — guard errors, remove dead code, add tests - Guard logNodeSkip/logWorkflowError against filesystem errors in dag-executor - Move mkdir(artifactsDir) inside try-catch with user-friendly error - Remove startFromStep dead parameter from executeWorkflow signature - Remove isDagWorkflow() tautology and all callers (20+ sites) - Remove dead BuilderMode/mode state from frontend components - Remove vestigial isLoop, selectedStep, stepIndex, step_index fields - Remove "DAG" prefix from user-facing resume/error messages - Fix 5 stale docs (README, getting-started, authoring-commands, web adapter) - Update event-emitter tests to use node events instead of removed step events - Add executor-shared.test.ts (12 tests) for substituteWorkflowVariables - Add executor.test.ts (11 tests) for concurrent-run, model resolution, resume * fix(workflows): add migration guide, port preamble tests, improve error message - Add docs/sequential-dag-migration-guide.md with 3 conversion patterns (single step, chain with clearContext, parallel block) and a Claude Code migration command for automated conversion - Update loader error message to point to migration guide and include ready-to-run claude command - Port 8 preamble tests from deleted executor.test.ts to new executor-preamble.test.ts: staleness detection (3), concurrent-run guard (3), DAG resume (2) Addresses review feedback from #805. * fix(workflows): update loader test to match new error message wording * fix: address review findings — fail stuck runs, remove dead code, fix docs - Mark workflow run as failed when artifacts mkdir fails (prevents 15-min concurrent-run guard block) - Remove vestigial totalSteps from WorkflowStartedEvent and executor - Delete dead WorkflowToolbar.tsx (369 lines, no importers) - Remove stepIndex prop from StepLogs (always 0, label now "Node logs") - Restore cn() in StatusBar for consistent conditional classes - Promote resume-check log to error, add errorType to failure logs - Remove ghost $PLAN/$IMPLEMENTATION_SUMMARY from docs (never implemented) - Update workflows.md rules to DAG-only format - Fix migration guide trigger_rule example - Clean up blank-line residues and stale comments * fix: resolve rebase conflicts with #729 (forkSession) and #730 (dashboard) - Remove sequential forkSession/persistSession code from #729 (dead after sequential removal) - Fix loader type narrowing for DagNode context field - Update dashboard components from #730 to use dagNodes instead of steps - Remove WorkflowStepEvent/ParallelAgentEvent from dashboard SSE hook
Tyone88
pushed a commit
to Tyone88/Archon
that referenced
this pull request
Apr 16, 2026
…based retries (coleam00#729) * feat: safe session continuity across workflow steps with forkSession-based retries (coleam00#691) Retries previously discarded all prior conversation context by passing resumeSessionId=undefined, because naive session resume would append to the original session file and corrupt it on a crash. This adds forkSession:true support so the SDK copies the prior session history into a new file before appending — leaving the source untouched — making it safe to always pass the prior session ID on retry. Changes: - Add forkSession?: boolean to WorkflowAssistantOptions (deps.ts) and AssistantRequestOptions (types/index.ts) - Pass forkSession to SDK Options in ClaudeClient with enhanced resuming_session log - Sequential executor: set shouldForkSession=true when resuming; inject into stepOptions; fix retry to always pass resumeSessionId - DAG executor: set shouldForkSession=true when resuming in executeNodeInternal; fix retry to always pass resumeSessionId; update isFresh comment - Fix resolvedOptions to explicitly set persistSession:false for Claude workflows (was silently defaulting to SDK's true) - Add context:'shared' union to DagNodeBase.context type; update clearContext docstring Fixes coleam00#691 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove persistSession:false from workflow resolvedOptions to preserve cross-step session continuity SDK docs confirm that persistSession:false prevents session files from being written to disk, making resume impossible for subsequent steps. Removing this option restores the SDK default (true), so step 1's session ID can be used by step 2 to resume. Also clarify context:'shared' docstring to note it has no effect on parallel layer nodes, and add unit tests asserting that retries pass forkSession:true when a prior session exists. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: add persistSession: false to Claude resolvedOptions Implements the missing persistSession fix claimed in the PR description. Without this, the SDK defaults to true and writes session transcripts to disk. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(loader): preserve context: 'shared' in DAG node YAML parsing The PR added 'shared' to DagNodeBase.context type but the loader only preserved 'fresh', silently dropping 'shared' during YAML parsing. This meant context: shared in workflow YAML was ignored at load time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Tyone88
pushed a commit
to Tyone88/Archon
that referenced
this pull request
Apr 16, 2026
…ole format (coleam00#805) * refactor(workflows)!: remove sequential execution mode, DAG becomes sole format Remove the steps-based (sequential) workflow execution mode entirely. All workflows now use the nodes-based (DAG) format exclusively. - Convert 8 sequential default workflows to DAG format - Delete archon-fix-github-issue sequential (DAG version absorbs triggers) - Remove SingleStep, ParallelBlock, StepWorkflow types and guards - Gut executor.ts from ~2200 to ~730 lines (remove sequential loop) - Remove step_started/completed/failed and parallel_agent_* events - Remove logStepStart/Complete and logParallelBlockStart/Complete - Delete SequentialEditor, StepProgress, ParallelBlockView components - Remove sequential mode from workflow builder and execution views - Delete executor.test.ts (4395 lines), update ~45 test fixtures - Update CLAUDE.md and docs to reflect DAG-only format BREAKING CHANGE: Workflows using `steps:` format are no longer supported. Convert to `nodes:` (DAG) format. The loader provides a clear error message directing users to the migration guide. * fix: address review findings — guard errors, remove dead code, add tests - Guard logNodeSkip/logWorkflowError against filesystem errors in dag-executor - Move mkdir(artifactsDir) inside try-catch with user-friendly error - Remove startFromStep dead parameter from executeWorkflow signature - Remove isDagWorkflow() tautology and all callers (20+ sites) - Remove dead BuilderMode/mode state from frontend components - Remove vestigial isLoop, selectedStep, stepIndex, step_index fields - Remove "DAG" prefix from user-facing resume/error messages - Fix 5 stale docs (README, getting-started, authoring-commands, web adapter) - Update event-emitter tests to use node events instead of removed step events - Add executor-shared.test.ts (12 tests) for substituteWorkflowVariables - Add executor.test.ts (11 tests) for concurrent-run, model resolution, resume * fix(workflows): add migration guide, port preamble tests, improve error message - Add docs/sequential-dag-migration-guide.md with 3 conversion patterns (single step, chain with clearContext, parallel block) and a Claude Code migration command for automated conversion - Update loader error message to point to migration guide and include ready-to-run claude command - Port 8 preamble tests from deleted executor.test.ts to new executor-preamble.test.ts: staleness detection (3), concurrent-run guard (3), DAG resume (2) Addresses review feedback from coleam00#805. * fix(workflows): update loader test to match new error message wording * fix: address review findings — fail stuck runs, remove dead code, fix docs - Mark workflow run as failed when artifacts mkdir fails (prevents 15-min concurrent-run guard block) - Remove vestigial totalSteps from WorkflowStartedEvent and executor - Delete dead WorkflowToolbar.tsx (369 lines, no importers) - Remove stepIndex prop from StepLogs (always 0, label now "Node logs") - Restore cn() in StatusBar for consistent conditional classes - Promote resume-check log to error, add errorType to failure logs - Remove ghost $PLAN/$IMPLEMENTATION_SUMMARY from docs (never implemented) - Update workflows.md rules to DAG-only format - Fix migration guide trigger_rule example - Clean up blank-line residues and stale comments * fix: resolve rebase conflicts with coleam00#729 (forkSession) and coleam00#730 (dashboard) - Remove sequential forkSession/persistSession code from coleam00#729 (dead after sequential removal) - Fix loader type narrowing for DagNode context field - Update dashboard components from coleam00#730 to use dagNodes instead of steps - Remove WorkflowStepEvent/ParallelAgentEvent from dashboard SSE hook
joaobmonteiro
pushed a commit
to joaobmonteiro/Archon
that referenced
this pull request
Apr 26, 2026
…based retries (coleam00#729) * feat: safe session continuity across workflow steps with forkSession-based retries (coleam00#691) Retries previously discarded all prior conversation context by passing resumeSessionId=undefined, because naive session resume would append to the original session file and corrupt it on a crash. This adds forkSession:true support so the SDK copies the prior session history into a new file before appending — leaving the source untouched — making it safe to always pass the prior session ID on retry. Changes: - Add forkSession?: boolean to WorkflowAssistantOptions (deps.ts) and AssistantRequestOptions (types/index.ts) - Pass forkSession to SDK Options in ClaudeClient with enhanced resuming_session log - Sequential executor: set shouldForkSession=true when resuming; inject into stepOptions; fix retry to always pass resumeSessionId - DAG executor: set shouldForkSession=true when resuming in executeNodeInternal; fix retry to always pass resumeSessionId; update isFresh comment - Fix resolvedOptions to explicitly set persistSession:false for Claude workflows (was silently defaulting to SDK's true) - Add context:'shared' union to DagNodeBase.context type; update clearContext docstring Fixes coleam00#691 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove persistSession:false from workflow resolvedOptions to preserve cross-step session continuity SDK docs confirm that persistSession:false prevents session files from being written to disk, making resume impossible for subsequent steps. Removing this option restores the SDK default (true), so step 1's session ID can be used by step 2 to resume. Also clarify context:'shared' docstring to note it has no effect on parallel layer nodes, and add unit tests asserting that retries pass forkSession:true when a prior session exists. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: add persistSession: false to Claude resolvedOptions Implements the missing persistSession fix claimed in the PR description. Without this, the SDK defaults to true and writes session transcripts to disk. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(loader): preserve context: 'shared' in DAG node YAML parsing The PR added 'shared' to DagNodeBase.context type but the loader only preserved 'fresh', silently dropping 'shared' during YAML parsing. This meant context: shared in workflow YAML was ignored at load time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
joaobmonteiro
pushed a commit
to joaobmonteiro/Archon
that referenced
this pull request
Apr 26, 2026
…ole format (coleam00#805) * refactor(workflows)!: remove sequential execution mode, DAG becomes sole format Remove the steps-based (sequential) workflow execution mode entirely. All workflows now use the nodes-based (DAG) format exclusively. - Convert 8 sequential default workflows to DAG format - Delete archon-fix-github-issue sequential (DAG version absorbs triggers) - Remove SingleStep, ParallelBlock, StepWorkflow types and guards - Gut executor.ts from ~2200 to ~730 lines (remove sequential loop) - Remove step_started/completed/failed and parallel_agent_* events - Remove logStepStart/Complete and logParallelBlockStart/Complete - Delete SequentialEditor, StepProgress, ParallelBlockView components - Remove sequential mode from workflow builder and execution views - Delete executor.test.ts (4395 lines), update ~45 test fixtures - Update CLAUDE.md and docs to reflect DAG-only format BREAKING CHANGE: Workflows using `steps:` format are no longer supported. Convert to `nodes:` (DAG) format. The loader provides a clear error message directing users to the migration guide. * fix: address review findings — guard errors, remove dead code, add tests - Guard logNodeSkip/logWorkflowError against filesystem errors in dag-executor - Move mkdir(artifactsDir) inside try-catch with user-friendly error - Remove startFromStep dead parameter from executeWorkflow signature - Remove isDagWorkflow() tautology and all callers (20+ sites) - Remove dead BuilderMode/mode state from frontend components - Remove vestigial isLoop, selectedStep, stepIndex, step_index fields - Remove "DAG" prefix from user-facing resume/error messages - Fix 5 stale docs (README, getting-started, authoring-commands, web adapter) - Update event-emitter tests to use node events instead of removed step events - Add executor-shared.test.ts (12 tests) for substituteWorkflowVariables - Add executor.test.ts (11 tests) for concurrent-run, model resolution, resume * fix(workflows): add migration guide, port preamble tests, improve error message - Add docs/sequential-dag-migration-guide.md with 3 conversion patterns (single step, chain with clearContext, parallel block) and a Claude Code migration command for automated conversion - Update loader error message to point to migration guide and include ready-to-run claude command - Port 8 preamble tests from deleted executor.test.ts to new executor-preamble.test.ts: staleness detection (3), concurrent-run guard (3), DAG resume (2) Addresses review feedback from coleam00#805. * fix(workflows): update loader test to match new error message wording * fix: address review findings — fail stuck runs, remove dead code, fix docs - Mark workflow run as failed when artifacts mkdir fails (prevents 15-min concurrent-run guard block) - Remove vestigial totalSteps from WorkflowStartedEvent and executor - Delete dead WorkflowToolbar.tsx (369 lines, no importers) - Remove stepIndex prop from StepLogs (always 0, label now "Node logs") - Restore cn() in StatusBar for consistent conditional classes - Promote resume-check log to error, add errorType to failure logs - Remove ghost $PLAN/$IMPLEMENTATION_SUMMARY from docs (never implemented) - Update workflows.md rules to DAG-only format - Fix migration guide trigger_rule example - Clean up blank-line residues and stale comments * fix: resolve rebase conflicts with coleam00#729 (forkSession) and coleam00#730 (dashboard) - Remove sequential forkSession/persistSession code from coleam00#729 (dead after sequential removal) - Fix loader type narrowing for DagNode context field - Update dashboard components from coleam00#730 to use dagNodes instead of steps - Remove WorkflowStepEvent/ParallelAgentEvent from dashboard SSE hook
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
1 participant
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.
Summary
Reorganize hook structure to follow vertical slice architecture by moving truly shared hooks from
features/ui/hookstofeatures/shared/hooks.Changes Made
useSmartPolling,useThemeAware, anduseToasthooks tofeatures/shared/hooksui/hooksdirectoryType of Change
Affected Services
Testing
Test Evidence
Checklist
Breaking Changes
None - This is a pure refactoring with no API or functionality changes.
Additional Notes
This change aligns shared utilities with the architectural pattern where truly shared code resides in the shared directory, following the vertical slice architecture principles outlined in the project documentation.
Import path changes:
@/features/ui/hooks→@/features/shared/hooks🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Tests
Chores