Fix phase advance check by vas3a · Pull Request #34 · topcoder-platform/autopilot-v6

vas3a · 2026-03-27T14:27:34Z

The biggest problem is a key-format mismatch in sync.

In autopilot-v6/src/autopilot/services/scheduler.service.ts, scheduled job IDs are stored as challengeId|phaseId.

But autopilot-v6/src/sync/sync.service.ts reads those raw keys, then later looks schedules up using challengeId:phaseId and removes them by splitting on : in autopilot-v6/src/sync/sync.service.ts and autopilot-v6/src/sync/sync.service.ts.

That explains these log patterns:

repeated “New active phase found … Scheduling …” for the same phase every 3 minutes

immediate “Obsolete schedule found …”

No active schedule found for challenge ...|..., phase undefined

So that part is a real bug.

Second issue: the same scheduled transition is being executed twice.

autopilot-v6/src/autopilot/services/scheduler.service.ts does both:

triggerKafkaEvent(...)

advancePhase(...)

If the Kafka consumer also processes that same event and advances the phase, you get exactly what the logs show:

event consumed

Advancing phase ...

same phase advanced again

one succeeds, the other errors with “already closed”

That also lines up with the BullMQ removal race:

Job ... could not be removed because it is locked by another worker

Third issue: open review phases are reprocessed on every challenge update.

autopilot-v6/src/autopilot/services/phase-schedule-manager.service.ts scans already-open review/screening/approval phases and calls handlePhaseOpenedForChallenge() again.

That matches:

review phase opens and creates 1 pending review

later manual phase detection processes the same open review phase

then it creates 2 more pending reviews

That may be intentional as backfill, but it is definitely another re-entry path.

Fourth issue: review opportunity creation is noisy and likely non-idempotent.

autopilot-v6/src/autopilot/services/phase-schedule-manager.service.ts calls review opportunity creation on every ACTIVE challenge update.

The create flow is list-then-create in autopilot-v6/src/autopilot/services/phase-schedule-manager.service.ts.

That makes 409 conflicts unsurprising under races or stale reads. It should probably treat HTTP 409 as “already exists” instead of an error.

So: yes, there are real issues here, not just noisy logs.

Fixes implemented:

Fixed sync key mismatch in sync.service.ts:
- Uses scheduler job IDs (challenge|phase) for map lookups/comparisons.
- Adds robust job-id parsing for cleanup, with safe skip + warning on malformed IDs.
- Removes the phase undefined cancellation pattern caused by : splitting.
Removed duplicate execution path in scheduler.service.ts:
- runScheduledTransition() now emits the Kafka transition event and no longer also calls local advancePhase().
- Prevents “same END processed twice / already closed” behavior from scheduler path.
Added manual open-phase dedupe in phase-schedule-manager.service.ts:
- Tracks per challenge+phase fingerprint for open-phase processing.
- Skips reprocessing unchanged open review/screening/approval phases on repeated challenge updates.
- Prunes stale phase entries when phases are no longer open.

Tests

Added regression test in sync.service.spec.ts for pipe-delimited job IDs + obsolete cancellation parsing.
Added regression test in phase-schedule-manager.service.spec.ts to ensure unchanged open phases are processed once.
Verified: targeted suites pass (2/2).
Note: the broad scheduler suite is currently noisy/flaky in this env (Redis/connect + timeout), but this was outside the targeted change verification.

github-actions · 2026-03-27T14:27:50Z

-      {
-        setPhaseChainCallback: jest.fn(),
-      } as any,
+      schedulerService as any,


[⚠️ maintainability]
Casting schedulerService and phaseReviewService to any may hide type errors. Consider defining proper interfaces for these services to ensure type safety.

github-actions · 2026-03-27T14:27:50Z

      challengeApiService as any,
-      {} as any,
+      phaseReviewService as any,
      {} as any,


[⚠️ maintainability]
The use of as any for dependency injection can lead to runtime errors if the injected objects do not conform to expected interfaces. Consider defining and using specific interfaces for these dependencies.

github-actions · 2026-03-27T14:27:56Z

+  }
+
+  private buildOpenPhaseFingerprint(phase: IPhase): string {
+    const effectiveStart = phase.actualStartDate ?? phase.scheduledStartDate;


[⚠️ correctness]
The buildOpenPhaseFingerprint method uses a mix of actual and scheduled dates to build the fingerprint. If both actualStartDate and scheduledStartDate are null, it defaults to 'UNKNOWN'. Consider whether this is the desired behavior or if it might lead to unintended deduplication issues.

github-actions · 2026-03-27T14:27:56Z

+      ),
+    );
+
+    for (const key of this.processedOpenPhaseFingerprints.keys()) {


[⚠️ performance]
In pruneOpenPhaseProcessingState, the loop iterates over all keys in processedOpenPhaseFingerprints. If the map grows large, this could become a performance bottleneck. Consider optimizing this by maintaining a separate structure to track keys by challenge ID.

github-actions · 2026-03-27T14:28:03Z

+      buildJobId: jest
+        .fn()
+        .mockImplementation((challengeId: string, phaseId: string) => {
+          return `${challengeId}|${phaseId}`;


[💡 style]
Consider using a template literal directly in the return statement for simplicity: return ${challengeId}|${phaseId};.

github-actions · 2026-03-27T14:28:03Z

    });
  });
