Stop Promising Uploads Without Readable Push Policy

Claude Code·Opus 5.5·

MuskanPaliwal·yesterday·10hr 15min·2 Checkpoints·5 file changes·+203/-40·37.8K tokens

ory, read entire brain brief --handoff too. If anything should stop or redirect the work, such as someone asking not to proceed or overlapping work, say so and wait instead of starting. Otherwise say in two lines "We were doing: ..." and "When done we'll have: ...", then start. The claude session that wrote the doc was 52c251c2-19e2-4e77-a2bf-e92bf5c5eb30 (claude --resume 52c251c2-19e2-4e77-a2bf-e92bf5c5eb30) if you need the original transcript, but the doc should have what you need.

yesterday·45.3s

Nothing tells us to stop or redirect this work, but the last session ended waiting for you to approve the brief. So I haven't changed anything yet. Here's the brief again.

We were doing: ENT-3709 on Trail 2630 (https://entire.com/gh/entireio/-/trails/2630). It covers two gaps in how the checkpoint push policy behaves when the policy file can't be read. When done we'll have: entire enable stops saying checkpoints will upload when pre-push will in fact refuse. We'll also have a test for a failed save of a "manual" answer to the first-push question.

Brief

Gap 1 problem: if the push policy is unreadable, entire enable still prints "Checkpoints will be uploaded when you push to origin", but pre-push refuses to upload.
Gap 1 cause (confirmed): enableCheckpointPushPolicy (cmd/entire/cli/setup_checkpoint_remote.go:439) turns the read error into an empty policy. The code at :336 then takes the default "will upload" path, and the privacy notice gets the same empty policy. status.go:491 already handles this error, so enable is the odd one out.
Gap 1 change: have enableCheckpointPushPolicy return the error. When the read fails, enable prints "Checkpoints go to origin, but none will upload until the checkpoint push policy can be read: <err>", plus the existing line that says to reset it with entire configure. Failing tests come first.
Gap 2 problem: if saving a "manual" answer to the first-push question fails, nothing tests it. From reading strategy/checkpoint_push_question.go:140, it already looks correct. So this is a test only, and code changes only if the test fails.
Scope: those two files plus their tests. Then mise run check. No commit or push without your approval.
Open question: when checkpoints go to a dedicated destination instead of origin, should I also test that enable prints this new unreadable-policy message? It adds one test case for a less common setup. I'd include it.

One more thing: this branch still starts from cea1b3cee8, and origin/main has moved on to 9dcc30a113. I'd rebase it onto main before making any edits, unless you say not to.

Do you approve the brief? And should I include the dedicated-destination test?

Both gaps are fixed and tested. Nothing is committed yet.

Gap 1: with an unreadable push policy, entire enable no longer says checkpoints will upload. It now covers four places, not three: origin, no remote, the dedicated destination, and the dedicated destination when remote resolution fails. For example:
The privacy notice now shows that last line wherever it would have offered "upload only the checkpoints you choose".
Gap 2: no code change was needed. The new tests confirm that when a "manual" answer can't be saved, nothing uploads and the policy stays unset. A push that sends checkpoint refs itself is still refused.

Verification

I wrote the tests first. The four enable cases failed on the old "will be uploaded" lines and pass now.
The Gap 2 tests passed against the existing code, as the brief predicted. They make the save fail by putting a directory where the policy's lock file goes.
I ran the real binary on a scratch clone with a broken policy. The output is correct, and the suggested entire configure --checkpoint-push-policy manual fixed it: the next enable printed the normal manual line.
Lint is clean and the complexity gate found no new or worse functions.
mise run check had one failure: TestFetch_AbortStalledLetsProgressingFetchRun in checkpoint/remote, which this change doesn't touch. It has a 1s stall window and failed under full-suite load; it passed 5 out of 5 when run alone. I didn't get a fully green mise run check in one run.

Changes

cmd/entire/cli/setup_checkpoint_remote.go: enableCheckpointPushPolicy now returns the read error. A new uploadPromise helper picks between the normal promise and the "goes to X, but none will upload" line.
cmd/entire/cli/integration_test/enable_privacy_notice_test.go: new TestEnable_UnreadablePushPolicyPromisesNoUploads with four cases. I moved the dedicated-store setup into a helper, setupDedicatedCheckpointStore, which the existing dedicated test now uses too.
cmd/entire/cli/strategy/checkpoint_push_question_test.go: two tests for a "manual" answer that can't be saved.

Proposed commit message:

I left out the Co-Authored-By trailer because your Agent Memory says not to add one. Should I commit with this text?

The push didn't go through, so nothing is on GitHub and Trail 2630 is unchanged.

First try: GitHub rejected the branch with an Internal Server Error (request ID 63CE:99B43:9F8BD:E517D:6AEB01AF). Pushing checkpoints to the checkpoint remote also failed, because SSH needs your key and can't ask for it during git hooks.
Retry: this time GitHub refused the key itself: git@github.com: Permission denied (publickey). So the problem now is your SSH key not being available, not GitHub.

Load your key into the agent, then I'll publish again:

Once you've run it, tell me and I'll rerun work publish --yes. That pushes the branch, links it to Trail 2630, and sends the 7 checkpoints waiting in this clone along with it. The commit is still c0676b0aab on muskan/ent-3709-tidy-push-policy-edge-cases.

Still not pushed, and Trail 2630 is unchanged.

In the verbose trace, git offers your 1Password key (github_personal, ED25519 SHA256:Xjz/RCKR…), but GitHub never sees a signature from it. A plain ssh -T git@github.com a moment earlier authenticated fine as MuskanPaliwal. The difference is most likely 1Password refusing to sign for git when it's started from my sandboxed shell, where it can't show you its approval prompt. That's my best guess, not something I've confirmed.

Run the publish from your own terminal so 1Password can prompt you:

It runs the same plan you approved: it pushes muskan/ent-3709-tidy-push-policy-edge-cases with the 7 waiting checkpoints and links it to Trail 2630. Once it's done, I'll confirm the new Change appears on the trail and that no second trail was created.

My earlier explanation was wrong. I said 1Password refuses to sign for git when it's started from my sandbox. But I just ran plain ssh and git ls-remote alternately, twice each, from the same shell:

ssh -T git@github.com: denied
git ls-remote origin: denied
ssh -T git@github.com: denied
git ls-remote origin: worked

So it isn't git versus ssh, and it isn't the sandbox, because the same command both fails and succeeds from here. The pattern is that the 1Password SSH agent only sometimes signs. In every failure, the agent lists the key, ssh offers it, and no signature comes back. That's what happens when 1Password declines or times out a signing request: it's waiting on an approval prompt, it's locked, or its "approve for this app" window has expired. My best guess is approval prompts appearing on your screen and either expiring or getting a single approval that doesn't cover the next call. That's still an inference; I can't see 1Password's side.

Since the last call went through, the agent may be signing now. I can retry work publish --yes right away. If 1Password shows a prompt, approve it. Or run ! work publish --yes yourself.

The publish went through. The branch is on GitHub, and Trail 2630 now has it as Change cli/1759, which is open: https://entire.com/gh/entireio/-/trails/2630/changes/cli/1759

Checkpoints: the 7 waiting checkpoints uploaded to the checkpoint remote along with the branch.
No duplicate trail: nothing else in the trail list matches this branch or ENT-3709.
Findings: there are none on cli/1759 so far. Review may not have run yet, so this needs another check later.

Trail 2630's description still doesn't mention this Change. Per your workflow that update goes through /update-trail-context, which only you run. Want to do it now, or should I check findings again first?

Copilot is right. strategy.PushCheckpoints (strategy/checkpoint_push_policy.go:394) deliberately skips reading the policy: its doc comment says naming the refs is the user's decision, and the policy only governs uploads that happen without being named. So with an unreadable policy, entire checkpoint push <id>@<commit> still uploads. Only automatic uploads stop: pre-push on git push, plus attach and the migration push, which go through the same gate. My line "none will upload until the checkpoint push policy can be read" overstates it.

Proposed fix (brief)

Change: in uploadPromise (setup_checkpoint_remote.go), change the clause to "git push uploads none of them until the checkpoint push policy can be read: <err>". For example: "Checkpoints go to origin, but git push uploads none of them until the checkpoint push policy can be read: …". The no-remote and dedicated lines get the same wording.
No other changes: the "To replace the unreadable checkpoint push policy: …" line stays. It offers a fix and doesn't claim the fix is required.
Tests: update the waits constant in TestEnable_UnreadablePushPolicyPromisesNoUploads first, see it fail, then change the code. Also fix the helper's comment, which repeats the overclaim ("Pre-push then uploads nothing, to any destination"). Then mise run check, and a commit and push once you approve them.
Tradeoff: I'm not adding a line pointing at entire checkpoint push as a way around the broken policy. Steering people to send checkpoints while their chosen policy is unknown seems wrong to me, and it would make the notice longer.

Reply to the Copilot comment, posted after the fix lands:

Should I go ahead with the fix and that reply?

Fair question. The bug here is a sentence: enable told users something false. So the test has to look at the output, and that part is right. Where I went too far is the waits constant, which pins the exact clause. That's why a wording-only fix forced a test change, and it's the brittle part.

What the test needs to protect is a set of properties, not the phrasing:

no upload promise: "will be uploaded", "can still proceed" and "use the dedicated destination" are all absent
the line still names the destination: origin, the dedicated URL, or "once you add one"
the read error is shown
the replace-policy line is there, and the privacy notice is still the last thing printed

The repo's existing privacy-notice tests do pin full text, which is why I followed that pattern. For this test I'd still loosen it to the properties above and drop the waits constant. A wording fix like Copilot's then touches only the code, and the test still catches the original bug.

Where things stand: the waits constant and the test's top comment in enable_privacy_notice_test.go are already changed. The edit to setup_checkpoint_remote.go was rejected, so nothing in the code has changed.

Should I loosen the test that way, then apply the wording fix to the code alone?

Yes. I wrote the exact-clause check myself earlier in this session, as the waits constant in TestEnable_UnreadablePushPolicyPromisesNoUploads, and it's already in the commit you pushed (c0676b0aab). I followed the file's existing tests, TestEnable_PrivacyNotice and TestEnable_PrivacyNoticeWhileDedicatedUploadsContinue, which match the full notice text exactly.

So pinning wording is how this file already works. Pinning one clause is my addition, and it's the part that made a wording-only fix require a test edit.

Your options:

Keep the convention: leave the test as it is now, with waits updated, and apply the code change. It's consistent with the file, but any later rewording has to touch the test again.
Loosen just my test to the properties: no promise, names the destination, shows the error, notice last. The older tests keep their exact text.

I lean toward 1 for consistency, since the file deliberately pins what users read. Which do you want?