Replace the always-on runtime-backup service with the opt-in github-sync skill#259
Merged
Conversation
…ync skill GitHub sync is now opt-in: the runtime-backup supervisord service, bootstrap's runtime/ worktree init (mindsbackup/$MNGR_AGENT_ID), the always-installed core.hooksPath post-commit auto-push, and all GH_TOKEN usage are removed from the default template. The new github-sync skill enables sync on request: it creates a dedicated PRIVATE GitHub repo through latchkey (no token in the container), points origin at it, wires plain git push through the latchkey gateway for every checkout (per-VPS secondary gateway as offline fallback), installs the post-commit hook so agent and worker commits auto-push, and adds a [program:github-sync] service that syncs runtime/ to the stable runtime-sync orphan branch. libs/runtime_backup is renamed and extended into libs/github_sync. The service re-verifies repo visibility and halts all pushes when the repo is public or unverifiable. Workspaces recreated from a synced repo self-heal once permissions are re-granted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: scripts/git_hooks/post-commit read the halt flag with jq -r '.is_push_allowed // true'. jq's // operator falls through to its right side when the left side is false OR null, so an explicit "is_push_allowed": false evaluated to true and the hook kept auto-pushing branches while the github-sync service had halted pushes (sync repo public or unverifiable), defeating the private-only enforcement. Fix: read the field with jq -r '.is_push_allowed' and compare against "false" (missing file/field reads as null/empty, preserving the intended default-allow before the service's first report). Made the hook's config/status/log paths env-overridable (production defaults unchanged; GITHUB_SYNC_STATUS_FILE matches the override the runner already honors) and added hook tests in libs/github_sync, including a regression test proven to fail against the old jq expression. The github_sync time.sleep ratchet count is bumped 1 -> 2 for the bounded condition-poll awaiting the hook's intentionally-backgrounded push. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: libs/github_sync/README.md and the runner module docstring said pushes are refused whenever the repo's visibility "cannot be confirmed", but _refresh_visibility deliberately keeps the last confirmed answer when a re-check fails outright (retrying every tick), so a prior private confirmation is not invalidated by e.g. an offline gateway. Fix: reworded both to state the exact semantics -- pushes held until the first confirmed-private answer, halted whenever the repo is confirmed public, and a failed re-check retains the last confirmed answer (in that failure mode pushes would fail too, since they use the same gateway). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: scripts/git_hooks/post-commit composed the secondary-gateway push URL by appending .git to the raw repo_url extracted from github_sync.toml. A repo_url written with a trailing .git suffix or trailing slash (which github_sync.config.load_repo_url normalizes and therefore the Python side accepts) produced a broken URL like .../repo.git.git, silently breaking the offline-primary fallback. Fix: strip a trailing slash and .git suffix after extraction, mirroring load_repo_url's normalization; a no-op for the canonical bare form the skill writes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: deleting the _create_orphan_runtime_worktree test section at the end of libs/bootstrap/src/bootstrap/manager_test.py left two trailing blank lines, so the file was no longer ruff-format clean. Fix: stripped the trailing blank lines (file now passes ruff format --check). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: runner._write_status wrote /tmp/github-sync-status.json with a truncate-then-write (Path.write_text) while the post-commit hook reads the same file with jq on every commit to honor a visibility halt. A read landing mid-write saw truncated JSON, jq failed silently, and the hook defaulted to allowing the push -- defeating the security halt during the recurring 60s rewrite window. Fix: write the payload to a sibling .tmp file and os.replace it into place (the same atomic-write pattern libs/browser/src/browser/manifest.py uses), so the hook always reads either the old or the new complete file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: load_repo_url in libs/github_sync/src/github_sync/config.py stripped the .git suffix before the trailing slash, so a repo_url written as 'https://github.com/u/repo.git/' kept its .git suffix; the post-commit hook normalizes the same config value in the opposite (correct) order, so the service and hook could disagree on the remote URL and the visibility check could query a wrong repo name. Fix: strip the trailing slash first, then the .git suffix, matching the hook's sed normalization; extended the normalization unit test with the combined '.git/' case as a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: the hook header said all four skip cases are silent and normal, but case 4 (the github-sync visibility halt) writes a log line and is a security condition, not a normal state. Fix: reword the comment to distinguish the silent normal skips (1-3) from the logged security halt (4). Comment-only; no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: every _do_tick test pre-created the runtime worktree, leaving the recreated-workspace self-heal branch of runner._do_tick untested (restore runtime/ from origin's runtime-sync branch when the worktree is missing, and defer with a status error when origin is unreachable). Fix: add two runner tests driving that branch through a full tick -- one restoring prior runtime state from a local bare origin and pushing, one asserting the deferred-init error when origin cannot be reached. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: load_repo_url only checked the https://github.com/ prefix, so a github_sync.toml with e.g. repo_url = "https://github.com/owner-only" loaded fine and the GithubSyncConfigError was raised later by parse_owner_and_name inside check_repo_visibility. runner._do_tick only guards load_repo_url, so the error escaped the tick and crash-looped the supervised service (and made check-visibility traceback) instead of taking the graceful config-error status path. Fix: load_repo_url now validates the full <owner>/<repo> shape via parse_owner_and_name after normalizing, so every malformed-config case raises at the single load point that all callers already handle; added a unit test covering missing and extra path segments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: _refresh_visibility's re-check policy -- a confirmed answer is cached for VISIBILITY_CHECK_INTERVAL_SECONDS, and a stale answer whose re-check fails outright keeps the last confirmed answer with checked_at unchanged so it retries next tick -- had no direct tests; only the first-check-from-scratch states were covered, so a regression in either behavior would pass the suite. Fix: add three _refresh_visibility unit tests: a fresh private confirmation is trusted without re-asking GitHub, a stale confirmation whose re-check fails keeps pushes enabled and retries next tick, and a stale confirmation that re-checks to public halts pushes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: the visibility.py module docstring said pushes halt whenever the repo's visibility cannot be established, but the implemented policy (and the README / runner docs) keep the last confirmed answer when a re-check fails outright -- only the initial establishment and a confirmed-public answer block pushes. Fix: reworded the docstring to describe the real policy and point at runner._refresh_visibility. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… else Problem: the if/elif chain that configures the branch after a successful worktree add in libs/github_sync/src/github_sync/worktree.py had no else clause, leaving the reattach-to-existing-local-branch no-op case implicit; style_guide.md requires every if/elif chain to end with an else. Fix: added an else branch with a pass and a comment explaining why the reattach case needs no setup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: init_runtime_worktree named its two parallel branch-existence booleans asymmetrically (has_remote_branch vs local_branch_exists), and the style guide's prefixed-boolean convention favors the has_/is_ shape. Fix: renamed the function-local variable to has_local_branch to mirror has_remote_branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: six files in libs/github_sync were not clean under the formatter the pre-commit hook enforces (uvx ruff format), so any future commit touching them would produce noise reformat diffs. Fix: ran the hook's exact ruff check --fix and ruff format commands over libs/github_sync; the changes are wrapping and quote-style only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: _do_tick in libs/github_sync/src/github_sync/runner.py carried the previous repo's confirmed-private visibility answer across a github_sync.toml repo_url change, so a repointed config could receive pushes for up to 15 minutes without the new repo's visibility ever being confirmed -- violating the per-repo pushes-held-until-confirmed- private invariant. Fix: detect a repo_url change in _do_tick and reset visibility to UNKNOWN (clearing visibility_checked_at) so the swapped-in repo must earn its own confirmed-private answer; added a regression test proving a fresh confirmation for a different repo no longer authorizes pushes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: the comment above _GIT_GLOBAL_CONFIG_ARGVS in libs/bootstrap/src/bootstrap/manager.py implied that the ssh->https rewrite's output is then routed through github-sync's gateway wiring, but git applies at most one insteadOf rewrite per URL (verified empirically) -- ssh-form remotes end at plain https and never reach the gateway. Fix: reword the comment to state the non-chaining behavior explicitly and note that only remotes stored as https URLs (the shape the skill always configures) route through the gateway. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: .agents/skills/use-inspiration/SKILL.md (section 7) still said 'the post-commit hook handles any push', implying the auto-push hook is always active, but on this branch the hook is inert until the opt-in github-sync skill wires it up. Fix: reword the parenthetical to condition the push on GitHub sync being enabled, matching the branch-wide doc sweep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: .agents/skills/show-files-in-chat/SKILL.md said runtime/chat-images/ 'is backed up along with the rest of runtime/', phrasing written for the removed always-on runtime-backup service; runtime/ git sync is now opt-in. Fix: reword to say the directory persists with runtime/ and is covered by the opt-in GitHub sync when enabled, matching the branch-wide doc sweep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: the hook's background push attempts (plain push, --set-upstream fallback, secondary-gateway fallback) in scripts/git_hooks/post-commit ran without GIT_TERMINAL_PROMPT=0. A push hitting an auth challenge (e.g. a 401 from a lapsed latchkey gateway permission) would make git prompt for a username on the controlling TTY, writing a stray prompt into the committing agent's tmux session and hanging the background job forever. The hook's own test already had to inject GIT_TERMINAL_PROMPT=0 to guard against this, and the Python-side git helpers (github_sync.worktree._git_noninteractive_env) set it for the same reason. Fix: export GIT_TERMINAL_PROMPT=0 at the top of the hook. Credentials are injected server-side via the gateway headers, so a prompt can never lead to a successful push; disabling it turns the hang into a fast failure recorded in the hook log. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem: init_runtime_worktree wrote the secrets-excluding .gitignore only on the fresh-orphan path. _create_orphan_runtime_worktree creates the runtime-sync branch (pointing at the bare empty-tree init commit) before its final `git worktree add`; if that add failed, the retry reattached to a branch with no .gitignore in its history, and the next sync tick's `git add -A` would have staged and pushed runtime/secrets (Cloudflare tunnel token, restic credentials) to the GitHub remote. Fix: after any successful worktree add, write and commit the .gitignore whenever it is missing (never clobbering an existing one), instead of only on the fresh-orphan path. Added a regression test that reattaches to an interrupted-init branch and asserts the exclusion is in place and committed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
roughly lgtm remaining:
|
…tion
The enable flow's REST-API permission ask requested github-write-repos, but
the latchkey gateway scopes that (and github-read-repos) only to
/repos/{owner}/{repo} paths -- existing-repo operations. Repo creation via
POST /user/repos requires github-write-all, so the old ask forced a third,
avoidable approval prompt mid-flow. Request github-write-all in the same
combined ask (keeping github-read-repos for the least-privilege visibility
read via GET /repos/{owner}/{repo}), and note at the repo-creation step why
the broader permission is needed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Live-testing the enable flow surfaced two skill bugs that made the user
approve GitHub access three times instead of one round.
1. Repo creation needs github-write-all (fixed in the previous commit):
github-write-repos only covers existing-repo /repos/{owner}/{repo}
paths, so POST /user/repos was rejected after the user had already
approved, forcing a third mid-flow request.
2. The agent probed 'GET /user' to check whether the grants had landed,
but that endpoint needs github-read-user, which the skill never
requested. The probe therefore returned "Request not permitted"
even after both grants had been approved, so the agent concluded the
approval had not arrived and nagged the user to approve it again.
Fix: request the exact set the flow needs in one up-front round
(github-read-user + github-read-repos + github-write-all on the REST
scope; read/write on the git scope), document what each permission is
for, and state plainly that a rejected endpoint means that endpoint is
out of scope -- not that a grant is missing -- with GET /user named as
the one legitimate post-grant probe. Also drop the stale disable-path
caveat that repo deletion needs an extra permission (github-write-all
covers it) and use the create response's full_name as the authoritative
owner/repo.
Two approval dialogs remain the floor: a latchkey permission request
carries exactly one scope, and GitHub's git and REST access are separate
scopes. Both are now fired back-to-back before any GitHub call, so the
user approves once and the flow runs to completion without returning.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Implements the plan in imbue-ai/mngr#2439 (blueprint/github-sync-skill).
github-syncskill: creates a dedicated PRIVATE GitHub repo via latchkey, points origin at it, wires plaingit pushthrough the latchkey gateway for every checkout (secondary per-VPS gateway as offline fallback), activates the post-commit hook, and adds a[program:github-sync]service syncing runtime/ to the stableruntime-syncbranch.libs/runtime_backuprenamed + extended intolibs/github_sync; visibility is re-verified periodically and pushes halt while the repo is public or unverifiable.🤖 Generated with Claude Code