Skip to content

Harden skills sync login and stash recovery #134

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.

Sign up for GitHub

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

Open
friuns2 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Harden skills sync login and stash recovery #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
da88442
Clarify device login repo permission errors
friuns May 4, 2026
1862aba
Guard skills sync reset behind stash checkpoint
friuns May 6, 2026
e1cc11d
Restore untracked skills after stash conflict
friuns May 6, 2026
aed6caa
Surface git diff failures in skills sync
friuns May 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 50 additions & 13 deletions src/server/skillsRoutes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -761,12 +761,24 @@ async function ensurePrivateForkFromUpstream(token: string, username: string, re
throw new Error(`Failed to check personal repo existence (${existing.status})`)
}

await getGithubJson(
'https://api.github.com/user/repos',
token,
'POST',
{ name: repoName, private: true, auto_init: false, description: 'Codex skills private mirror sync' },
)
const createRepo = await fetch('https://api.github.com/user/repos', {
method: 'POST',
headers: {
Accept: 'application/vnd.github+json',
'Content-Type': 'application/json',
Authorization: `Bearer ${token}`,
'X-GitHub-Api-Version': '2022-11-28',
'User-Agent': 'codex-web-local',
},
body: JSON.stringify({ name: repoName, private: true, auto_init: false, description: 'Codex skills private mirror sync' }),
})
if (!createRepo.ok) {
const text = await createRepo.text()
if (createRepo.status === 403 && text.includes('Resource not accessible by integration')) {
throw new Error(`GitHub login cannot create the private ${repoName} sync repo with this token. Create an empty private repo named ${repoName} on GitHub, then retry Device Login, or use the regular GitHub login button with repo access.`)
}
throw new Error(`GitHub API POST https://api.github.com/user/repos failed (${createRepo.status}): ${text}`)
}
created = true

let ready = false
Expand Down Expand Up @@ -903,10 +915,18 @@ async function ensureSkillsWorkingTreeRepo(repoUrl: string, branch: string): Pro
const hasLocalChangesBeforePull = await hasLocalUncommittedChanges(localDir)
const localMtimesBeforePull = hasLocalChangesBeforePull ? await snapshotFileMtimes(localDir) : new Map<string, number>()
let createdAutostash = false
let autostashRef = ''
try {
const stashOutput = await runCommandWithOutput('git', ['stash', 'push', '--include-untracked', '-m', 'codex-skills-autostash'], { cwd: localDir })
createdAutostash = !stashOutput.includes('No local changes to save')
} catch {}
if (createdAutostash) {
autostashRef = (await runCommandWithOutput('git', ['rev-parse', 'stash@{0}'], { cwd: localDir })).trim()
}
Comment on lines 920 to +924
Copy link
Copy Markdown

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Locale-dependent stash detection 🐞 Bug ☼ Reliability

ensureSkillsWorkingTreeRepo decides whether an autostash was created by substring-matching the
human-readable output "No local changes to save", which is not a stable signal and can misclassify
stash creation. Misclassification can incorrectly drive the later "stash pop"/conflict-recovery
path, obscuring the true repo state and failure mode.
Agent Prompt 
### Issue description
Autostash creation is detected by string-matching `git stash push` output (`"No local changes to save"`), which is not a reliable, machine-stable signal.

### Issue Context
`createdAutostash` gates whether the code later runs `git stash pop` and the associated recovery steps; if the detection is wrong, the recovery flow becomes misleading and may hide the real failure mode.

### Fix Focus Areas
- src/server/skillsRoutes.ts[915-943]

### Suggested change
Replace output parsing with a ref-based check:
- Capture the current top stash hash (or empty) before stashing: `git rev-parse -q --verify stash@{0}` (catch -> empty).
- Run `git stash push ...`.
- Re-read `git rev-parse -q --verify stash@{0}`; if it changed (or became non-empty when previously empty), then a new stash was created.
- Set `createdAutostash` and `autostashRef` based on this hash, not on human-readable output.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

} catch (error) {
if (hasLocalChangesBeforePull) {
throw new Error(`Refusing to reset skills repo because local changes could not be stashed first: ${getErrorMessage(error, 'git stash failed')}`)
}
}
let pulledMtimes = new Map<string, number>()
await runGitFetchWithRefLockRetry(localDir, ['fetch', 'origin', branch])
await runCommand('git', ['reset', '--hard', `origin/${branch}`], { cwd: localDir })
Expand All @@ -916,6 +936,9 @@ async function ensureSkillsWorkingTreeRepo(repoUrl: string, branch: string): Pro
await runCommand('git', ['stash', 'pop'], { cwd: localDir })
} catch {
await resolveStashPopConflictsByFileTime(localDir, localMtimesBeforePull, pulledMtimes)
if (autostashRef) {
await restoreMissingUntrackedFilesFromStash(localDir, autostashRef)
}
}
}
return localDir
Expand Down Expand Up @@ -1039,6 +1062,22 @@ async function resolveStashPopConflictsByFileTime(
}
}

async function restoreMissingUntrackedFilesFromStash(repoDir: string, stashRef: string): Promise<void> {
let untrackedFiles = ''
try {
untrackedFiles = await runCommandWithOutput('git', ['ls-tree', '-r', '--name-only', `${stashRef}^3`], { cwd: repoDir })
} catch {
return
}
for (const filePath of untrackedFiles.split(/\r?\n/).map((row) => row.trim()).filter(Boolean)) {
try {
await stat(join(repoDir, filePath))
continue
} catch {}
await runCommand('git', ['checkout', `${stashRef}^3`, '--', filePath], { cwd: repoDir })
}
}

