Skip to content

feat: add POST /bible/recover endpoint (homelab#176) - #69

Merged
dvystrcil merged 1 commit into
mainfrom
feat/bible-recover-endpoint
Aug 2, 2026
Merged

feat: add POST /bible/recover endpoint (homelab#176)#69
dvystrcil merged 1 commit into
mainfrom
feat/bible-recover-endpoint

Conversation

@dvystrcil

@dvystrcil dvystrcil commented Aug 2, 2026

Copy link
Copy Markdown
Owner

User description

Summary

  • Adds POST /bible/recover — the destructive-reset auto-recovery path for the residual bible_bridge wedge case that _ensure_on_default_branch/git_pull can't fix (local main diverged from origin/main at the SHA level, needs git reset --hard).
  • confirm: true required (footgun-guard, endpoint is destructive). Refuses on uncommitted changes (no auto-data-loss).
  • Returns a diagnostic payload (abandoned branch/SHA/dropped commits, new HEAD) so the homelab#123 health-watcher n8n workflow can post a meaningful alert.
  • AC1-AC5 from homelab#176.

Test plan

  • python3 -m pytest tests/test_bible_recover.py -v — 5/5 passed, using real git repos (a local bare "origin" + real clone), not mocks
  • Covers: confirm-required refusal, uncommitted-changes refusal, happy-path divergence recovery (verifies filesystem state + report match), recovery from a non-default branch, no-op success when already in sync

PR Type

Feature, New Endpoint


Description

  • Introduces /bible/recover POST endpoint to safely resolve diverged local branches via git reset --hard origin/main.

  • Implements git_recover() with strict safety guards: requires explicit confirm=True and refuses if uncommitted changes exist.

  • Returns detailed payload reporting abandoned commits, dropped SHAs, and new HEAD state.

  • Adds comprehensive integration tests using real git repositories to validate recovery logic, safety checks, and edge cases.


Diagram Walkthrough

flowchart TD
  A[POST /bible/recover] --> B{confirm=True?}
  B -- No --> C[400 confirm_required]
  B -- Yes --> D{Uncommitted changes?}
  D -- Yes --> E[400 uncommitted_changes_present]
  D -- No --> F[Fetch origin/main]
  F --> G[Checkout main if needed]
  G --> H[git reset --hard origin/main]
  H --> I{Reset success?}
  I -- No --> J[500 reset_failed]
  I -- Yes --> K[200 Recovery payload]
Loading

File Walkthrough

Relevant files
Feature
bible_bridge.py
Implement git recovery endpoint and core logic                     

bible_bridge.py

Added git_recover() function and /bible/recover POST route with safety
validations, fetch/reset logic, and detailed response payloads.

Test
test_bible_recover.py
Add comprehensive tests for /bible/recover                             

tests/test_bible_recover.py

Added full test suite covering confirm guards, dirty tree rejection,
happy path divergence, non-default branches, and no-op scenarios using
real git repos.

+155/-0 

Closes the residual bible_bridge wedge case _ensure_on_default_branch
can't fix: local main has diverged from origin/main at the SHA level
(post-squash-merge or rebased-force-push), which needs a destructive
git reset --hard that the bridge correctly refuses to do silently.

git_recover() requires confirm:true (footgun-guard) and refuses on
uncommitted changes (no auto-data-loss), matching the existing
_ensure_on_default_branch pattern. Returns a diagnostic payload
(abandoned branch/SHA/dropped commits, new HEAD) so callers (the
homelab#123 health-watcher workflow) can post a meaningful alert.

Tests use real git repos (a local bare "origin" + a real clone) rather
than mocking subprocess calls, per feedback_mocks_encode_assumed_contracts
-- git_recover's entire job is orchestrating real git commands
correctly.
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 Security concerns

The endpoint performs a destructive git reset --hard. While the confirm flag and uncommitted changes check mitigate accidental data loss, it remains a high-risk operation. Ensure API documentation clearly warns users that local commits will be permanently discarded.

⚡ Recommended focus areas for review

Local branch creation before checkout
The code fetches origin/main but then attempts git checkout main. If the local 'main' branch doesn't exist yet, this will fail. It should either create it (-b) or ensure it exists after fetch.

Untracked files not caught by status check
git status --porcelain only shows tracked file changes. Untracked files are ignored, so the function might proceed with reset even if untracked files exist (though reset --hard won't delete them anyway). Consider adding --untracked-files to the status check for completeness or clarify in docs.

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Robustness
Fix multi-line commit message parsing and validate remote branch existence before reset

The current parsing logic splits commit logs by newlines, which will incorrectly
fragment multi-line commit messages. It also lacks validation that the remote branch
actually exists after fetching. Replace newline splitting with a null-byte delimiter
for safe parsing and add a rev-parse --verify check to prevent silent failures on
missing remote refs.

helpers/bible_bridge.py [260-267]

+ref_ok, ref_out = git(project_path, "rev-parse", "--verify", f"{GIT_REMOTE}/{GIT_BRANCH}")
+if not ref_ok:
+    return False, {"ok": False, "status": "remote_branch_missing", "detail": ref_out}
+
 log_ok, log_out = git(
-    project_path, "log", f"{GIT_REMOTE}/{GIT_BRANCH}..HEAD", "--format=%h %s"
+    project_path, "log", f"{GIT_REMOTE}/{GIT_BRANCH}..HEAD", "--format=%x00%h%x00%s"
 )
 commits_dropped = []
 if log_ok and log_out.strip():
-    for line in log_out.strip().split("\n"):
-        sha, _, subject = line.partition(" ")
-        commits_dropped.append({"sha": sha, "subject": subject})
+    for entry in log_out.strip().split("\x00"):
+        if len(entry) >= 2:
+            sha, subject = entry[:7], entry[8:]
+            commits_dropped.append({"sha": sha, "subject": subject})
Suggestion importance[1-10]: 8.5

__

Why: The current implementation splits git log output by newlines, which incorrectly fragments multi-line commit messages and leads to inaccurate commits_dropped reporting. Replacing newline splitting with null-byte delimiters (%x00) ensures robust parsing regardless of commit message formatting. Additionally, adding a rev-parse --verify check for the remote branch prevents silent failures or undefined behavior if the expected ref is missing after fetching.

Medium

@dvystrcil

Copy link
Copy Markdown
Owner Author

Checked both of the PR-agent bot's suggestions against real git behavior before deciding whether to act on them:

  1. "checkout main will fail if local main doesn't exist" — tested directly: it doesn't. git checkout main (no -b) auto-creates a local tracking branch from origin/main via git's DWIM behavior when no local branch exists but a matching remote-tracking ref does (confirmed with a real repo: deleted local main, fetched, git checkout main succeeded and set up tracking). No change needed.
  2. "untracked files not caught by status check" — also tested directly and it's factually wrong: git status --porcelain includes untracked files (?? filename) by default, same as plain git status. The existing check already catches this correctly.

No code changes from this review pass.

@dvystrcil
dvystrcil merged commit 8dfe92d into main Aug 2, 2026
5 checks passed
@dvystrcil
dvystrcil deleted the feat/bible-recover-endpoint branch August 2, 2026 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant