Skip to content

Commit 7deb215

Browse files
flyingrobotsflyingrobots
committed
fix(git): classify stdout-only ref misses
1 parent 1338f8f commit 7deb215

8 files changed

Lines changed: 142 additions & 87 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
135135
- **Review-feedback test style guards** — privacy error assertions now use
136136
`ErrorCodes` constants, and ManifestDiff declaration checks use regex matching
137137
so benign JSDoc formatting does not break release tests.
138+
- **Stdout-only missing vault refs** — Git ref resolution now treats
139+
`rev-parse refs/cas/vault` failures that only echo the unresolved ref on
140+
stdout as `GIT_REF_NOT_FOUND`, preventing empty-vault initialization flakes
141+
from surfacing as `VAULT_HEAD_INVALID`.
138142
- **Vault metadata enforces the AES-GCM cipher boundary** — `.vault.json`
139143
metadata now rejects unsupported `encryption.cipher` values with
140144
`VAULT_METADATA_INVALID`; the v6 vault metadata format remains AES-256-GCM.

‎docs/VAULT_INTERNALS.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,11 @@ mutable, so ref resolution must not be cached by `VaultStateCache` or
193193
capacity is exceeded; the default capacity is intentionally a memory bound, not
194194
a durability or correctness boundary.
195195

196+
Missing vault refs are normalized before `VaultService` sees them. Git adapters
197+
must treat both English stderr forms and stdout-only `rev-parse <ref>` misses as
198+
an absent vault ref; corrupt or unreadable refs still fail closed as
199+
`VAULT_HEAD_INVALID`.
200+
196201
Cache entries may contain:
197202

198203
- raw immutable tree entries copied from persistence

‎src/domain/helpers/gitRefErrors.js‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
const GIT_REV_PARSE = 'rev-parse';
2+
const GIT_REF_NOT_FOUND_STATUS = 128;
3+
4+
const MISSING_REF_MARKERS = Object.freeze({
5+
ambiguousArgument: 'ambiguous argument',
6+
neededSingleRevision: 'needed a single revision',
7+
unknownRevision: 'unknown revision',
8+
});
9+
10+
/**
11+
* @param {unknown} err
12+
* @param {string} ref
13+
* @returns {boolean}
14+
*/
15+
export function isGitMissingRefError(err, ref) {
16+
return isStdoutOnlyRevParseMiss(errorDetails(err), ref) ||
17+
isGitMissingRefMessage(errorDetailsText(err), ref);
18+
}
19+
20+
/**
21+
* @param {unknown} err
22+
* @returns {string}
23+
*/
24+
export function errorDetailsText(err) {
25+
if (!(err instanceof Error)) {
26+
return String(err);
27+
}
28+
const details = errorDetails(err);
29+
return [
30+
err.message,
31+
typeof details.stderr === 'string' ? details.stderr : '',
32+
typeof details.stdout === 'string' ? details.stdout : '',
33+
].join('\n');
34+
}
35+
36+
/**
37+
* @param {unknown} err
38+
* @returns {Record<string, unknown>}
39+
*/
40+
function errorDetails(err) {
41+
return err instanceof Error && typeof err.details === 'object' && err.details
42+
? err.details
43+
: {};
44+
}
45+
46+
/**
47+
* @param {Record<string, unknown>} details
48+
* @param {string} ref
49+
* @returns {boolean}
50+
*/
51+
function isStdoutOnlyRevParseMiss(details, ref) {
52+
// Some plumbing runners surface a stdout-only `rev-parse <ref>` miss: Git
53+
// exits 128 and echoes the unresolved ref without emitting locale text.
54+
return details.code === GIT_REF_NOT_FOUND_STATUS &&
55+
Array.isArray(details.args) &&
56+
details.args[0] === GIT_REV_PARSE &&
57+
details.args.at(-1) === ref &&
58+
typeof details.stdout === 'string' &&
59+
details.stdout.trim() === ref &&
60+
`${details.stderr ?? ''}`.trim() === '';
61+
}
62+
63+
/**
64+
* @param {string} message
65+
* @param {string} ref
66+
* @returns {boolean}
67+
*/
68+
function isGitMissingRefMessage(message, ref) {
69+
const normalized = message.toLowerCase();
70+
const normalizedRef = ref.toLowerCase();
71+
if (!normalized.includes(normalizedRef)) {
72+
return false;
73+
}
74+
// C/English-locale missing-ref fallback: normal adapters should return
75+
// GIT_REF_NOT_FOUND. This best-effort fallback is only for third-party ports
76+
// that expose Git stderr without a structured code.
77+
return (
78+
normalized.includes(MISSING_REF_MARKERS.neededSingleRevision) ||
79+
(
80+
normalized.includes(MISSING_REF_MARKERS.ambiguousArgument) &&
81+
normalized.includes(MISSING_REF_MARKERS.unknownRevision)
82+
)
83+
);
84+
}

‎src/domain/services/VaultPersistence.js‎

Lines changed: 5 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,12 @@ import VaultTreeCodec, {
55
VAULT_PRIVACY_INDEX_ENTRY,
66
} from './VaultTreeCodec.js';
77
import { ErrorCodes } from '../errors/index.js';
8+
import {
9+
errorDetailsText,
10+
isGitMissingRefError,
11+
} from '../helpers/gitRefErrors.js';
812

913
export const VAULT_REF = 'refs/cas/vault';
10-
const MISSING_REF_MARKERS = Object.freeze({
11-
ambiguousArgument: 'ambiguous argument',
12-
neededSingleRevision: 'needed a single revision',
13-
unknownRevision: 'unknown revision',
14-
});
1514
const UPDATE_REF_CONFLICT_MARKERS = Object.freeze({
1615
butExpected: 'but expected',
1716
cannotLockRef: 'cannot lock ref',
@@ -347,29 +346,7 @@ function isMissingVaultRefError(err) {
347346
if (typeof err?.code === 'string' && err.code === ErrorCodes.GIT_REF_NOT_FOUND) {
348347
return true;
349348
}
350-
const message = errorDetailsText(err);
351-
return isGitMissingRefMessage(message);
352-
}
353-
354-
/**
355-
* @param {string} message
356-
* @returns {boolean}
357-
*/
358-
function isGitMissingRefMessage(message) {
359-
const normalized = message.toLowerCase();
360-
if (!normalized.includes(VAULT_REF)) {
361-
return false;
362-
}
363-
// C/English-locale missing-ref fallback: normal adapters should return
364-
// GIT_REF_NOT_FOUND. This best-effort fallback is only for third-party ports
365-
// that expose Git stderr without a structured code.
366-
return (
367-
normalized.includes(MISSING_REF_MARKERS.neededSingleRevision) ||
368-
(
369-
normalized.includes(MISSING_REF_MARKERS.ambiguousArgument) &&
370-
normalized.includes(MISSING_REF_MARKERS.unknownRevision)
371-
)
372-
);
349+
return isGitMissingRefError(err, VAULT_REF);
373350
}
374351

375352
/**
@@ -430,20 +407,4 @@ function isGitUpdateRefCasMismatch(message) {
430407
);
431408
}
432409

433-
/**
434-
* @param {unknown} err
435-
* @returns {string}
436-
*/
437-
function errorDetailsText(err) {
438-
if (!(err instanceof Error)) {
439-
return String(err);
440-
}
441-
const details = typeof err.details === 'object' && err.details ? err.details : {};
442-
return [
443-
err.message,
444-
typeof details.stderr === 'string' ? details.stderr : '',
445-
typeof details.stdout === 'string' ? details.stdout : '',
446-
].join('\n');
447-
}
448-
449410
export { VAULT_METADATA_ENTRY, VAULT_PRIVACY_INDEX_ENTRY };

‎src/infrastructure/adapters/GitRefAdapter.js‎

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Policy } from '@git-stunts/alfred';
22
import GitRefPort from '../../ports/GitRefPort.js';
33
import { CasError, ErrorCodes } from '../../domain/errors/index.js';
4+
import { isGitMissingRefError } from '../../domain/helpers/gitRefErrors.js';
45

56
/**
67
* Default resilience policy: 30 s timeout (no retry).
@@ -12,11 +13,6 @@ import { CasError, ErrorCodes } from '../../domain/errors/index.js';
1213
*/
1314
const DEFAULT_POLICY = Policy.timeout(30_000);
1415
const GIT_NULL_OID = '0'.repeat(40);
15-
const MISSING_REF_MARKERS = Object.freeze({
16-
ambiguousArgument: 'ambiguous argument',
17-
neededSingleRevision: 'needed a single revision',
18-
unknownRevision: 'unknown revision',
19-
});
2016

2117
/**
2218
* {@link GitRefPort} implementation backed by `@git-stunts/plumbing`.
@@ -47,7 +43,7 @@ export default class GitRefAdapter extends GitRefPort {
4743
this.plumbing.execute({ args: ['rev-parse', ref] }),
4844
);
4945
} catch (err) {
50-
if (isGitMissingRefMessage(errorDetailsText(err), ref)) {
46+
if (isGitMissingRefError(err, ref)) {
5147
throw new CasError(`Git ref not found: ${ref}`, ErrorCodes.GIT_REF_NOT_FOUND, {
5248
ref,
5349
originalError: err,
@@ -104,39 +100,3 @@ export default class GitRefAdapter extends GitRefPort {
104100
);
105101
}
106102
}
107-
108-
/**
109-
* @param {string} message
110-
* @param {string} ref
111-
* @returns {boolean}
112-
*/
113-
function isGitMissingRefMessage(message, ref) {
114-
const normalized = message.toLowerCase();
115-
const normalizedRef = ref.toLowerCase();
116-
if (!normalized.includes(normalizedRef)) {
117-
return false;
118-
}
119-
return (
120-
normalized.includes(MISSING_REF_MARKERS.neededSingleRevision) ||
121-
(
122-
normalized.includes(MISSING_REF_MARKERS.ambiguousArgument) &&
123-
normalized.includes(MISSING_REF_MARKERS.unknownRevision)
124-
)
125-
);
126-
}
127-
128-
/**
129-
* @param {unknown} err
130-
* @returns {string}
131-
*/
132-
function errorDetailsText(err) {
133-
if (!(err instanceof Error)) {
134-
return String(err);
135-
}
136-
const details = typeof err.details === 'object' && err.details ? err.details : {};
137-
return [
138-
err.message,
139-
typeof details.stderr === 'string' ? details.stderr : '',
140-
typeof details.stdout === 'string' ? details.stdout : '',
141-
].join('\n');
142-
}

‎test/unit/docs/test-style.test.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,11 @@ describe('documentation test style', () => {
4848
});
4949

5050
it('documents the locale assumption for vault missing-ref fallback parsing', () => {
51-
const source = read('src/domain/services/VaultPersistence.js');
51+
const source = read('src/domain/helpers/gitRefErrors.js');
5252

5353
expect(source).toContain('C/English-locale missing-ref fallback');
5454
expect(source).toContain('best-effort fallback');
55+
expect(source).toContain('stdout-only');
5556
});
5657
});
5758