async function snapshotFileMtimes(dir: string): Promise<Map<string, number>> {
const mtimes = new Map<string, number>()
await walkFileMtimes(dir, dir, mtimes)
Expand All @@ -1051,12 +1090,10 @@ async function hasLocalUncommittedChanges(repoDir: string): Promise<boolean> {
}

async function hasCommittableWorkingTreeChanges(repoDir: string): Promise<boolean> {
try {
await runCommand('git', ['diff', '--quiet', '--exit-code', '--ignore-submodules=dirty'], { cwd: repoDir })
await runCommand('git', ['diff', '--cached', '--quiet', '--exit-code', '--ignore-submodules=dirty'], { cwd: repoDir })
} catch {
return true
}
const unstaged = (await runCommandWithOutput('git', ['diff', '--name-only', '--ignore-submodules=dirty'], { cwd: repoDir })).trim()
if (unstaged.length > 0) return true
const staged = (await runCommandWithOutput('git', ['diff', '--cached', '--name-only', '--ignore-submodules=dirty'], { cwd: repoDir })).trim()
if (staged.length > 0) return true
const untracked = (await runCommandWithOutput('git', ['ls-files', '--others', '--exclude-standard'], { cwd: repoDir })).trim()
return untracked.length > 0
}
Expand Down
92 changes: 92 additions & 0 deletions tests.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,98 @@ Skills Sync skips unchanged manifest writes and does not fail parent commits whe

---

### Skills sync device login repo-create permission error

#### Feature/Change Name
Device Login shows an actionable message when GitHub rejects automatic private sync repo creation.

#### Prerequisites/Setup
1. Dev server running (`pnpm run dev --host 127.0.0.1 --port 5173`)
2. GitHub account used for Device Login does not already have a `codexskills` repository
3. GitHub returns `403 Resource not accessible by integration` for `POST /user/repos`
4. Light theme and dark theme are available from the appearance switcher

#### Steps
1. In light theme, open `#/skills`.
2. Click `Device Login`.
3. Complete the GitHub device-code prompt.
4. Wait for the sync panel to show the login failure.
5. Confirm the error explains that the token cannot create the private `codexskills` repo and tells the user to create an empty private repo named `codexskills` or use regular GitHub login with repo access.
6. Switch to dark theme and repeat steps 1 through 5.

#### Expected Results
- The raw `GitHub API POST https://api.github.com/user/repos failed (403)` payload is not shown as the primary error.
- The user sees a clear recovery path for Device Login.
- The error panel remains readable in light theme and dark theme.

#### Rollback/Cleanup
- Delete any test-only `codexskills` repository created during validation.

---

### Skills sync refuses hard reset when local checkpoint fails

#### Feature/Change Name
Skills Sync aborts before `reset --hard` if local changes cannot be stashed first.

#### Prerequisites/Setup
1. Dev server running (`pnpm run dev --host 127.0.0.1 --port 5173`)
2. GitHub Skills Sync is configured and connected
3. `/Users/igor/.codex/skills` has a local tracked or untracked test edit
4. A stash failure can be simulated in the skills repo
5. Light theme and dark theme are available from the appearance switcher

#### Steps
1. In light theme, open `#/skills`.
2. Create a local test edit under `/Users/igor/.codex/skills`.
3. Simulate `git stash push --include-untracked` failure for the skills repo.
4. Click `Pull` or `Startup Sync`.
5. Confirm sync fails before any hard reset and the local test edit is still present.
6. Switch to dark theme and repeat steps 1 through 5.

#### Expected Results
- Sync reports that local changes could not be stashed.
- Sync does not run the hard reset path after the failed stash.
- Local test edits remain recoverable in the skills repo.
- The error panel remains readable in light theme and dark theme.

#### Rollback/Cleanup
- Remove the local test edit and clear the simulated stash failure.

---

### Skills sync restores untracked files after stash-pop conflict

#### Feature/Change Name
Skills Sync restores untracked local skill files when `git stash pop` conflicts on tracked files.

#### Prerequisites/Setup
1. Dev server running (`pnpm run dev --host 127.0.0.1 --port 5173`)
2. GitHub Skills Sync is configured and connected
3. `/Users/igor/.codex/skills` has an untracked local skill folder such as `feedback-triage/SKILL.md`
4. Remote sync has a conflicting tracked edit such as a changed `installed-skills.json`
5. Light theme and dark theme are available from the appearance switcher

#### Steps
1. In light theme, open `#/skills`.
2. Create an untracked local skill folder under `/Users/igor/.codex/skills`.
3. Make remote `installed-skills.json` differ so `git stash pop` conflicts during pull/startup sync.
4. Click `Pull` or `Startup Sync`.
5. Confirm sync resolves the tracked conflict and restores the untracked skill file from the stash.
6. Confirm the restored skill file is included in the later skills repo commit/push path.
7. Switch to dark theme and repeat steps 1 through 5.

#### Expected Results
- Untracked local skill files do not remain stranded only in `refs/stash`.
- Restored untracked skill files are present in `/Users/igor/.codex/skills` after sync.
- The next parent repo commit can stage and push the restored skill files.
- The sync panel remains readable in light theme and dark theme.

#### Rollback/Cleanup
- Remove any test-only skill folder and restore remote manifest test edits.

---

### Header Git branch dropdown with commit reset

#### Feature/Change Name
Expand Down