Skip to content

Quality of Life improvements #267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nytamin wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Quality of Life improvements #267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cee6aa4
fix: track all reasons why a worker does not support an expectation.
nytamin Feb 12, 2026
0599314
fix: improve tracking of reasons why a worker does not support an exp…
nytamin Feb 12, 2026
e3fde76
fix: Issue with thrown error
nytamin Feb 12, 2026
83f0c2d
fix: clear state when RESTARTING expectations
nytamin Mar 2, 2026
02e88ff
chore: lint
nytamin Mar 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion shared/packages/api/src/ffmpeg.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export async function testFFProbe(worker: ExecutableAliasSource): Promise<string
}
export async function testFFExecutable(ffExecutable: string): Promise<string | null> {
try {
return new Promise<string | null>((resolve) => {
const result = await new Promise<string | null>((resolve) => {
const ffMpegProcess = spawn(ffExecutable, ['-version'])
let output = ''
ffMpegProcess.stderr.on('data', (data) => {
Expand Down Expand Up @@ -75,6 +75,7 @@ export async function testFFExecutable(ffExecutable: string): Promise<string | n
}
})
})
return result
} catch (err) {
return `Error when spawning process ${ffExecutable}: ${stringifyError(err)}`
}
Expand Down
16 changes: 12 additions & 4 deletions shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/new.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,21 @@ export async function evaluateExpectationStateNew({ manager, tracker, trackedExp
})
}
} else {
let anyNoAvailableWorkersReasonUser = ''
const allNoAvailableWorkersReasonsTech: string[] = []
for (const reason of trackedExp.noAvailableWorkersReasons.values()) {
if (!anyNoAvailableWorkersReasonUser) anyNoAvailableWorkersReasonUser = reason.user // Just pick the first one
allNoAvailableWorkersReasonsTech.push(reason.tech)
}
if (!anyNoAvailableWorkersReasonUser) anyNoAvailableWorkersReasonUser = 'Unknown reason'

tracker.trackedExpectationAPI.updateTrackedExpectationStatus(trackedExp, {
state: ExpectedPackageStatusAPI.WorkStatusState.NEW,
reason: {
user: `Found no workers who supports this Expectation, due to: ${trackedExp.noAvailableWorkersReason.user}`,
tech: `Found no workers who supports this Expectation: "${
trackedExp.noAvailableWorkersReason.tech
}", have asked workers: [${Array.from(trackedExp.queriedWorkers.keys()).join(',')}]`,
user: `Found no workers who supports this Expectation, due to: ${anyNoAvailableWorkersReasonUser}`,
tech: `Found no workers who supports this Expectation. Reasons why: ${allNoAvailableWorkersReasonsTech.join(
', '
)}, have asked workers: [${Array.from(trackedExp.queriedWorkers.keys()).join(',')}]`,
},
// Don't update the package status, since we don't know anything about the package yet:
dontUpdatePackage: true,
Expand Down
6 changes: 6 additions & 0 deletions ...d/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/restarted.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ export async function evaluateExpectationStateRestarted({
}: EvaluateContext): Promise<void> {
assertState(trackedExp, ExpectedPackageStatusAPI.WorkStatusState.RESTARTED)

// Clear this state, so that they will be repopulated in NEW:
trackedExp.queriedWorkers.clear()
trackedExp.availableWorkers.clear()
trackedExp.noAvailableWorkersReasons.clear()

await manager.workerAgents.assignWorkerToSession(trackedExp)
Comment on lines +16 to 21
Copy link

@coderabbitai coderabbitai bot Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for evaluateExpectationStateRestarted and related restart/restarted patterns
echo "=== Searching for restart-related code ==="
rg -n -C4 --type=ts 'evaluateExpectationStateRestarted|WorkStatusState\.RESTARTED' | head -100

echo -e "\n=== Searching for assignWorkerToSession ==="
rg -n -C6 --type=ts 'assignWorkerToSession\s*\('

echo -e "\n=== Searching for noWorkerAssigned ==="
rg -n -C4 --type=ts 'noWorkerAssigned\s*\('

echo -e "\n=== Searching for waitingForWorkerTime ==="
rg -n -C4 --type=ts 'waitingForWorkerTime\b'

Repository: Sofie-Automation/sofie-package-manager

Length of output: 39882

🏁 Script executed:

#!/bin/bash
# Get the structure of the restarted.ts file to understand the full restart flow
echo "=== Reading restarted.ts file ==="
wc -l shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/restarted.ts
head -50 shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/restarted.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 2038

🏁 Script executed:

#!/bin/bash
# Look for assignWorkerToSession implementation to understand what it does
echo "=== Finding assignWorkerToSession implementation ==="
fd -t f -e ts | xargs rg -l 'assignWorkerToSession.*=' | head -5

Repository: Sofie-Automation/sofie-package-manager

Length of output: 134

🏁 Script executed:

#!/bin/bash
# Get the full assignWorkerToSession implementation
sed -n '191,250p' shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 1833

🏁 Script executed:

#!/bin/bash
# Get the noWorkerAssigned implementation to understand what happens when no worker is available
sed -n '158,175p' shared/packages/expectationManager/src/expectationTracker/lib/trackedExpectationAPI.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 795

🏁 Script executed:

#!/bin/bash
# Check if there's any logic that delays assignment based on noWorkerAssignedTime or cleared state
rg -n -C8 'noWorkerAssignedTime' shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/restarted.ts
rg -n -C5 'noWorkerAssignedTime' shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationState.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 64

🏁 Script executed:

#!/bin/bash
# Search for any logic that would prevent assignment when state is just cleared
rg -n -B3 -A3 'queriedWorkers\|availableWorkers' shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts | head -60

Repository: Sofie-Automation/sofie-package-manager

Length of output: 64

🏁 Script executed:

#!/bin/bash
# Find and examine determineBestWorkerForExpectation
rg -n -A30 'async determineBestWorkerForExpectation' shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 1333

🏁 Script executed:

#!/bin/bash
# Check if there are any guard conditions that would skip finding a worker after clearing
rg -n -B5 -A15 'updateAvailableWorkersForExpectation' shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts | head -80

Repository: Sofie-Automation/sofie-package-manager

Length of output: 1763

🏁 Script executed:

#!/bin/bash
# Look at the flow to see if clearing state would affect the assignment in the same cycle
echo "=== Check if clearing affects waitingForWorkerTime ==="
rg -n 'waitingForWorkerTime' shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/restarted.ts

echo -e "\n=== Full restarted.ts context around assignment ==="
sed -n '14,50p' shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/restarted.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 1519

🏁 Script executed:

#!/bin/bash
# Check if clearing state should include resetting waitingForWorkerTime to trigger updateAvailableWorkersForExpectation
echo "=== Check how waitingForWorkerTime is managed in restart ==="
rg -n 'waitingForWorkerTime.*=' shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/ -A2 -B2

echo -e "\n=== Check if noWorkerAssigned sets waitingForWorkerTime ==="
sed -n '158,185p' shared/packages/expectationManager/src/expectationTracker/lib/trackedExpectationAPI.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 1244

🏁 Script executed:

#!/bin/bash
# Compare with how other state handlers handle worker assignment
echo "=== Check aborted.ts for comparison ==="
cat shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/aborted.ts

echo -e "\n=== Check new.ts to see if clearing is redundant ==="
cat shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/new.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 5704

🏁 Script executed:

#!/bin/bash
# Check the complete noWorkerAssigned flow to understand state transitions
sed -n '158,195p' shared/packages/expectationManager/src/expectationTracker/lib/trackedExpectationAPI.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 1427

🏁 Script executed:

#!/bin/bash
# Verify: does clearing state prevent immediate removal in the same cycle?
# Check if the removal would be skipped when no worker is assigned
echo "=== Trace the RESTARTED flow path ==="
echo "1. Clear state in restarted.ts"
echo "2. Call assignWorkerToSession"
echo "3. If availableWorkers is empty and waitingForWorkerTime is null, what happens?"
echo ""
echo "Check if waitingForWorkerTime affects state update:"
rg -n 'waitingForWorkerTime' shared/packages/expectationManager/src/expectationTracker/lib/trackedExpectationAPI.ts -B3 -A3

Repository: Sofie-Automation/sofie-package-manager

Length of output: 313

🏁 Script executed:

#!/bin/bash
# Compare RESTARTED vs ABORTED - why does RESTARTED clear but ABORTED doesn't?
echo "=== ABORTED (without clearing) ==="
sed -n '15,25p' shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/aborted.ts

echo -e "\n=== RESTARTED (with clearing) ==="
sed -n '14,22p' shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/restarted.ts

echo -e "\n=== Check if determineBestWorkerForExpectation needs populated availableWorkers ==="
sed -n '100,120p' shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 1780

🏁 Script executed:

#!/bin/bash
# Critical: Check if clearing availableWorkers prevents finding workers in determineBestWorkerForExpectation
echo "=== Check the exact flow when availableWorkers is empty ==="
echo "In determineBestWorkerForExpectation, line 112:"
echo "const workerIds = Array.from(trackedExp.availableWorkers.keys())"
echo ""
echo "After clearing, availableWorkers.size = 0"
echo "So workerIds will be empty array"
echo "PromisePool.for([]).process() will not execute any iterations"
echo "bestWorker will be undefined"
echo ""
echo "=== Check updateAvailableWorkersForExpectation trigger ==="
sed -n '215,220p' shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts

echo -e "\n=== Does clearing reset waitingForWorkerTime? ==="
grep -n 'waitingForWorkerTime' shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/restarted.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 845

🏁 Script executed:

#!/bin/bash
# Check if noWorkerAssigned clears or sets waitingForWorkerTime
sed -n '158,210p' shared/packages/expectationManager/src/expectationTracker/lib/trackedExpectationAPI.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 2010

🏁 Script executed:

#!/bin/bash
# Verify: When state transitions to NEW via noWorkerAssigned, is the removal message already sent?
# Check the session state during the transition
rg -n -B5 -A10 'state: ExpectedPackageStatusAPI.WorkStatusState.NEW' shared/packages/expectationManager/src/expectationTracker/lib/trackedExpectationAPI.ts | head -50

Repository: Sofie-Automation/sofie-package-manager

Length of output: 1477

🏁 Script executed:

#!/bin/bash
# Final verification: Does clearing state actually matter, or is it redundant with NEW state?
echo "=== Check if restarted.ts actually needs to clear state ==="
echo "After noWorkerAssigned, state transitions to NEW"
echo "NEW state calls updateAvailableWorkersForExpectation (which repopulates workers)"
echo ""
echo "So the question: Is the clearing necessary at all?"
echo ""
echo "=== Check what state is AFTER clearing and BEFORE calling assignWorkerToSession ==="
echo "State is still RESTARTED when clearing happens"
echo "So if assignWorkerToSession finds a worker, it will:"
echo "1. Assign the worker"
echo "2. Call removeExpectation in RESTARTED state"
echo "3. Transition to NEW state"
echo ""
echo "If no worker found (empty availableWorkers after clear):"
echo "1. noWorkerAssigned() called in RESTARTED state"
echo "2. Transitions to NEW (because availableWorkers.size === 0)"
echo "3. Next cycle: NEW state re-queries workers"
echo ""
echo "=== The concern: Does clearing + empty availableWorkers skip the removal? ==="
sed -n '21,49p' shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/restarted.ts

Repository: Sofie-Automation/sofie-package-manager

Length of output: 1958

🏁 Script executed:

#!/bin/bash
# Check if there's any scenario where workers wouldn't be re-queried
echo "=== If state goes to NEW, will updateAvailableWorkersForExpectation always run? ==="
sed -n '8,35p' shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/new.ts

echo -e "\n=== Conclusion check: Can assignment and removal happen in same cycle after restart? ==="
echo "YES, IF availableWorkers is not empty after clearing"
echo "NO, IF availableWorkers is empty after clearing (because updateAvailableWorkersForExpectation is skipped)"

Repository: Sofie-Automation/sofie-package-manager

Length of output: 1680

Clear worker state before assignment forces an extra iteration in restart flows.

Clearing queriedWorkers, availableWorkers, and noAvailableWorkersReasons (lines 17-19) before calling assignWorkerToSession (line 21) prevents worker assignment in the same cycle. Since assignWorkerToSession only calls updateAvailableWorkersForExpectation when waitingForWorkerTime !== null, the cleared availableWorkers set remains empty, causing determineBestWorkerForExpectation to find no workers. This triggers noWorkerAssigned(), which transitions to NEW state. The removal operation then cannot execute until the next cycle when NEW state re-queries workers.

Consider either:

  1. Resetting waitingForWorkerTime when clearing to trigger immediate worker re-query in the same cycle, or
  2. Accepting the extra iteration as intentional by updating the comment to explain this behavior
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@shared/packages/expectationManager/src/evaluationRunner/evaluateExpectationStates/restarted.ts`
around lines 16 - 21, The code clears trackedExp.queriedWorkers,
trackedExp.availableWorkers, and trackedExp.noAvailableWorkersReasons before
calling manager.workerAgents.assignWorkerToSession which prevents immediate
worker selection because assignWorkerToSession only re-populates
availableWorkers when trackedExp.waitingForWorkerTime !== null; fix by either
(A) resetting trackedExp.waitingForWorkerTime to a non-null value immediately
after clearing those sets so
updateAvailableWorkersForExpectation/determineBestWorkerForExpectation can run
in the same cycle and avoid transitioning to noWorkerAssigned/NEW, or (B) if the
extra iteration is intended, update the inline comment above the clears to
explicitly state that clearing these sets forces a new worker query in the next
cycle and is an intentional behavior to avoid in-cycle reassignments. Ensure
changes reference trackedExp and manager.workerAgents.assignWorkerToSession and
preserve existing state transition logic (determineBestWorkerForExpectation,
noWorkerAssigned, NEW).

if (trackedExp.session.assignedWorker) {
// Start by removing the expectation
Expand All @@ -30,6 +35,7 @@ export async function evaluateExpectationStateRestarted({
})
} else {
// Something went wrong when trying to remove
// Todo: Should we go to NEW here, if it happens multiple times?
tracker.trackedExpectationAPI.updateTrackedExpectationStatus(trackedExp, {
state: ExpectedPackageStatusAPI.WorkStatusState.RESTARTED,
reason: removed.reason,
Expand Down
21 changes: 15 additions & 6 deletions shared/packages/expectationManager/src/internalManager/lib/trackedWorkerAgents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,28 @@ export class TrackedWorkerAgents {

if (support.support) {
trackedExp.availableWorkers.add(workerId)
trackedExp.noAvailableWorkersReasons.delete(workerId)
} else {
trackedExp.availableWorkers.delete(workerId)
trackedExp.noAvailableWorkersReason = support.reason
trackedExp.noAvailableWorkersReasons.set(workerId, {
user: support.reason.user,
tech: `${workerId}: ${support.reason.tech}`,
})
}
} catch (err) {
trackedExp.availableWorkers.delete(workerId)

if ((err + '').match(/timeout/i)) {
trackedExp.noAvailableWorkersReason = {
trackedExp.noAvailableWorkersReasons.set(workerId, {
user: 'Worker timed out',
tech: `Worker "${workerId} timeout"`,
}
} else throw err
tech: `${workerId}: Timeout in doYouSupportExpectation()`,
})
} else {
trackedExp.noAvailableWorkersReasons.set(workerId, {
user: 'Error in Worker',
tech: `${workerId}: Error thrown: ${stringifyError(err)}`,
})
}
}
}
})
Expand All @@ -94,7 +103,7 @@ export class TrackedWorkerAgents {
}> {
/** How many requests to send out simultaneously */
const BATCH_SIZE = 10
/** How many answers we want to have before continuing with picking one */
/** If we've gotten this amount of positive answers, we won't be asking more workers */
const minWorkerCount = 5

let countQueried = 0
Expand Down
9 changes: 3 additions & 6 deletions shared/packages/expectationManager/src/lib/trackedExpectation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export interface TrackedExpectation {
queriedWorkers: Map<WorkerAgentId, number>
/** List of worker ids that supports this Expectation */
availableWorkers: Set<WorkerAgentId>
/** Contains the latest reason why a worker refused to support an Expectation */
noAvailableWorkersReason: Reason
/** Contains the reasons why a worker refused to support an Expectation */
noAvailableWorkersReasons: Map<WorkerAgentId, Reason>
/** Timestamp of the last time the expectation was evaluated. */
lastEvaluationTime: number
/**
Expand Down Expand Up @@ -113,10 +113,7 @@ export function getDefaultTrackedExpectation(
state: existingtrackedExp?.state || ExpectedPackageStatusAPI.WorkStatusState.NEW,
queriedWorkers: new Map(),
availableWorkers: new Set(),
noAvailableWorkersReason: {
user: 'Unknown reason',
tech: 'N/A (init)',
},
noAvailableWorkersReasons: new Map(),
lastEvaluationTime: 0,
skipEvaluationCount: 0,
waitingForWorkerTime: null,
Expand Down
1 change: 1 addition & 0 deletions shared/packages/expectationManager/src/lib/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface ExpectationStateHandlerSession {

/** The Worker assigned to the Expectation during this evaluation-session */
assignedWorker?: WorkerAgentAssignment
/** If no Worker was assigned, a Reason why */
noAssignedWorkerReason?: Reason
}
export interface WorkerAgentAssignment {
Expand Down
3 changes: 2 additions & 1 deletion shared/packages/worker/src/worker/workers/genericWorker/lib/executableDependencyHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class ExecutableDependencyHandler {

async testFFExecutable(executable: string): Promise<string | null> {
try {
return new Promise<string | null>((resolve) => {
const result = await new Promise<string | null>((resolve) => {
const execProcess = spawn(executable, ['-v'])

// Guard against the process not exiting on its own:
Expand All @@ -88,6 +88,7 @@ export class ExecutableDependencyHandler {
resolve(null)
})
})
return result
} catch (err) {
return `Error when spawning process ${executable}: ${stringifyError(err)}`
}
Expand Down