‎test/unit/domain/services/VaultPersistence.test.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,26 @@ describe('VaultPersistence head reads', () => {
5555

5656
await expect(vaultPersistence.resolveHead()).resolves.toBeNull();
5757
});
58+
});
59+
60+
describe('VaultPersistence missing vault ref shapes', () => {
61+
it('resolves stdout-only rev-parse misses as null', async () => {
62+
const rootCause = Object.assign(new Error('Git command failed with code 128'), {
63+
details: {
64+
args: ['rev-parse', 'refs/cas/vault'],
65+
code: 128,
66+
stdout: 'refs/cas/vault\n',
67+
stderr: '',
68+
},
69+
});
70+
const ref = mockRef({ resolveRef: vi.fn().mockRejectedValue(rootCause) });
71+
const vaultPersistence = new VaultPersistence({ persistence: mockPersistence(), ref });
72+
73+
await expect(vaultPersistence.resolveHead()).resolves.toBeNull();
74+
});
75+
});
5876

77+
describe('VaultPersistence current head reads', () => {
5978
it('resolves the current vault head', async () => {
6079
const ref = mockRef({
6180
resolveRef: vi.fn().mockResolvedValue('commit-oid'),

‎test/unit/infrastructure/adapters/GitRefAdapter.test.js‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,27 @@ describe('GitRefAdapter.resolveRef()', () => {
3333
},
3434
});
3535
});
36+
37+
it('normalizes stdout-only rev-parse misses into a structured ref-not-found error', async () => {
38+
const { adapter, plumbing } = createAdapter();
39+
const rootCause = Object.assign(new Error('Git command failed with code 128'), {
40+
details: {
41+
args: ['rev-parse', 'refs/cas/vault'],
42+
code: 128,
43+
stdout: 'refs/cas/vault\n',
44+
stderr: '',
45+
},
46+
});
47+
plumbing.execute.mockRejectedValueOnce(rootCause);
48+
49+
await expect(adapter.resolveRef('refs/cas/vault')).rejects.toMatchObject({
50+
code: ErrorCodes.GIT_REF_NOT_FOUND,
51+
meta: {
52+
ref: 'refs/cas/vault',
53+
originalError: rootCause,
54+
},
55+
});
56+
});
3657
});
3758

3859
describe('GitRefAdapter.updateRef()', () => {

0 commit comments

Comments
 (0)