-
Notifications
You must be signed in to change notification settings - Fork 5
fix(loop-state): change state files from .md to .state extension #111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -32,7 +32,7 @@ set -euo pipefail | |||||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" || exit | ||||||
| readonly SCRIPT_DIR | ||||||
| readonly STATE_DIR=".agent/loop-state" | ||||||
| readonly STATE_FILE="${STATE_DIR}/quality-loop.local.md" | ||||||
| readonly STATE_FILE="${STATE_DIR}/quality-loop.local.state" | ||||||
|
|
||||||
| # Legacy state directory (for backward compatibility during migration) | ||||||
| # shellcheck disable=SC2034 # Defined for documentation | ||||||
|
|
@@ -228,7 +228,7 @@ get_pending_checks() { | |||||
| # $2 - Max iterations | ||||||
| # $3 - Options string (key=value pairs separated by commas) | ||||||
| # Returns: 0 | ||||||
| # Side effects: Creates .agent/loop-state/quality-loop.local.md | ||||||
| # Side effects: Creates .agent/loop-state/quality-loop.local.state | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of hardcoding the filename in this comment, it's better to refer to the
Suggested change
|
||||||
| create_state() { | ||||||
| local loop_type="$1" | ||||||
| local max_iterations="$2" | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -44,12 +44,12 @@ fi | |||||||||
|
|
||||||||||
| # State directories | ||||||||||
| readonly RALPH_STATE_DIR=".agent/loop-state" | ||||||||||
| readonly RALPH_STATE_FILE="${RALPH_STATE_DIR}/ralph-loop.local.md" | ||||||||||
| readonly RALPH_STATE_FILE="${RALPH_STATE_DIR}/ralph-loop.local.state" | ||||||||||
|
|
||||||||||
| # Legacy state directory (for backward compatibility during migration) | ||||||||||
| readonly RALPH_LEGACY_STATE_DIR=".claude" | ||||||||||
| # shellcheck disable=SC2034 # Defined for documentation, used in status checks | ||||||||||
| readonly RALPH_LEGACY_STATE_FILE="${RALPH_LEGACY_STATE_DIR}/ralph-loop.local.md" | ||||||||||
| readonly RALPH_LEGACY_STATE_FILE="${RALPH_LEGACY_STATE_DIR}/ralph-loop.local.state" | ||||||||||
|
|
||||||||||
| # Adaptive timing constants (evidence-based from PR #19 analysis) | ||||||||||
| readonly RALPH_DELAY_BASE="${RALPH_DELAY_BASE:-2}" | ||||||||||
|
|
@@ -599,8 +599,8 @@ show_status_all() { | |||||||||
| # Check for v2 state (new location first, then legacy) | ||||||||||
| local v2_state="$worktree_path/.agent/loop-state/loop-state.json" | ||||||||||
| local v2_state_legacy="$worktree_path/.claude/loop-state.json" | ||||||||||
| local legacy_state="$worktree_path/.agent/loop-state/ralph-loop.local.md" | ||||||||||
| local legacy_state_old="$worktree_path/.claude/ralph-loop.local.md" | ||||||||||
| local legacy_state="$worktree_path/.agent/loop-state/ralph-loop.local.state" | ||||||||||
| local legacy_state_old="$worktree_path/.claude/ralph-loop.local.state" | ||||||||||
|
Comment on lines
+602
to
+603
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The file paths are being reconstructed here, but you have
Suggested change
|
||||||||||
|
|
||||||||||
| # Check any of the state file locations | ||||||||||
| if [[ -f "$v2_state" ]] || [[ -f "$v2_state_legacy" ]] || [[ -f "$legacy_state" ]] || [[ -f "$legacy_state_old" ]]; then | ||||||||||
|
|
@@ -671,8 +671,8 @@ check_other_loops() { | |||||||||
| # Check all possible state file locations (new and legacy) | ||||||||||
| local v2_state="$worktree_path/.agent/loop-state/loop-state.json" | ||||||||||
| local v2_state_legacy="$worktree_path/.claude/loop-state.json" | ||||||||||
| local legacy_state="$worktree_path/.agent/loop-state/ralph-loop.local.md" | ||||||||||
| local legacy_state_old="$worktree_path/.claude/ralph-loop.local.md" | ||||||||||
| local legacy_state="$worktree_path/.agent/loop-state/ralph-loop.local.state" | ||||||||||
| local legacy_state_old="$worktree_path/.claude/ralph-loop.local.state" | ||||||||||
|
Comment on lines
+674
to
+675
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the
Suggested change
|
||||||||||
|
|
||||||||||
| if [[ -f "$v2_state" ]] || [[ -f "$v2_state_legacy" ]] || [[ -f "$legacy_state" ]] || [[ -f "$legacy_state_old" ]]; then | ||||||||||
| local branch | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -90,8 +90,8 @@ get_todo_status() { | |
| get_ralph_status() { | ||
| local project_root="$1" | ||
| # Check new location first, then legacy | ||
| local ralph_file="$project_root/.agent/loop-state/ralph-loop.local.md" | ||
| local ralph_file_legacy="$project_root/.claude/ralph-loop.local.md" | ||
| local ralph_file="$project_root/.agent/loop-state/ralph-loop.local.state" | ||
| local ralph_file_legacy="$project_root/.claude/ralph-loop.local.state" | ||
|
Comment on lines
+93
to
+94
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| local active_file="" | ||
| [[ -f "$ralph_file" ]] && active_file="$ralph_file" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -103,8 +103,8 @@ epoch_to_date() { | |
| get_ralph_loop_status() { | ||
| local worktree_path="$1" | ||
| # Check new location first, then legacy | ||
| local state_file="$worktree_path/.agent/loop-state/ralph-loop.local.md" | ||
| local state_file_legacy="$worktree_path/.claude/ralph-loop.local.md" | ||
| local state_file="$worktree_path/.agent/loop-state/ralph-loop.local.state" | ||
| local state_file_legacy="$worktree_path/.claude/ralph-loop.local.state" | ||
|
Comment on lines
+106
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| local active_file="" | ||
| [[ -f "$state_file" ]] && active_file="$state_file" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -231,7 +231,7 @@ migrate_loop_state_directories() { | |
|
|
||
| # Check for loop state files in old location | ||
| local has_loop_state=false | ||
| if [[ -f "$old_state_dir/ralph-loop.local.md" ]] || \ | ||
| if [[ -f "$old_state_dir/ralph-loop.local.state" ]] || \ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| [[ -f "$old_state_dir/loop-state.json" ]] || \ | ||
| [[ -d "$old_state_dir/receipts" ]]; then | ||
| has_loop_state=true | ||
|
|
@@ -245,7 +245,7 @@ migrate_loop_state_directories() { | |
| mkdir -p "$new_state_dir" | ||
|
|
||
| # Move loop-related files | ||
| for file in ralph-loop.local.md loop-state.json re-anchor.md guardrails.md; do | ||
| for file in ralph-loop.local.state loop-state.json re-anchor.md guardrails.md; do | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This migration logic appears incomplete and could lead to loss of user loop state.
A more robust migration would be to explicitly check for old # Files to move and rename from .md to .state
local files_to_rename=("ralph-loop.local" "quality-loop.local" "full-loop.local")
for base_name in "${files_to_rename[@]}"; do
if [[ -f "$old_state_dir/${base_name}.md" ]]; then
mv "$old_state_dir/${base_name}.md" "$new_state_dir/${base_name}.state"
print_info " Moved and renamed ${base_name}.md"
fi
done
# Move other files that don't need renaming
for file in loop-state.json re-anchor.md guardrails.md; do
if [[ -f "$old_state_dir/$file" ]]; then
mv "$old_state_dir/$file" "$new_state_dir/"
print_info " Moved $file"
fi
done |
||
| if [[ -f "$old_state_dir/$file" ]]; then | ||
| mv "$old_state_dir/$file" "$new_state_dir/" | ||
| print_info " Moved $file" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These file paths for sub-loops are hardcoded, and a similar issue exists in
cmd_resume. To improve maintainability and avoid magic strings, it would be best to define them asreadonlyconstants at the top of the script. This would centralize the paths and make future changes easier.For example, at the top of the script:
Then these constants could be used here and in
cmd_resume.