fix(doctor): keep the session recovery marker out of tracked .sageox/ - #1072
Open
aditeya08varma wants to merge 1 commit into
Open
aditeya08varma wants to merge 1 commit into
aditeya08varma wants to merge 1 commit into
Conversation
Contributor
|
Warning Review limit reachedNext included review available in 21 minutes. View limit detailsLimit details: You’ve used the included review currently available. Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing. Review configuration: ⚙️ Run configurationConfiguration used: Repository: sageox/ox/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Fixes #1062
What broke
.sageox/.session-recovery.jsonholds a per-machine absoluteworkspace_path, and it was not in the managed.sageox/.gitignore.ox doctor(no--fix) stages the file itself.GetSageoxFilesToCommitmatches.sageox/*.json, andForceAddSageoxFilesrunsgit add -f, which overrides every ignore rule. So a gitignore entry alone, or a.git/info/excludeentry, does not stop it. That is why the file in the report sat staged even with a local exclude.What this PR ships
.session-recovery.jsontosageoxGitignoreContentandrequiredGitignoreEntries, next to the.needs-doctor*markers. Existing repos pick it up on their nextox doctor, the same wayagent_tasks/was rolled out.GetSageoxFilesToCommitskips any match whose name is inrequiredGitignoreEntries, so ox never force-stages a file its own gitignore ignores. Today this only changes behavior for this marker.requiredGitignoreEntriesas config to commit. If one is already in the index (the case in the report, where the marker was staged before this fix), doctor warns and says to unstage it withgit rm --cached <path>instead of "Run 'git commit' to persist config". It only gives the advice and does not touch the index, matching how doctor handles other index removals.git check-ignore .sageox/.session-recovery.jsonox doctorA .sageox/.session-recovery.json+ "run git commit" nudgegit rm --cached .sageox/.session-recovery.jsonWhy ignore it instead of moving it
Moving the file into
agent_instances/(option 1 in the issue) would leave existing copies at the current path, and doctor would keep force-staging them. Nothing reads or deletes the marker today, so they would never be cleaned up. Ignoring it plus the staging skip covers new and existing files, with no storage path change.Testing
TestSessionRecoveryMarker_NotStagedOrNudgedByDoctor: on a fresh repo, a plainox doctormust not stage the marker. Before the fix it fails withA .sageox/.session-recovery.json.TestSessionRecoveryMarker_IgnoredAfterDoctorUpgradesOldGitignore: a repo with the old gitignore gets the new rule.TestSessionRecoveryMarker_AlreadyStagedIsFlaggedNotNudgedToCommit: a marker already in the index must get the unstage advice. Before the fix it fails with"Run 'git commit' to persist config".-race -count=3, andmake testpass.Notes
git rm --cached .sageox/.session-recovery.json. Doctor shows that advice when the file is staged or modified. A committed copy that has not changed since does not show up ingit status, so doctor does not flag it..sageox/distill-state.jsonlooks like the same kind of per-machine file. I left it alone because I am not sure it is meant to be local.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.