+
+  it('matches scheduled jobs by BullMQ job ID and cancels obsolete job IDs safely', async () => {


[💡 readability]
The test case name 'matches scheduled jobs by BullMQ job ID and cancels obsolete job IDs safely' is quite long and could be simplified for better readability. Consider shortening it while maintaining clarity.

github-actions · 2026-03-27T14:28:07Z

          );
-          const [challengeId, phaseId] = scheduledPhaseKey.split(':');
+
+          const parsedJob = this.parseScheduledJobId(scheduledJobId);


[💡 maintainability]
The parseScheduledJobId method returns null if the job ID does not contain a delimiter or if the split results in empty challengeId or phaseId. Consider logging these cases for better traceability and debugging.

Copilot

Pull request overview

Fixes repeated rescheduling/cancellation churn and duplicate phase-transition execution by aligning scheduler job-id handling across sync + scheduler, and adding dedupe for manual open-phase processing.

Changes:

Sync now matches/cancels scheduled transitions using BullMQ job IDs (challengeId|phaseId) and adds safer parsing for cleanup.
Scheduler no longer advances phases locally when a scheduled transition fires; it only emits the Kafka transition event.
Manual open-phase processing is deduped per challenge+phase fingerprint, with state pruning for phases that are no longer open.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/sync/sync.service.ts	Uses scheduler job IDs for lookups/comparisons; adds parsing helper for cancellation cleanup.
src/sync/sync.service.spec.ts	Adds regression coverage for pipe-delimited job IDs and obsolete cancellation behavior.
src/autopilot/services/scheduler.service.ts	Removes local `advancePhase()` call from scheduled transition execution to avoid double-processing.
src/autopilot/services/phase-schedule-manager.service.ts	Adds dedupe state for processing open review/screening/approval phases and pruning helper.
src/autopilot/services/phase-schedule-manager.service.spec.ts	Adds regression coverage to ensure unchanged open phases are only processed once.
.circleci/config.yml	Adjusts workflow filters to include `dev-*` tag builds for the DEV workflow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-30T08:16:57Z

+              `[MANUAL PHASE DETECTION] Skipping unchanged open phase ${phase.id} (${phase.name}) for challenge ${challengeDetails.id}; already processed for current phase-open fingerprint.`,
+            );
+            continue;


The open-phase dedupe fingerprint is based only on phase fields, but the processing logic also depends on challenge.reviewers (via resolveScorecardIdForOpenPhase). If reviewer configs/scorecard IDs change while the phase remains open, the fingerprint may be unchanged and this block will skip both pending-review scorecard updates and handlePhaseOpenedForChallenge, leaving stale pending reviews. Consider including the relevant reviewer/scorecard inputs in the fingerprint (e.g., selected scorecardId(s) for the phase) or running scorecard updates even when the phase fingerprint is unchanged.

Suggested change

`[MANUAL PHASE DETECTION] Skipping unchanged open phase ${phase.id} (${phase.name}) for challenge ${challengeDetails.id}; already processed for current phase-open fingerprint.`,

);

continue;

`[MANUAL PHASE DETECTION] Open phase ${phase.id} (${phase.name}) for challenge ${challengeDetails.id} has unchanged fingerprint; proceeding to re-run reviewer/scorecard processing.`,

);

Copilot · 2026-03-30T08:16:57Z

      if (openPhasesRequiringScorecards.length > 0) {
+        this.pruneOpenPhaseProcessingState(
+          challengeDetails.id,
+          openPhasesRequiringScorecards,
+        );
+


processedOpenPhaseFingerprints entries are only pruned when openPhasesRequiringScorecards.length > 0. If a challenge previously had an open review/screening/approval phase and later has none (or transitions out of ACTIVE and returns early), the per-challenge entries will never be deleted, so this map can grow unbounded over time. Consider pruning/clearing the challenge's entries when there are no open phases requiring scorecards (and/or when skipping non-ACTIVE challenges).

devin-ai-integration

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

jmgasper

Looks good - what's the best way to test this out in dev?

…to fix-phase-advance-check

vas3a added 2 commits March 27, 2026 14:36

Fix phase advance check

6939da8

deploy

ed9b479

vas3a requested a review from jmgasper March 27, 2026 14:27

github-actions Bot reviewed Mar 27, 2026

View reviewed changes

kkartunov requested a review from Copilot March 30, 2026 08:12

Copilot started reviewing on behalf of kkartunov March 30, 2026 08:12 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

devin-ai-integration Bot reviewed Mar 30, 2026

View reviewed changes

kkartunov self-requested a review March 30, 2026 12:39

kkartunov approved these changes Mar 30, 2026

View reviewed changes

jmgasper approved these changes Mar 31, 2026

View reviewed changes

vas3a added 2 commits April 15, 2026 15:26

erge branch 'develop' of github.com:topcoder-platform/autopilot-v6 in…

c80a26d

…to fix-phase-advance-check

fix tests

8cb60b4

vas3a merged commit 5108fd7 into develop Apr 15, 2026
6 of 7 checks passed

vas3a deleted the fix-phase-advance-check branch April 15, 2026 14:01

Conversation

vas3a commented Mar 27, 2026 • edited by devin-ai-integration Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The biggest problem is a key-format mismatch in sync.

Second issue: the same scheduled transition is being executed twice.

Third issue: open review phases are reprocessed on every challenge update.

Fourth issue: review opportunity creation is noisy and likely non-idempotent.

Uh oh!

github-actions Bot Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

✅ Devin Review: No Issues Found

Uh oh!

jmgasper left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vas3a commented Mar 27, 2026 •

edited by devin-ai-integration Bot

Loading