Skip to content

Commit ee38cf7

Browse files
coopernetesclaude
andcommitted
fix: record ERROR status when upstream forward fails
The post-receive persistence hook checked ReceiveCommand results to determine FORWARDED vs ERROR, but JGit only delivers Result.OK commands to post-receive hooks. When the upstream push failed, the command result stayed OK and the record was incorrectly saved as FORWARDED. Now checks pushContext for a failed "forward" step instead, which is where ForwardingPostReceiveHook records the upstream outcome. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent becd828 commit ee38cf7

2 files changed

Lines changed: 72 additions & 12 deletions

File tree

git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushStorePersistenceHook.java

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -251,22 +251,25 @@ public PostReceiveHook postReceiveHook() {
251251
return;
252252
}
253253

254-
boolean allOk = commands.stream().allMatch(cmd -> cmd.getResult() == ReceiveCommand.Result.OK);
255-
boolean anyTransportFailed = commands.stream()
256-
.anyMatch(cmd -> cmd.getResult() != ReceiveCommand.Result.OK
257-
&& cmd.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED);
254+
// Check if the forwarding step failed (upstream rejected the push).
255+
// Command results stay OK in post-receive even when the upstream push fails,
256+
// because JGit already accepted the objects locally. The forwarding outcome
257+
// is recorded in pushContext by ForwardingPostReceiveHook.
258+
boolean forwardFailed = pushContext != null
259+
&& pushContext.getSteps().stream()
260+
.anyMatch(step -> "forward".equals(step.getStepName())
261+
&& step.getStatus() == StepStatus.FAIL);
258262

259263
PushRecord record = copyBase(initial);
260-
if (allOk) {
261-
record.setStatus(PushStatus.FORWARDED);
262-
} else if (anyTransportFailed) {
264+
if (forwardFailed) {
263265
record.setStatus(PushStatus.ERROR);
264-
commands.stream()
265-
.filter(cmd -> cmd.getResult() != ReceiveCommand.Result.OK
266-
&& cmd.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED)
266+
pushContext.getSteps().stream()
267+
.filter(step ->
268+
"forward".equals(step.getStepName()) && step.getStatus() == StepStatus.FAIL)
267269
.findFirst()
268-
.ifPresent(cmd -> record.setErrorMessage(
269-
cmd.getRefName() + ": " + cmd.getResult() + " - " + cmd.getMessage()));
270+
.ifPresent(step -> record.setErrorMessage(step.getErrorMessage()));
271+
} else {
272+
record.setStatus(PushStatus.FORWARDED);
270273
}
271274

272275
pushStore.save(record);

git-proxy-java-core/src/test/java/org/finos/gitproxy/git/PushStorePersistenceHookTest.java

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
import org.finos.gitproxy.db.PushStore;
1919
import org.finos.gitproxy.db.memory.InMemoryPushStore;
2020
import org.finos.gitproxy.db.model.PushStatus;
21+
import org.finos.gitproxy.db.model.PushStep;
22+
import org.finos.gitproxy.db.model.StepStatus;
2123
import org.finos.gitproxy.provider.GitHubProvider;
2224
import org.junit.jupiter.api.BeforeEach;
2325
import org.junit.jupiter.api.Test;
@@ -240,6 +242,61 @@ void serviceUrl_setBeforeHook_includedInBlockedRecord() {
240242
assertFalse(records.isEmpty(), "should have a PENDING record");
241243
}
242244

245+
// ---- post-receive hook: forwarding outcome ----
246+
247+
@Test
248+
void postReceiveHook_forwardFailed_savesErrorStatus() {
249+
var pushContext = new PushContext();
250+
hook.setPushContext(pushContext);
251+
252+
ReceivePack rp = makeReceivePack();
253+
ReceiveCommand cmd = newBranchCommand(commitId);
254+
255+
// Pre-receive creates the initial RECEIVED record
256+
hook.preReceiveHook().onPreReceive(rp, List.of(cmd));
257+
258+
// Simulate a failed forward step (upstream rejected the push)
259+
pushContext.addStep(PushStep.builder()
260+
.stepName("forward")
261+
.status(StepStatus.FAIL)
262+
.errorMessage("REJECTED_OTHER_REASON (pre-receive hook declined)")
263+
.build());
264+
265+
// Mark command as OK (JGit accepted locally) — post-receive only sees OK commands
266+
cmd.setResult(ReceiveCommand.Result.OK);
267+
268+
hook.postReceiveHook().onPostReceive(rp, List.of(cmd));
269+
270+
var records = pushStore.find(org.finos.gitproxy.db.model.PushQuery.builder()
271+
.status(PushStatus.ERROR)
272+
.build());
273+
assertFalse(records.isEmpty(), "a failed forward should produce an ERROR record");
274+
assertNotNull(records.get(0).getErrorMessage(), "ERROR record should have an error message");
275+
}
276+
277+
@Test
278+
void postReceiveHook_forwardSucceeded_savesForwardedStatus() {
279+
var pushContext = new PushContext();
280+
hook.setPushContext(pushContext);
281+
282+
ReceivePack rp = makeReceivePack();
283+
ReceiveCommand cmd = newBranchCommand(commitId);
284+
285+
hook.preReceiveHook().onPreReceive(rp, List.of(cmd));
286+
287+
pushContext.addStep(
288+
PushStep.builder().stepName("forward").status(StepStatus.PASS).build());
289+
290+
cmd.setResult(ReceiveCommand.Result.OK);
291+
292+
hook.postReceiveHook().onPostReceive(rp, List.of(cmd));
293+
294+
var records = pushStore.find(org.finos.gitproxy.db.model.PushQuery.builder()
295+
.status(PushStatus.FORWARDED)
296+
.build());
297+
assertFalse(records.isEmpty(), "a successful forward should produce a FORWARDED record");
298+
}
299+
243300
// ---- email validation config ----
244301

245302
private CommitConfig blockNoreplyConfig() {

0 commit comments

Comments
 (0)