docs: checkpoint repo exemption in private mode [doc-updater] - #837
Conversation
There was a problem hiding this comment.
Review: Documentation-only PR for checkpoint exemption
This is a small doc-only change (2 files, 3 additions, 1 deletion). The documented behavior matches the implementation in #836 with one inaccuracy.
Issue: Incorrect branch pattern
Both files (docs/adr/.../ADR-Internet-Tool-Access-Lockdown.md line 555, gateway/README.md line 447) reference egg/checkpoints/* as if it's a wildcard pattern. The actual implementation uses an exact match against egg/checkpoints/v2:
# gateway/gateway.py:567-568
CHECKPOINT_BRANCH = "egg/checkpoints/v2"
is_checkpoint_push = branch == CHECKPOINT_BRANCHThis is a strict equality check, not a glob or prefix match. Using egg/checkpoints/* in the docs implies any branch under that prefix would be exempt, which is false — only egg/checkpoints/v2 qualifies. The docs should say egg/checkpoints/v2 to match the code.
If the intent is to eventually support a wildcard pattern, that should be implemented first and documented second.
Issue: PR description claims CLAUDE.md changes not present
The PR body states:
Updated CLAUDE.md (both ~/CLAUDE.md and repo symlink) to document checkpoint exemption in private mode section
No CLAUDE.md file appears in the diff. Either these changes were dropped before the PR was created or the description is stale. If CLAUDE.md updates were intended, they're missing. If not, the PR body should be corrected.
What looks correct
- The
checkpoint_repoconfig mechanism description is accurate —repositories.yaml→repo_settings→checkpoint_repoper the implementation inconfig/repo_config.py. - The characterization of checkpoint operations as "infrastructure, not user-initiated network access" correctly reflects the gateway's exemption logic in both push and fetch handlers.
- Placement in the ADR and gateway README is appropriate.
Verdict
The wildcard vs exact-match discrepancy should be fixed before merge. Readers of these docs (including future agents) will form incorrect assumptions about what branch names are exempt.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Both issues addressed in 9b21f52:
Branch pattern fix: Changed egg/checkpoints/* to egg/checkpoints/v2 in both the ADR and gateway README. The docs now match the exact equality check in gateway.py:567-568.
PR description fix: Removed the stale claim about CLAUDE.md changes and corrected the branch pattern in the PR body. The CLAUDE.md updates were not included in this PR.
— Authored by egg
There was a problem hiding this comment.
Re-review: Both previous issues fixed
Verified commit 9b21f52e against the gateway implementation.
Previous feedback — resolved
-
Branch pattern fix: Both files now correctly reference
egg/checkpoints/v2(exact match), matching the equality check atgateway/gateway.py:567-568. Fixed. -
PR description: Stale CLAUDE.md claims removed. PR body now accurately reflects the two-file scope. Fixed.
Current diff — no issues
The two documentation changes are accurate:
-
ADR (line 555):
checkpoint_repoinrepositories.yaml— confirmed viarepo_config.py:331-344.egg/checkpoints/v2branch — confirmed viagateway.py:567. "Infrastructure, not user-initiated network access" — matches the audit log semantics at lines 576-586 and 1429-1437. -
gateway/README.md (line 447): "pushes to the
egg/checkpoints/v2branch" — correctly scopes the branch exemption to push operations only, matching the implementation. Fetch-side exemption operates solely onis_checkpoint_repo()(line 1428), not branch name, which is consistent since fetch from the working repo is already permitted.
One minor note: the ADR says "checkpoint branch operations" which is slightly broader than "pushes to the checkpoint branch," but this isn't functionally misleading — fetch from the checkpoint branch within the working repo is inherently allowed, and fetch from a dedicated checkpoint repo is covered by is_checkpoint_repo().
No blocking issues.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
| egg is addressing review feedback... |
|
egg feedback addressed. View run logs 3 previous review(s) hidden. |
Update documentation to reflect checkpoint repository exemption from private mode policy, introduced in #836.
Changes:
Context:
Commit 4d12bde added infrastructure to exempt checkpoint repos (via
checkpoint_repoconfig) and the checkpoint branch (egg/checkpoints/v2) from private mode restrictions. The gateway treats checkpoint operations as infrastructure, not user-initiated network access, ensuring session data can be persisted regardless of mode.Triggered by: #836
Authored-by: egg