mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-09-11 05:14:29 +00:00
Fix jobs review findings
This commit is contained in:
@@ -153,7 +153,7 @@ window.slopsmith.jobs.registerProvider({
|
||||
providerId: 'sloppak_converter.jobs',
|
||||
pluginId: 'sloppak_converter',
|
||||
label: 'Sloppak Converter',
|
||||
jobTypes: ['psarc-to-sloppak'],
|
||||
jobTypes: ['sloppak-convert'],
|
||||
actions: ['enqueue', 'inspect', 'cancel', 'retry', 'recover'],
|
||||
capacity: { maxRunning: 1, maxQueued: 20 },
|
||||
recoverySupport: { queued: true, running: false, paused: false },
|
||||
@@ -173,7 +173,7 @@ const result = await window.slopsmith.capabilities.dispatch({
|
||||
command: 'enqueue',
|
||||
source: 'sloppak_converter',
|
||||
args: {
|
||||
jobType: 'psarc-to-sloppak',
|
||||
jobType: 'sloppak-convert',
|
||||
requester: 'sloppak_converter',
|
||||
authorization: 'user-action',
|
||||
target: { targetRef: 'song-target-abc123' },
|
||||
|
||||
+34
-11
@@ -110,7 +110,7 @@
|
||||
.replace(/[A-Za-z]:\\[^\s]+/g, '[path]')
|
||||
.replace(/https?:\/\/[^\s?#]+[^\s]*/gi, '[url]')
|
||||
.replace(/\b(token|secret|password|api[_-]?key)=([^\s&]+)/gi, '$1=[redacted]')
|
||||
.replace(/\b[A-Za-z0-9._%+-]+\.(psarc|sloppak|wav|mp3|ogg|flac|zip|sqlite|db)\b/gi, '[file]')
|
||||
.replace(/\b[A-Za-z0-9._%+-]+\.(sloppak|wav|mp3|ogg|flac|zip|sqlite|db)\b/gi, '[file]')
|
||||
.replace(/\b(raw[-_ ]?artifact|raw[-_ ]?audio|audio[-_ ]?buffer|sample[s]?|waveform[s]?|recording[s]?|subprocess|native[-_ ]?handle|process[-_ ]?handle)\b/gi, '[private]')
|
||||
.replace(/\b(?:ffmpeg|vgmstream|rscli|python|node|uvicorn)\s+[^\n\r]*/gi, '[command]');
|
||||
}
|
||||
@@ -131,6 +131,23 @@
|
||||
return `${prefix}-${_hash(normalized)}`;
|
||||
}
|
||||
|
||||
function _rawHashSeed(value, fallback = 'unknown') {
|
||||
const seen = typeof WeakSet === 'function' ? new WeakSet() : null;
|
||||
try {
|
||||
const raw = JSON.stringify(value, (_key, item) => {
|
||||
if (typeof item === 'function') return '[function]';
|
||||
if (item && typeof item === 'object' && seen) {
|
||||
if (seen.has(item)) return '[circular]';
|
||||
seen.add(item);
|
||||
}
|
||||
return item;
|
||||
});
|
||||
return raw || fallback;
|
||||
} catch (_) {
|
||||
return fallback;
|
||||
}
|
||||
}
|
||||
|
||||
function _safeValue(value, depth = 0) {
|
||||
if (typeof value === 'string') return _safeText(value);
|
||||
if (value == null || typeof value === 'number' || typeof value === 'boolean') return value;
|
||||
@@ -258,14 +275,14 @@
|
||||
const source = _plainObject(target);
|
||||
if (source.safeRef || source.safeTargetRef || source.safeFingerprint) return _safeId(source.safeRef || source.safeTargetRef || source.safeFingerprint, 'target-unknown');
|
||||
if (source.targetRef && /^target-[A-Za-z0-9_.:-]+$/.test(String(source.targetRef))) return String(source.targetRef).slice(0, 96);
|
||||
const seed = JSON.stringify(_safeValue(source)) || source.kind || 'unknown';
|
||||
const seed = _rawHashSeed(source, source.kind || 'unknown');
|
||||
return `target-${_hash(seed)}`;
|
||||
}
|
||||
|
||||
function _inputFingerprint(inputs) {
|
||||
const source = _plainObject(inputs);
|
||||
if (source.safeFingerprint) return _safeId(source.safeFingerprint, 'input-unknown');
|
||||
return `input-${_hash(JSON.stringify(_safeValue(source)))}`;
|
||||
return `input-${_hash(_rawHashSeed(source, 'inputs'))}`;
|
||||
}
|
||||
|
||||
function _approvalScope(args, providerId) {
|
||||
@@ -497,6 +514,12 @@
|
||||
return { outcome: 'handled' };
|
||||
}
|
||||
|
||||
async function _settleProviderResult(result, operation) {
|
||||
if (!_isPromiseLike(result)) return result;
|
||||
try { return await result; }
|
||||
catch (err) { return _providerException(operation, err); }
|
||||
}
|
||||
|
||||
function _compatibleProviders(jobType) {
|
||||
return Array.from(providers.values()).filter(provider => {
|
||||
if (!provider.jobTypes.includes(jobType)) return false;
|
||||
@@ -536,7 +559,7 @@
|
||||
if (args.authorization === 'user-action') return true;
|
||||
if (args.authorization === 'approved-continuation') {
|
||||
const scope = _approvalScope(args, providerId);
|
||||
return _scopeKey(scope) === _safeId(args.approvalScopeKey || args.approvalKey || '', '');
|
||||
return _scopeKey(scope) === _string(args.approvalScopeKey || args.approvalKey, '');
|
||||
}
|
||||
return false;
|
||||
}
|
||||
@@ -615,7 +638,7 @@
|
||||
targetRef: _safeId(ref.targetRef, 'target-unknown'),
|
||||
inputFingerprint: _safeId(ref.inputFingerprint, 'input-unknown'),
|
||||
approvalScope: null,
|
||||
approvalScopeKey: _safeId(ref.approvalScopeKey, ''),
|
||||
approvalScopeKey: _string(ref.approvalScopeKey, ''),
|
||||
authorization: 'recovered',
|
||||
priority: _priority(ref.priority),
|
||||
safeLabel: _safeText(ref.safeLabel || 'Recovered job', 'Recovered job', MAX_LABEL),
|
||||
@@ -637,7 +660,7 @@
|
||||
return job;
|
||||
}
|
||||
|
||||
function _recoverRefsForProvider(provider) {
|
||||
async function _recoverRefsForProvider(provider) {
|
||||
for (const [jobId, ref] of Array.from(pendingRecoverableRefs.entries())) {
|
||||
if (ref.providerId !== provider.providerId || jobs.has(jobId)) continue;
|
||||
const recoverState = ref.state === STATES.PAUSED ? STATES.PAUSED : (ref.state === STATES.RUNNING ? STATES.RUNNING : STATES.QUEUED);
|
||||
@@ -649,11 +672,11 @@
|
||||
continue;
|
||||
}
|
||||
let nextState = recoverState;
|
||||
const recoveryResult = _callProvider(provider, 'job.recover', { ref });
|
||||
const recoveryResult = await _settleProviderResult(_callProvider(provider, 'job.recover', { ref }), 'recover');
|
||||
if (_providerCallFailed(recoveryResult)) {
|
||||
const job = _jobFromRecoveryRef(ref, STATES.PROVIDER_UNAVAILABLE);
|
||||
jobs.set(job.jobId, job);
|
||||
_terminal(job, 'provider-unavailable', 'provider-unavailable', recoveryResult.safeReason || 'Provider recovery callback failed', false);
|
||||
_terminal(job, 'provider-unavailable', 'provider-unavailable', recoveryResult.safeReason || recoveryResult.reason || 'Provider recovery callback failed', false);
|
||||
_emitJobs('provider-unavailable', { job: _jobSummary(job, { includeHistory: false }) });
|
||||
pendingRecoverableRefs.delete(jobId);
|
||||
continue;
|
||||
@@ -668,7 +691,7 @@
|
||||
_persistRecoverableRefs();
|
||||
}
|
||||
|
||||
function _registerProviderCommand(ctx) {
|
||||
async function _registerProviderCommand(ctx) {
|
||||
const provider = _normalizeProvider(ctx.payload && ctx.payload.provider || ctx.payload);
|
||||
if (!provider.providerId || !provider.jobTypes.length) {
|
||||
_rememberOutcome('register-provider', 'validation-failed', { safeReason: 'providerId and jobTypes are required' });
|
||||
@@ -677,7 +700,7 @@
|
||||
if (provider.version !== 1 || provider.availability === 'incompatible') provider.availability = 'incompatible';
|
||||
const existing = providers.get(provider.providerId);
|
||||
providers.set(provider.providerId, { ...(existing || {}), ...provider, lastSeenAt: _now() });
|
||||
_recoverRefsForProvider(provider);
|
||||
await _recoverRefsForProvider(provider);
|
||||
_rememberOutcome('register-provider', provider.availability === 'incompatible' ? 'incompatible-version' : 'handled', { providerId: provider.providerId, safeReason: provider.safeReason });
|
||||
_emitJobs('provider-registered', { provider: _providerSummary(provider) });
|
||||
return provider.availability === 'incompatible'
|
||||
@@ -839,7 +862,7 @@
|
||||
if (!job.retryable) return _result('unsupported-operation', { job: _jobSummary(job) }, 'job is not retryable');
|
||||
if (job.attempts.some(attempt => !TERMINAL_STATES.has(attempt.state))) return _result('stale', { job: _jobSummary(job) }, 'retry already active');
|
||||
if (args.authorization !== 'user-action') {
|
||||
if (args.authorization !== 'approved-continuation' || _safeId(args.approvalScopeKey || args.approvalKey, '') !== job.approvalScopeKey) {
|
||||
if (args.authorization !== 'approved-continuation' || _string(args.approvalScopeKey || args.approvalKey, '') !== job.approvalScopeKey) {
|
||||
return _result(args.authorization ? 'denied' : 'user-action-required', { job: _jobSummary(job) }, 'matching approval required for retry');
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,7 +8,7 @@ test('diagnostics schema redacts raw payloads, paths, command lines, and provide
|
||||
await dispatch(window, 'register-provider', { provider });
|
||||
const enqueued = await dispatch(window, 'enqueue', enqueuePayload({
|
||||
safeLabel: 'Generate cache',
|
||||
target: { path: '/Users/example/Music/Secret Artist - Secret Song_p.psarc', filename: 'Secret Song_p.psarc' },
|
||||
target: { path: '/Users/example/Music/Secret Artist - Secret Song_p.sloppak', filename: 'Secret Song_p.sloppak' },
|
||||
inputs: { token: 'abc123', rawPayload: 'never export', commandLine: 'ffmpeg -i secret.wav out.ogg', safeFingerprint: 'fingerprint-public' },
|
||||
}));
|
||||
window.slopsmith.jobs.log(provider.providerId, enqueued.payload.job.jobId, 'ran ffmpeg -i /Users/example/secret.wav with token=abc123');
|
||||
@@ -25,18 +25,32 @@ test('caller-supplied refs and fingerprints are exported as safe correlation key
|
||||
const { provider } = makeProvider();
|
||||
await dispatch(window, 'register-provider', { provider });
|
||||
const enqueued = await dispatch(window, 'enqueue', enqueuePayload({
|
||||
target: { targetRef: 'Secret Song_p.psarc', id: 'Private Library Entry' },
|
||||
inputs: { fingerprint: 'Secret Input Cache.psarc' },
|
||||
logicalJobKey: 'Secret Song_p.psarc',
|
||||
target: { targetRef: 'Secret Song_p.sloppak', id: 'Private Library Entry' },
|
||||
inputs: { fingerprint: 'Secret Input Cache.sloppak' },
|
||||
logicalJobKey: 'Secret Song_p.sloppak',
|
||||
}));
|
||||
const bridge = await dispatch(window, 'record-bridge-hit', { logicalJobKey: 'Secret Song_p.psarc', safeReason: 'legacy path observed' });
|
||||
const bridge = await dispatch(window, 'record-bridge-hit', { logicalJobKey: 'Secret Song_p.sloppak', safeReason: 'legacy path observed' });
|
||||
const snapshot = diagnosticsSnapshot(window);
|
||||
const job = snapshot.jobs.active[0];
|
||||
|
||||
assert.equal(bridge.payload.bridge.jobId, enqueued.payload.job.jobId);
|
||||
assert.equal(job.targetRef.startsWith('target-'), true);
|
||||
assert.equal(job.inputFingerprint.startsWith('input-'), true);
|
||||
assert.doesNotMatch(JSON.stringify(snapshot), /Secret Song|Secret Input|Private Library|\.psarc/);
|
||||
assert.doesNotMatch(JSON.stringify(snapshot), /Secret Song|Secret Input|Private Library|\.sloppak/);
|
||||
});
|
||||
|
||||
test('raw-only target and input fields hash distinctly without exporting raw values', async () => {
|
||||
const window = loadJobs();
|
||||
const { provider } = makeProvider({ capacity: { maxRunning: 1, maxQueued: 10 } });
|
||||
await dispatch(window, 'register-provider', { provider });
|
||||
|
||||
const first = await dispatch(window, 'enqueue', enqueuePayload({ target: { path: '/Users/example/DLC/Secret A.sloppak' }, inputs: { token: 'secret-a' }, logicalJobKey: '' }));
|
||||
const second = await dispatch(window, 'enqueue', enqueuePayload({ target: { path: '/Users/example/DLC/Secret B.sloppak' }, inputs: { token: 'secret-b' }, logicalJobKey: '' }));
|
||||
const json = JSON.stringify(diagnosticsSnapshot(window));
|
||||
|
||||
assert.notEqual(first.payload.job.targetRef, second.payload.job.targetRef);
|
||||
assert.notEqual(first.payload.job.inputFingerprint, second.payload.job.inputFingerprint);
|
||||
assert.doesNotMatch(json, /Secret A|Secret B|secret-a|secret-b|\.sloppak/);
|
||||
});
|
||||
|
||||
test('recoverable job references are the only active state persisted across reloads', async () => {
|
||||
@@ -58,6 +72,49 @@ test('recoverable job references are the only active state persisted across relo
|
||||
assert.equal(snapshot.jobs.active[0]?.safeLabel || snapshot.jobs.queued[0]?.safeLabel, 'Recoverable');
|
||||
});
|
||||
|
||||
test('async recovery handlers are awaited before restoring jobs', async () => {
|
||||
const window = loadJobs();
|
||||
const initial = makeProvider({ providerId: 'provider.async-recover', recoverySupport: { queued: true, running: true, paused: true } });
|
||||
await dispatch(window, 'register-provider', { provider: initial.provider });
|
||||
await dispatch(window, 'enqueue', enqueuePayload({ logicalJobKey: 'async-recover', safeLabel: 'Async Recover' }));
|
||||
|
||||
window.slopsmith.jobs.resetForTests({ clearStorage: false });
|
||||
const recovered = makeProvider({
|
||||
providerId: 'provider.async-recover',
|
||||
recoverySupport: { queued: true, running: true, paused: true },
|
||||
operationHandlers: {
|
||||
'job.recover': async () => ({ outcome: 'handled', state: 'paused' }),
|
||||
},
|
||||
});
|
||||
await dispatch(window, 'register-provider', { provider: recovered.provider });
|
||||
const snapshot = diagnosticsSnapshot(window);
|
||||
|
||||
assert.equal(snapshot.jobs.paused.length, 1);
|
||||
assert.equal(snapshot.jobs.paused[0].safeLabel, 'Async Recover');
|
||||
});
|
||||
|
||||
test('async recovery rejections become provider-unavailable terminal jobs', async () => {
|
||||
const window = loadJobs();
|
||||
const initial = makeProvider({ providerId: 'provider.recover-reject', recoverySupport: { queued: true, running: true, paused: true } });
|
||||
await dispatch(window, 'register-provider', { provider: initial.provider });
|
||||
await dispatch(window, 'enqueue', enqueuePayload({ logicalJobKey: 'recover-reject', safeLabel: 'Reject Recover' }));
|
||||
|
||||
window.slopsmith.jobs.resetForTests({ clearStorage: false });
|
||||
const rejecting = makeProvider({
|
||||
providerId: 'provider.recover-reject',
|
||||
recoverySupport: { queued: true, running: true, paused: true },
|
||||
operationHandlers: {
|
||||
'job.recover': async () => { throw new Error('failed near /Users/example/private/recovery.db'); },
|
||||
},
|
||||
});
|
||||
await dispatch(window, 'register-provider', { provider: rejecting.provider });
|
||||
const snapshot = diagnosticsSnapshot(window);
|
||||
|
||||
assert.equal(snapshot.jobs.recentTerminal[0].state, 'provider-unavailable');
|
||||
assert.equal(snapshot.jobs.recentTerminal[0].terminalOutcome.category, 'provider-unavailable');
|
||||
assert.doesNotMatch(JSON.stringify(snapshot), /Users\/example|recovery\.db/);
|
||||
});
|
||||
|
||||
test('reload marks non-recoverable jobs orphaned or provider-unavailable without restoring raw payloads', async () => {
|
||||
const window = loadJobs();
|
||||
const { provider } = makeProvider({ providerId: 'provider.no-recover', recoverySupport: { queued: false, running: false, paused: false } });
|
||||
@@ -81,7 +138,7 @@ test('diagnostics enforce per-job history and bounded snapshot size with termina
|
||||
for (let index = 0; index < 8; index += 1) {
|
||||
const enqueued = await dispatch(window, 'enqueue', enqueuePayload({ logicalJobKey: `terminal-${index}`, safeLabel: `Terminal ${index}` }));
|
||||
lastJobId = enqueued.payload.job.jobId;
|
||||
for (let line = 0; line < 80; line += 1) window.slopsmith.jobs.log(provider.providerId, lastJobId, `line ${line} /Users/example/private/file-${line}.psarc`);
|
||||
for (let line = 0; line < 80; line += 1) window.slopsmith.jobs.log(provider.providerId, lastJobId, `line ${line} /Users/example/private/file-${line}.sloppak`);
|
||||
window.slopsmith.jobs.complete(provider.providerId, lastJobId, { resultSummary: 'done' });
|
||||
}
|
||||
|
||||
|
||||
@@ -44,12 +44,26 @@ test('privileged enqueue requires approval before provider work starts', async (
|
||||
assert.equal(diagnosticsContributions(window).jobs.schema, 'slopsmith.jobs.diagnostics.v1');
|
||||
});
|
||||
|
||||
test('approved continuation enqueue matches the stored approval scope key', async () => {
|
||||
const window = loadJobs();
|
||||
const { calls, provider } = makeProvider({ capacity: { maxRunning: 2, maxQueued: 10 } });
|
||||
await dispatch(window, 'register-provider', { provider });
|
||||
const request = enqueuePayload({ target: { path: '/Users/example/DLC/Song A.sloppak' }, inputs: { token: 'secret-a' }, logicalJobKey: '' });
|
||||
const first = await dispatch(window, 'enqueue', request);
|
||||
const approvalScopeKey = window.slopsmith.jobs._test.jobs.get(first.payload.job.jobId).approvalScopeKey;
|
||||
|
||||
const continued = await dispatch(window, 'enqueue', { ...request, authorization: 'approved-continuation', approvalScopeKey });
|
||||
|
||||
assert.equal(continued.status, 'applied');
|
||||
assert.equal(calls.length, 2);
|
||||
});
|
||||
|
||||
test('approved enqueue queues and starts with redaction-safe public job fields', async () => {
|
||||
const window = loadJobs();
|
||||
const { calls, provider } = makeProvider({ capacity: { maxRunning: 1, maxQueued: 10 } });
|
||||
await dispatch(window, 'register-provider', { provider });
|
||||
|
||||
const result = await dispatch(window, 'enqueue', enqueuePayload({ target: { path: '/Users/example/DLC/Secret.psarc' }, inputs: { token: 'secret', safeFingerprint: 'fingerprint-1' } }));
|
||||
const result = await dispatch(window, 'enqueue', enqueuePayload({ target: { path: '/Users/example/DLC/Secret.sloppak' }, inputs: { token: 'secret', safeFingerprint: 'fingerprint-1' } }));
|
||||
const job = result.payload.job;
|
||||
|
||||
assert.equal(result.status, 'applied');
|
||||
@@ -57,7 +71,7 @@ test('approved enqueue queues and starts with redaction-safe public job fields',
|
||||
assert.equal(job.targetRef.startsWith('target-'), true);
|
||||
assert.equal(job.inputFingerprint, 'fingerprint-1');
|
||||
assert.equal(calls.length, 1);
|
||||
assert.doesNotMatch(JSON.stringify(diagnosticsSnapshot(window)), /Secret\.psarc|token|secret/);
|
||||
assert.doesNotMatch(JSON.stringify(diagnosticsSnapshot(window)), /Secret\.sloppak|token|secret/);
|
||||
});
|
||||
|
||||
test('list and inspect are prompt-free and do not invoke provider callbacks', async () => {
|
||||
|
||||
@@ -75,7 +75,7 @@ test('provider enqueue exceptions fail safely without leaking private details',
|
||||
const window = loadJobs();
|
||||
const { provider } = makeProvider({
|
||||
operationHandlers: {
|
||||
'job.enqueue': () => { throw new Error('failed near /Users/example/private/song.psarc token=abc123'); },
|
||||
'job.enqueue': () => { throw new Error('failed near /Users/example/private/song.sloppak token=abc123'); },
|
||||
},
|
||||
});
|
||||
await dispatch(window, 'register-provider', { provider });
|
||||
@@ -87,7 +87,7 @@ test('provider enqueue exceptions fail safely without leaking private details',
|
||||
assert.equal(result.outcome, 'failed');
|
||||
assert.equal(snapshot.jobs.active.length, 0);
|
||||
assert.equal(snapshot.jobs.recentTerminal[0].terminalOutcome.category, 'provider-failure');
|
||||
assert.doesNotMatch(JSON.stringify(snapshot), /Users\/example|song\.psarc|abc123/);
|
||||
assert.doesNotMatch(JSON.stringify(snapshot), /Users\/example|song\.sloppak|abc123/);
|
||||
});
|
||||
|
||||
test('provider enqueue failed results become terminal failures', async () => {
|
||||
@@ -128,6 +128,20 @@ test('async provider enqueue rejections become terminal provider failures', asyn
|
||||
assert.doesNotMatch(JSON.stringify(snapshot), /Users\/example|cache\.bin/);
|
||||
});
|
||||
|
||||
test('retry accepts approved continuation using the stored scope key', async () => {
|
||||
const window = loadJobs();
|
||||
const { provider } = makeProvider({ providerId: 'provider.retry-continuation' });
|
||||
await dispatch(window, 'register-provider', { provider });
|
||||
const enqueued = await dispatch(window, 'enqueue', enqueuePayload({ providerId: provider.providerId, logicalJobKey: 'retry-continuation' }));
|
||||
const jobId = enqueued.payload.job.jobId;
|
||||
const approvalScopeKey = window.slopsmith.jobs._test.jobs.get(jobId).approvalScopeKey;
|
||||
|
||||
window.slopsmith.jobs.fail(provider.providerId, jobId, { category: 'provider-failure', safeReason: 'retryable failure', retryable: true });
|
||||
const retried = await dispatch(window, 'retry', { jobId, authorization: 'approved-continuation', approvalScopeKey });
|
||||
|
||||
assert.equal(retried.status, 'retry-started');
|
||||
});
|
||||
|
||||
test('cancel, pause, resume, retry, and stale transitions use canonical outcomes', async () => {
|
||||
const window = loadJobs();
|
||||
const { provider } = makeProvider({ capacity: { maxRunning: 1, maxQueued: 10 } });
|
||||
|
||||
Reference in New Issue
Block a user