mirror of
https://github.com/leonvanzyl/autocoder.git
synced 2026-03-17 02:43:09 +00:00
fix: handle pausing/draining states in UI guards and process cleanup
Follow-up fixes after merging PR #183 (graceful pause/drain mode): - process_manager: _stream_output finally block now transitions from pausing/paused_graceful to crashed/stopped (not just running), and cleans up the drain signal file on process exit - App.tsx: block Reset button and R shortcut during pausing/paused_graceful - AgentThought/ProgressDashboard: keep thought bubble visible while pausing - OrchestratorAvatar: add draining/paused cases to animation, glow, and description switch statements - AgentMissionControl: show Draining/Paused badge text for new states - registry.py: remove redundant type annotation to fix mypy no-redef - process_manager.py: add type:ignore for SQLAlchemy Column assignment - websocket.py: reclassify test-pass lines as 'testing' not 'success' - review-pr.md: add post-review recommended action guidance Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -73,3 +73,20 @@ Pull request(s): $ARGUMENTS
|
|||||||
- The key concerns, if any (or "no significant concerns")
|
- The key concerns, if any (or "no significant concerns")
|
||||||
- **Verdict: MERGE** / **MERGE (with minor follow-up)** / **DON'T MERGE** with a one-line reason
|
- **Verdict: MERGE** / **MERGE (with minor follow-up)** / **DON'T MERGE** with a one-line reason
|
||||||
- This section should be scannable in under 10 seconds
|
- This section should be scannable in under 10 seconds
|
||||||
|
|
||||||
|
10. **Post-Review Action**
|
||||||
|
- Immediately after the TLDR, provide a `## Recommended Action` section
|
||||||
|
- Based on the verdict, recommend one of the following actions:
|
||||||
|
|
||||||
|
**If verdict is MERGE (no concerns):**
|
||||||
|
- Recommend merging as-is. No further action needed.
|
||||||
|
|
||||||
|
**If verdict is MERGE (with minor follow-up):**
|
||||||
|
- If the concerns are low-risk and straightforward to fix (e.g., naming tweaks, small refactors, missing type annotations, minor style issues, trivial bug fixes), recommend merging the PR now and offer to immediately address the concerns in a follow-up commit directly on the target branch
|
||||||
|
- List the specific changes you would make in the follow-up
|
||||||
|
- Ask the user: *"Should I merge this PR and push a follow-up commit addressing these concerns?"*
|
||||||
|
|
||||||
|
**If verdict is DON'T MERGE:**
|
||||||
|
- If the blocking concerns are still relatively contained and you are confident you can resolve them quickly (e.g., a small bug fix, a missing validation, a straightforward architectural adjustment), recommend merging the PR and immediately addressing the issues in a follow-up commit — but only if the fixes are low-risk and well-understood
|
||||||
|
- If the issues are too complex, risky, or require author input (e.g., design decisions, major refactors, unclear intent), recommend sending the PR back to the author with specific feedback on what needs to change
|
||||||
|
- Be honest about your confidence level — if you're unsure whether you can address the concerns correctly, say so and defer to the author
|
||||||
@@ -743,7 +743,7 @@ def get_effective_sdk_env() -> dict[str, str]:
|
|||||||
sdk_env[var] = value
|
sdk_env[var] = value
|
||||||
return sdk_env
|
return sdk_env
|
||||||
|
|
||||||
sdk_env: dict[str, str] = {}
|
sdk_env = {}
|
||||||
|
|
||||||
# Explicitly clear credentials that could leak from the server process env.
|
# Explicitly clear credentials that could leak from the server process env.
|
||||||
# For providers using ANTHROPIC_AUTH_TOKEN (GLM, Custom), clear ANTHROPIC_API_KEY.
|
# For providers using ANTHROPIC_AUTH_TOKEN (GLM, Custom), clear ANTHROPIC_API_KEY.
|
||||||
|
|||||||
@@ -277,7 +277,7 @@ class AgentProcessManager:
|
|||||||
).all()
|
).all()
|
||||||
if stuck:
|
if stuck:
|
||||||
for f in stuck:
|
for f in stuck:
|
||||||
f.in_progress = False
|
f.in_progress = False # type: ignore[assignment]
|
||||||
session.commit()
|
session.commit()
|
||||||
logger.info(
|
logger.info(
|
||||||
"Cleaned up %d stuck feature(s) for %s",
|
"Cleaned up %d stuck feature(s) for %s",
|
||||||
@@ -346,7 +346,7 @@ class AgentProcessManager:
|
|||||||
# Check if process ended
|
# Check if process ended
|
||||||
if self.process and self.process.poll() is not None:
|
if self.process and self.process.poll() is not None:
|
||||||
exit_code = self.process.returncode
|
exit_code = self.process.returncode
|
||||||
if exit_code != 0 and self.status == "running":
|
if exit_code != 0 and self.status in ("running", "pausing", "paused_graceful"):
|
||||||
# Check buffered output for auth errors if we haven't detected one yet
|
# Check buffered output for auth errors if we haven't detected one yet
|
||||||
if not auth_error_detected:
|
if not auth_error_detected:
|
||||||
combined_output = '\n'.join(output_buffer)
|
combined_output = '\n'.join(output_buffer)
|
||||||
@@ -354,10 +354,16 @@ class AgentProcessManager:
|
|||||||
for help_line in AUTH_ERROR_HELP.strip().split('\n'):
|
for help_line in AUTH_ERROR_HELP.strip().split('\n'):
|
||||||
await self._broadcast_output(help_line)
|
await self._broadcast_output(help_line)
|
||||||
self.status = "crashed"
|
self.status = "crashed"
|
||||||
elif self.status == "running":
|
elif self.status in ("running", "pausing", "paused_graceful"):
|
||||||
self.status = "stopped"
|
self.status = "stopped"
|
||||||
self._cleanup_stale_features()
|
self._cleanup_stale_features()
|
||||||
self._remove_lock()
|
self._remove_lock()
|
||||||
|
# Clean up drain signal file if present
|
||||||
|
try:
|
||||||
|
from autoforge_paths import get_pause_drain_path
|
||||||
|
get_pause_drain_path(self.project_dir).unlink(missing_ok=True)
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
|
||||||
async def start(
|
async def start(
|
||||||
self,
|
self,
|
||||||
|
|||||||
@@ -61,7 +61,7 @@ THOUGHT_PATTERNS = [
|
|||||||
(re.compile(r'(?:Testing|Verifying|Running tests|Validating)\s+(.+)', re.I), 'testing'),
|
(re.compile(r'(?:Testing|Verifying|Running tests|Validating)\s+(.+)', re.I), 'testing'),
|
||||||
(re.compile(r'(?:Error|Failed|Cannot|Unable to|Exception)\s+(.+)', re.I), 'struggling'),
|
(re.compile(r'(?:Error|Failed|Cannot|Unable to|Exception)\s+(.+)', re.I), 'struggling'),
|
||||||
# Test results
|
# Test results
|
||||||
(re.compile(r'(?:PASS|passed|success)', re.I), 'success'),
|
(re.compile(r'(?:PASS|passed|success)', re.I), 'testing'),
|
||||||
(re.compile(r'(?:FAIL|failed|error)', re.I), 'struggling'),
|
(re.compile(r'(?:FAIL|failed|error)', re.I), 'struggling'),
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|||||||
@@ -210,8 +210,8 @@ function App() {
|
|||||||
setShowKeyboardHelp(true)
|
setShowKeyboardHelp(true)
|
||||||
}
|
}
|
||||||
|
|
||||||
// R : Open reset modal (when project selected and agent not running)
|
// R : Open reset modal (when project selected and agent not running/draining)
|
||||||
if ((e.key === 'r' || e.key === 'R') && selectedProject && wsState.agentStatus !== 'running') {
|
if ((e.key === 'r' || e.key === 'R') && selectedProject && !['running', 'pausing', 'paused_graceful'].includes(wsState.agentStatus)) {
|
||||||
e.preventDefault()
|
e.preventDefault()
|
||||||
setShowResetModal(true)
|
setShowResetModal(true)
|
||||||
}
|
}
|
||||||
@@ -380,7 +380,7 @@ function App() {
|
|||||||
variant="outline"
|
variant="outline"
|
||||||
size="sm"
|
size="sm"
|
||||||
aria-label="Reset Project"
|
aria-label="Reset Project"
|
||||||
disabled={wsState.agentStatus === 'running'}
|
disabled={['running', 'pausing', 'paused_graceful'].includes(wsState.agentStatus)}
|
||||||
>
|
>
|
||||||
<RotateCcw size={18} />
|
<RotateCcw size={18} />
|
||||||
</Button>
|
</Button>
|
||||||
|
|||||||
@@ -72,6 +72,10 @@ export function AgentMissionControl({
|
|||||||
? `${agents.length} ${agents.length === 1 ? 'agent' : 'agents'} active`
|
? `${agents.length} ${agents.length === 1 ? 'agent' : 'agents'} active`
|
||||||
: orchestratorStatus?.state === 'initializing'
|
: orchestratorStatus?.state === 'initializing'
|
||||||
? 'Initializing'
|
? 'Initializing'
|
||||||
|
: orchestratorStatus?.state === 'draining'
|
||||||
|
? 'Draining'
|
||||||
|
: orchestratorStatus?.state === 'paused'
|
||||||
|
? 'Paused'
|
||||||
: orchestratorStatus?.state === 'complete'
|
: orchestratorStatus?.state === 'complete'
|
||||||
? 'Complete'
|
? 'Complete'
|
||||||
: 'Orchestrating'
|
: 'Orchestrating'
|
||||||
|
|||||||
@@ -63,7 +63,7 @@ export function AgentThought({ logs, agentStatus }: AgentThoughtProps) {
|
|||||||
// Determine if component should be visible
|
// Determine if component should be visible
|
||||||
const shouldShow = useMemo(() => {
|
const shouldShow = useMemo(() => {
|
||||||
if (!thought) return false
|
if (!thought) return false
|
||||||
if (agentStatus === 'running') return true
|
if (agentStatus === 'running' || agentStatus === 'pausing') return true
|
||||||
if (agentStatus === 'paused') {
|
if (agentStatus === 'paused') {
|
||||||
return Date.now() - lastLogTimestamp < IDLE_TIMEOUT
|
return Date.now() - lastLogTimestamp < IDLE_TIMEOUT
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -103,6 +103,10 @@ function getStateAnimation(state: OrchestratorState): string {
|
|||||||
return 'animate-working'
|
return 'animate-working'
|
||||||
case 'monitoring':
|
case 'monitoring':
|
||||||
return 'animate-bounce-gentle'
|
return 'animate-bounce-gentle'
|
||||||
|
case 'draining':
|
||||||
|
return 'animate-thinking'
|
||||||
|
case 'paused':
|
||||||
|
return ''
|
||||||
case 'complete':
|
case 'complete':
|
||||||
return 'animate-celebrate'
|
return 'animate-celebrate'
|
||||||
default:
|
default:
|
||||||
@@ -121,6 +125,10 @@ function getStateGlow(state: OrchestratorState): string {
|
|||||||
return 'shadow-[0_0_16px_rgba(124,58,237,0.6)]'
|
return 'shadow-[0_0_16px_rgba(124,58,237,0.6)]'
|
||||||
case 'monitoring':
|
case 'monitoring':
|
||||||
return 'shadow-[0_0_8px_rgba(167,139,250,0.4)]'
|
return 'shadow-[0_0_8px_rgba(167,139,250,0.4)]'
|
||||||
|
case 'draining':
|
||||||
|
return 'shadow-[0_0_10px_rgba(251,191,36,0.5)]'
|
||||||
|
case 'paused':
|
||||||
|
return ''
|
||||||
case 'complete':
|
case 'complete':
|
||||||
return 'shadow-[0_0_20px_rgba(112,224,0,0.6)]'
|
return 'shadow-[0_0_20px_rgba(112,224,0,0.6)]'
|
||||||
default:
|
default:
|
||||||
@@ -141,6 +149,10 @@ function getStateDescription(state: OrchestratorState): string {
|
|||||||
return 'spawning agents'
|
return 'spawning agents'
|
||||||
case 'monitoring':
|
case 'monitoring':
|
||||||
return 'monitoring progress'
|
return 'monitoring progress'
|
||||||
|
case 'draining':
|
||||||
|
return 'draining active agents'
|
||||||
|
case 'paused':
|
||||||
|
return 'paused'
|
||||||
case 'complete':
|
case 'complete':
|
||||||
return 'all features complete'
|
return 'all features complete'
|
||||||
default:
|
default:
|
||||||
|
|||||||
@@ -55,7 +55,7 @@ export function ProgressDashboard({
|
|||||||
|
|
||||||
const showThought = useMemo(() => {
|
const showThought = useMemo(() => {
|
||||||
if (!thought) return false
|
if (!thought) return false
|
||||||
if (agentStatus === 'running') return true
|
if (agentStatus === 'running' || agentStatus === 'pausing') return true
|
||||||
if (agentStatus === 'paused') {
|
if (agentStatus === 'paused') {
|
||||||
return Date.now() - lastLogTimestamp < IDLE_TIMEOUT
|
return Date.now() - lastLogTimestamp < IDLE_TIMEOUT
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user