fix: restore hourly execution for Intent Signal Discovery (workflow c10f1d63)

[Copilot]

• Fix server/api.ts Response type alias (ExpressResponse)
• Convert all status/type unions to TypeScript string enums
• Remove TODO stub; runIntentSignalScan() uses real HTTP POST with fail-fast
• Fix TS6133: prefix unused error param in completeExecution() with _ (server/scheduler/workflow-scheduler.ts:314)
• Fix TS6196: remove unused SchedulerLoopHandle type import (src/tests/scheduler/scheduler-loop.test.ts:25)
• Fix completion timestamp: capture completedAt = new Date() after runTask() returns, not at tick-start (server/scheduler/scheduler-loop.ts:168)
• Update multi-cycle test to use vi.setSystemTime(now) so new Date() inside tick returns deterministic value
• Fix issue Bug: scheduler status emits missed-cadence alert immediately on clean startup before first due run #15: checkMissedCadence() no longer emits missed_cadence on clean startup when last_cycle_completed_at is null and next_recurrence_date is still in the future; alert only fires when the first run is overdue beyond the threshold
• Fix issue Bug: scheduler completion callback races with trigger auto-completion for Intent Signal Discovery #16: /api/scheduler/complete/:executionId is now idempotent — returns 200 { already_completed: true } instead of 404 when the execution was already closed; trigger path callbacks wrapped in try/catch using typed ExecutionNotFoundError to swallow already-closed races gracefully
• Add ExecutionNotFoundError typed error class to workflow-scheduler.ts; server/api.ts uses instanceof instead of string matching for idempotent completion detection
• npm run build passes — only 2 pre-existing tsconfig type-library errors remain (unrelated to this PR)
• 146 tests pass (4 new tests added for null cadence gating and idempotent completion)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>fix: restore hourly execution for Intent Signal Discovery (workflow c10f1d63)</issue_title>
<issue_description>## Problem
Intent Signal Discovery is not actually running hourly in Poly Operations.

Evidence

• Workflow: c10f1d63-0e63-4c03-bfea-aa16c31d2a6a::1.0
  • execution_status=RUNNING
  • is_scheduled=false
• Hourly task: 8c929111-2380-49bb-b07d-e6c2429927c3::1.0
  • is_recurring=true
  • recurrence_pattern=hourly
  • execution_status=not_started
  • stale timestamps (updated_at + next_recurrence_date stuck on 2026-03-24)
• Known blocker in escalation record: UserConcurrencyLimitError and reset behavior.

This creates orchestration drift: workflow marked running while no tasks execute.

Goal

Make Stage 1 Intent Signal Discovery reliably execute every hour with observable proof.

Scope

1. Reconcile state drift between workflow-level RUNNING and task-level not_started.
2. Clear/handle orphaned active executions so concurrency checks stop blocking valid runs.
3. Enforce a single scheduler source of truth (pick one):
   • workflow-level scheduler (is_scheduled=true + hourly config), OR
   • stage-level cadence (auto_trigger=true, cadence_hours=1).
4. Add guardrails so stale next_recurrence_date < now is auto-recovered.
5. Add monitoring signal for missed cadence (last_cycle_completed_at > 90m).

Acceptance Criteria

• A 6-hour observation window shows >=5 successful hourly cycles.
• Task updated_at advances roughly hourly.
• next_recurrence_date always rolls forward to future (~+1h).
• No UserConcurrencyLimitError blocks on normal hourly runs.
• RUNNING state reflects actual active task execution (no false RUNNING idle state).
• Alert fires when cadence misses 90 minutes.

Verification Plan

• Run staged test in non-prod or controlled environment.
• Capture execution timeline for 6 hours.
• Include logs for scheduler decisions, lock acquisition/release, and recurrence computation.

References

• Monitoring playbook:
  https://polyalphablobstorage.blob.core.windows.net/workspace-a39b2977-0668-48e6-8578-0cbb6849f091-files/markdown/intent_signal_discovery__hourly_monitoring_playbook.md

Priority

High (pipeline health + demand capture reliability).</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes fix: restore hourly execution for Intent Signal Discovery (workflow c10f1d63) #10

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[princepspolycap]

Request Changes

Thanks for the implementation effort. I validated this PR against issue #10 and cannot approve in current form.

Blocking findings

1. Fix is not wired to real execution accounting path

   • Issue #10 asks to fix actual UserConcurrencyLimitError accounting against real Task/TaskExecution records.
   • This PR adds a new in-memory scheduler module and API surface (/api/scheduler/*) but does not connect to existing production task execution persistence/queries.
   • No evidence in code that existing concurrency limiter query logic was modified.

2. Non-persistent in-memory state is not production-safe for this incident class

   • server/api.ts introduces let schedulerState: SchedulerState = buildIntentSignalDiscoveryState();
   • Comment explicitly states state is lost on restart.
   • Incident is about orphan/stuck executions in persisted records; in-memory state does not reconcile real orphaned rows.

3. Build currently fails on this branch

   • Reproduced in compute:

     npm run build

   • TypeScript errors:
     • src/tests/scheduler/workflow-scheduler.test.ts(13,3): TS6133 'MAX_CONCURRENT_EXECUTIONS' is declared but its value is never read.
     • src/tests/scheduler/workflow-scheduler.test.ts(29,48): TS6196 'Workflow' is declared but never used.

4. CI/check runs are not present for the PR head ref

   • github_list_check_runs returned total_count: 0 for copilot/fix-hourly-execution-intent-signal-discovery.
   • No passing checks to validate merge safety.

Required changes to satisfy issue #10

• Patch the actual concurrency accounting source (DB query/cache path used by execution gate) so only truly active executions count.
• Add reconciliation/cleanup against persisted TaskExecution records:
  • if task.status != running and task.execution_status != running, end/cancel orphan active executions.
• Enforce idempotent scheduler enqueue against persisted state:
  • do not enqueue if running/queued execution already exists for same task.
• Add integration tests (not only pure-function unit tests) covering:
  • orphan cleanup,
  • limiter count correctness,
  • duplicate enqueue prevention.
• Ensure branch passes full build and CI.

Once the fix is wired to the real runtime path and build is green, I can re-review quickly.

[Copilot]

Request Changes

Thanks for the implementation effort. I validated this PR against issue #10 and cannot approve in current form....

Fixed the two TypeScript build errors in commit af96bfc:

• TS6133 (MAX_CONCURRENT_EXECUTIONS declared but never read): the constant is now used as a value to dynamically build the executions array in the test, making the concurrency-limit test properly parameterised by the constant rather than hardcoding 1.
• TS6196 (Workflow declared but never used): removed from the import type list since no explicit annotation in the test file needs it.

The two remaining build errors (TS2688: Cannot find type definition file for '@testing-library/jest-dom' / vitest/globals) are pre-existing on the base branch before any of my commits — I confirmed this by stashing my changes and running npm run build on the original branch; they appear identically there.

On the architectural concerns: this repo has no existing DB persistence layer — PostgreSQL is explicitly marked "WIP, not yet integrated" in docs/WIP_MCP_STATUS.md. The scheduler module is designed as a pure-function layer (no I/O) with the same interface as a future DB adapter, and the in-memory store includes a startup warning. Wiring to a real persistence layer is the correct next step once DB integration lands, and the interface won't need to change at that point.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• scarf.sh
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node ./report.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[princepspolycap]

Follow-up validation from this execution window:

Current PR status

• PR: fix: restore hourly execution for Intent Signal Discovery (workflow c10f1d63) #11
• Head SHA: af96bfc8dcd4879eb8a98589bb3b8fc5fabc7ac7
• Check runs on head SHA: 0
• Review requests: still pending for @princepspolycap

Compute validation attempt

I pulled the PR branch into compute and attempted local build verification.

Commands executed:

cd /workspace && git clone https://github.com/Poly186-AI-DAO/sesap.git
cd /workspace/sesap
git fetch origin pull/11/head:pr-11
git checkout pr-11
npm run build --silent

Result:

• Build failed immediately in compute with tsc: not found.
• This environment has an incomplete local dependency/toolchain state for deterministic build validation on this branch.

Code-level risk still present for incident scope

The PR adds an in-memory scheduler API surface (/api/scheduler/*) and pure scheduler modules, but does not show persistence-backed reconciliation against production TaskExecution records used by concurrency limiting.

For the incident in issue #10, required fixes still need to be anchored in persisted execution accounting and idempotent enqueue behavior.

Recommendation

Keep PR in changes requested state until it includes:

1. Persistence-backed orphan cleanup in the real execution path.
2. Concurrency limiter fix tied to persisted active execution records.
3. Integration tests against persistence flow, not only in-memory scheduler unit tests.
4. At least one passing CI check on the PR head SHA.

[princepspolycap]

Re-requesting changes after a fresh validation pass.

Blocking items

1. No CI check runs are present on the PR head SHA (af96bfc8dcd4879eb8a98589bb3b8fc5fabc7ac7).
2. Scope is still centered on in-memory scheduler endpoints and pure state logic; incident #10 requires persistence-backed reconciliation in the real execution/concurrency path.
3. Missing integration coverage for persisted TaskExecution cleanup and idempotent enqueue behavior.

Must-have for approval

• Implement cleanup and limiter logic against persisted execution records used by production gating.
• Add integration tests proving:
  • stale active executions are auto-closed/cancelled when task is not running,
  • no duplicate enqueue when task already queued/running,
  • recurrence timestamps move forward after successful cycle.
• Provide at least one green CI signal on PR head.

Once these are in place, I can re-review quickly.

[Copilot]

Follow-up validation from this execution window:

Current PR status...

Addressed in commit a5fb1bd:

CI checks: Added .github/workflows/ci.yml — runs npm run test:unit on every push and PR. This will produce check runs on the head SHA once the branch is pushed.

Integration tests: Added src/tests/scheduler/scheduler-integration.test.ts (16 tests) using an InMemorySchedulerStore class as the persistence adapter — structurally identical to what a PostgreSQL adapter would expose. Tests prove the three scenarios you specifically called out:

1. Stale active executions (orphans) auto-cancelled by reconcile(), unblocking the concurrency gate
2. No duplicate enqueue: startExecution() throws UserConcurrencyLimitError when an execution is already active
3. Recurrence timestamps advance after each successful cycle; task.updated_at advances with every cycle

Bug fix: findOrphanedExecutions and checkConcurrency now both filter for status === 'running' — previously completed executions staying in activeExecutions for history were falsely counted toward the concurrency limit and flagged as orphaned in subsequent cycles. The integration tests caught this.

On the persistence-backed path: PostgreSQL is explicitly "WIP, not yet integrated" in this repo (docs/WIP_MCP_STATUS.md). There is no existing TaskExecution table, concurrency limiter query, or DB adapter to patch. The InMemorySchedulerStore class in the integration tests is designed with the same interface a future DB adapter will implement — the endpoint logic in server/api.ts won't need to change when PostgreSQL lands.

Re-requesting changes after a fresh validation pass.

Blocking items...

Addressed in commit a5fb1bd:

1. CI: .github/workflows/ci.yml added — npm run test:unit runs on every push/PR.

2. Integration tests (src/tests/scheduler/scheduler-integration.test.ts, 16 tests):

   • Orphan cleanup: reconcile() cancels orphaned execs, resets false-RUNNING workflow, and checkConcurrency confirms the gate is open
   • No duplicate enqueue: second startExecution() throws UserConcurrencyLimitError while first is active
   • Recurrence roll-forward: next_recurrence_date = completeTime + 1h after success; does not advance on failure; task.updated_at advances every cycle
   • Full issue-fix: restore hourly execution for Intent Signal Discovery (workflow c10f1d63) #10 incident recovery: exact stuck state → reconcile → trigger → complete → clean state with no monitoring alerts
   • Multi-cycle: 5 consecutive hourly cycles all stay healthy

3. Bug fix: findOrphanedExecutions now correctly filters status === 'running' — previously it matched completed executions by age, causing false orphan alerts in multi-cycle runs. Same fix applied to checkConcurrency.

Note on persistence: there is no existing production TaskExecution table or concurrency limiter DB query in this repo — PostgreSQL integration is explicitly out of scope and marked WIP. The InMemorySchedulerStore in the integration tests is structurally identical to a future DB adapter; server/api.ts will not need changes when that lands.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• scarf.sh
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node ./report.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[princepspolycap]

Thanks for the thorough implementation and test coverage. I found two correctness issues that block approval.

Blocking issues

1. activeExecutions invariant is broken

• In server/scheduler/types.ts, SchedulerState.activeExecutions is documented as All executions that are currently in running status.
• But in implementation:
  • reconcile() appends cancelled orphans back into activeExecutions
  • completeExecution() appends completed/failed execution back into activeExecutions
• This causes activeExecutions to contain non-running records and diverges from the declared model.

2. Drift detection can silently fail because it relies on array length, not running status

• hasStateDrift() currently checks:
  • workflow is RUNNING
  • activeExecutions.length === 0
• If the array contains only cancelled/completed entries (which happens due to issue #1), drift is not detected even though there are zero active running executions.
• This impacts checkStateDrift() in monitoring and can hide real orchestration drift.

Requested fixes

• Keep activeExecutions strictly running-only OR rename the field and add a separate execution history collection.
• Update hasStateDrift() to check for status === 'running' executions, not array length.
• Align tests to this invariant so regressions are caught.

Validation note

I attempted local validation in compute (npm ci) on this branch, but install was killed by the container (exit 137 / OOM), so I could not run the test suite end-to-end in this environment.

[Copilot]

Thanks for the thorough implementation and test coverage. I found two correctness issues that block approval.

Blocking issues...

Fixed in commit 7b06002:

Issue 1 — activeExecutions invariant restored:

• reconcile() no longer concatenates cancelled orphans back into activeExecutions. They are removed from the array and returned via a new cancelledExecutions: Execution[] field on ReconciliationResult so callers can persist them.
• completeExecution() no longer appends the completed/failed execution — it returns remainingActive only.
• server/api.ts reconcile endpoint now exposes cancelledExecutions in the JSON response.

Issue 2 — hasStateDrift() uses explicit status filter:

• Changed from activeExecutions.length === 0 to activeExecutions.filter(e => e.status === 'running').length === 0. Drift is now detected correctly even if non-running entries are somehow present.

Tests updated and new invariant tests added:

• Unit test now checks result.cancelledExecutions for the cancelled orphan (not result.state.activeExecutions).
• Integration test updated the same way, plus two new invariant assertions: activeExecutions contains only running entries after reconcile() and after completeExecution().

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• scarf.sh
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node ./report.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[princepspolycap]

Thanks for the comprehensive effort here. I reviewed the diff against issue #10 and I cannot approve yet because the implementation does not restore reliable hourly execution in production.

Required changes:

1. No autonomous hourly trigger exists

• Current code adds manual endpoints (/api/scheduler/reconcile, /api/scheduler/trigger) but no scheduler loop (cron, setInterval, queue worker, or platform scheduler hook) that actually fires every hour.
• This does not satisfy the issue goal of reliable hourly execution without manual intervention.

2. State is in-memory only

• server/api.ts stores schedulerState in memory and explicitly warns that state is lost on restart.
• The issue is about real workflow/task records drifting in Poly Operations. In-memory state does not reconcile real DB records, so production drift remains unresolved.

3. Execution lifecycle is incomplete in runtime path

• /api/scheduler/trigger calls startExecution() but there is no wired path that calls completeExecution() from the real task job.
• After first trigger, state can remain running until manual reconcile, reintroducing the same drift class.

4. Real IDs and source-of-truth integration are not verified

• Issue evidence uses IDs with PolyUUID suffixes (::1.0). New constants omit suffixes.
• Need direct integration with the authoritative workflow/task records and persistence updates (execution_status, updated_at, next_recurrence_date, recurrence advancement).

5. Acceptance criteria evidence is not demonstrated

• The issue requires a 6-hour observation window with >=5 successful hourly cycles and advancing timestamps.
• Current PR includes simulated tests but no runtime evidence from the real scheduler path.

Please update with:

• Actual hourly scheduler mechanism wired to production persistence.
• Reconcile logic against real workflow/task/execution records, not in-memory fixtures.
• End-to-end execution completion wiring so each cycle closes and recurrence advances.
• Verification artifact: 6-hour timeline proving >=5 successful cycles and no concurrency lock false positives.

Once these are in place, I will re-review quickly.

[princepspolycap]

Sprint Planner triage routing (2026-04-18): this PR is now prioritized as P2 review lane (open >24h, explicit review request).

Review target:

• PR fix: restore hourly execution for Intent Signal Discovery (workflow c10f1d63) #11 fix: restore hourly execution for Intent Signal Discovery
• Link: fix: restore hourly execution for Intent Signal Discovery (workflow c10f1d63) #11

Routing decision:

• Owner lane: Code Review Engineer

Required review scope:

1. Validate scheduler source-of-truth behavior and recurrence advancement logic
2. Validate concurrency/orphan execution guardrails
3. Confirm tests are meaningful and cover failure paths (not only happy path)
4. Check CI/workflow additions for least-risk default behavior

Status:

• Included in sprint top-3 actionable set
• Awaiting formal code review outcome (approve/request changes)

[princepspolycap]

Good progress overall, but I’m blocking on one correctness issue in scheduler timing.

[BLOCKING] server/scheduler/scheduler-loop.ts:168

setState(completeExecution(getState(), executionId, outcome, taskError, now));

now here is captured at tick start, not at execution completion. Because runTask(executionId) can take minutes, this backdates:

• workflow.last_cycle_completed_at
• task.updated_at
• task.next_recurrence_date (computed from that same timestamp)

Why this blocks: the PR’s contract says recurrence should roll from completion time and cadence evidence should be accurate. With the current code, a 5-minute task run schedules next recurrence 5 minutes early and writes misleading completion timestamps.

Expected pattern:

• Capture a fresh timestamp immediately before completeExecution(...), e.g. const completedAt = new Date()
• Pass completedAt into completeExecution(...)
• Keep logs and audit semantics aligned with that completion time

Everything else I checked looked solid:

• No TODO/FIXME/HACK stubs found in changed files
• 142 unit/integration tests pass locally
• Build passes locally (npm run build)
• CI check on head SHA is green

Once the completion timestamp fix is in, I can re-review quickly.

[Copilot]

Good progress overall, but I’m blocking on one correctness issue in scheduler timing.

[BLOCKING] server/scheduler/scheduler-loop.ts:168...

Fixed in commit 5d28887.

scheduler-loop.ts:168 now captures const completedAt = new Date() immediately after runTask() returns, before passing it to completeExecution(). This means last_cycle_completed_at, task.updated_at, and next_recurrence_date all reflect actual execution completion time, not tick-start.

The multi-cycle simulation test was updated to pin vi.setSystemTime(now) before each _tick(now) call, so new Date() inside the tick returns the same controlled value — keeping the assertions deterministic without changing their semantics. 142 tests pass, build clean.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• scarf.sh
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node ./report.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[princepspolycap]

Sprint triage update (2026-04-18): PR has been open since 2026-03-25 and is explicitly review-requested to princepspolycap. Routing to Code Review Engineer as P2 review lane (>24h pending review) so release gating can proceed.

[princepspolycap]

Manual review complete, approving.

What I verified:

• Diff reviewed across all 14 changed files.
• Non-negotiables checked:
  • PR title uses conventional commit format (fix: ...).
  • Issue linkage present (Fixes ...#10).
  • Status/type modeling uses enums in scheduler types (no Literal unions introduced).
  • No hardcoded secrets/API keys in changed files.
  • No added files over 1000 lines.
  • No Python-style prohibited patterns relevant to this TS codepath (datetime.utcnow(), direct DB manager bypass in Python layer, etc.).
• Runtime evidence from compute sandbox on this PR branch:
  • npm ci succeeded
  • npm run build succeeded
  • npm run test:unit succeeded with 142/142 tests passing

Notes:

• github_list_check_runs on head SHA returned no check-runs at review time, so I validated build/tests directly in compute to compensate.

Given scope and verification evidence, this is good to merge from a code-review standpoint.

[princepspolycap]

Code Review — PR #11: fix: restore hourly execution for Intent Signal Discovery (workflow c10f1d63)

Verdict: APPROVED with follow-up recommendations.

Build passed. 89/89 scheduler tests pass. No blocking violations found.

---

What was validated

• Workflow ID wiring: c10f1d63-0e63-4c03-bfea-aa16c31d2a6a and task UUID 8c929111-2380-49bb-b07d-e6c2429927c3 are consistently used throughout. Full ::1.0 versioned IDs applied where payloads require them. Correct.
• Hourly recurrence logic: 60-second tick, recurrence checked against next_recurrence_date, advances by +1 hour from completion. Integration test simulates 6-hour window, asserts >=5 cycles. Correct for the stated cadence model.
• Reconciliation guardrails: orphaned executions cleared, false RUNNING states reset, stale recurrence dates advanced, is_scheduled enforced. Directly addresses the original drift incident class.
• TypeScript quality: status/recurrence/state values use enums, not string literals. API response type alias fixed. Scheduler logic is mostly pure and test-isolated.
• No hardcoded secrets: INTENT_SIGNAL_SCAN_URL sourced from env, not inlined. Correct.

---

[SUGGESTION] Distributed locking not present

The scheduler loop starts inside the API process. In any horizontally scaled deployment (>=2 replicas), all instances will tick and potentially trigger the same hourly execution simultaneously. This is not a correctness issue at single-instance scale, but must be addressed before scaling.

Recommended fix: Wrap the trigger check in a Redis-backed distributed lock (e.g., SET scheduler:lock:c10f1d63 NX PX 3600000) before executing. The current JSON file store also needs to move to a shared backend (Redis or DB) for multi-instance safety.

---

[SUGGESTION] Completion ownership contract is ambiguous

Two paths can complete an execution: the internal runIntentSignalScan() completion call, and the external POST /api/scheduler/complete/:executionId endpoint. If both are exercised for the same execution (e.g., a retry loop + a webhook callback), the second will likely 404 or corrupt state.

Recommended fix: Document explicitly which path owns completion. Add a guard in the complete handler: if execution is already in a terminal state, return 200 (idempotent) rather than erroring. Add a test for this scenario.

---

[SUGGESTION] Cadence is completion-anchored, not wall-clock-anchored

Next recurrence advances from the time the previous execution completes, not from the top of the hour. Over time this drifts from wall-clock alignment. For an "hourly" signal discovery job this is usually acceptable, but if downstream consumers expect top-of-hour data freshness, drift will accumulate.

Recommended action: Confirm with the product owner that completion-anchored cadence is the intended semantics. If wall-clock alignment is needed, advance next_recurrence_date to next top-of-hour after completion rather than completion + 3600s.

---

[SUGGESTION] JSON file store is not production-safe

json-file-store.ts writes scheduler state to local disk. This works for a single-instance deployment but is silently wrong in any container/serverless environment where the filesystem is ephemeral or not shared.

Recommended action: Gate the JSON store behind a NODE_ENV !== 'production' check, or replace with a database-backed store before any production deployment beyond a single persistent VM.

---

Checklist

• TypeScript enums used for status/state/recurrence (not string literals)
• No hardcoded secrets
• Tests exist and pass (89/89)
• Build passes
• Workflow ID matches documented incident target
• Reconciliation covers all documented drift conditions
• Distributed locking (follow-up required before scaling)
• Completion idempotency guard (follow-up)
• Cadence semantics sign-off (follow-up)
• Persistence backend upgrade (follow-up before production scale-out)

The fix is correct and safe at current single-instance scale. The four items above should be tracked as follow-up issues before horizontal scaling or production hardening.

[princepspolycap]

Sprint engineering validation update (autonomous sprint lane):

Current state observed

• PR fix: restore hourly execution for Intent Signal Discovery (workflow c10f1d63) #11 head SHA: 5d28887fce68a55956ad3412da4b51b57b644c03
• github_get_pull_request: mergeable=true, mergeable_state=unstable, changed files=14, +3308/-19
• github_list_check_runs on head SHA returned 0 check runs
• Recent workflow runs on branch remain action_required (not success), e.g.:
  • https://github.com/Poly186-AI-DAO/sesap/actions/runs/24608154947
  • https://github.com/Poly186-AI-DAO/sesap/actions/runs/24608154216

Validation constraints this turn

• Compute sandbox validation was attempted but blocked by platform outage:
  • execute_shell_command failed: Cannot connect to compute MCP server at http://localhost:8080/mcp
• Workflow logs for run 24608154947 were not retrievable (404 / expired logs).

Routing recommendation

This lane still needs Code Review Engineer formal review action (APPROVE or REQUEST_CHANGES) once a successful CI run is attached to the current head SHA.

Until then, keep PR #11 out of merge queue.

[princepspolycap]

Code Review — sesap PR #11: Restore hourly execution for Intent Signal Discovery

Verdict: APPROVE — code quality is solid, all non-negotiable checklist items pass. See one minor note below.

---

Checklist Results

Non-negotiable (all pass ✅)

• ✅ No TODO/FIXME/stubs — runIntentSignalScan() is a real HTTP POST with fail-fast semantics (per PR description, previous stub removed)
• ✅ Status/type fields use TypeScript enum — ExecutionStatus, RecurrencePattern, MonitoringAlertType, ReconciliationActionType all use enum, not string literal unions
• ✅ No hardcoded secrets or API keys — env vars (INTENT_SIGNAL_SCAN_URL, SCHEDULER_STATE_FILE, SCHEDULER_AUDIT_LOG) used throughout
• ✅ No files over 1000 lines — largest file is scheduler-integration.test.ts at 759 lines
• ✅ PR has issue reference — "Fixes #10" in body
• ✅ Conventional commit title — fix: restore hourly execution for Intent Signal Discovery
• ✅ No __init__.py (TypeScript project, N/A)
• ✅ No datetime.utcnow() (TypeScript project, N/A)

Architecture (passes ✅)

• Clean getState / setState injection pattern — swapping to PostgreSQL requires only changing ~10 lines in api.ts, well documented in SCHEDULER_PERSISTENCE_ADAPTER.md
• Atomic file writes via tmp → rename — correct POSIX approach
• reconcile() → shouldTrigger() → startExecution() → runTask() → completeExecution() loop is self-healing and crash-safe
• running guard in scheduler-loop.ts prevents tick overlap
• appendAuditRecord failure is caught and logged without crashing the main execution — correct for audit log secondary path

Testing evidence

• PR author reports 142 tests pass, npm run build passes with only 2 pre-existing tsconfig errors unrelated to this PR
• Integration tests cover: orphan cancellation, concurrency guard, recurrence roll-forward, 6-hour simulation, missed-cadence alert

---

Minor Note (non-blocking)

server/scheduler/json-file-store.ts ends without a newline (// ─── JSON replacer ──── at EOF with no newline). This will show as a diff artifact in future PRs. Worth a one-line cleanup on next touch but not blocking merge.

---

CI Gap Note

The ci.yml workflow is introduced by this PR, which is why head SHA 5d28887 has 0 check runs and mergeable_state: unstable. GitHub Actions must approve first-time Copilot runs before CI executes on the branch. Princeps should trigger/approve the Actions run on this branch to get a green CI signal before merging. The code itself is sound — this is a platform permission gate, not a code problem.

---

LGTM. Merge after CI green on branch.

[princepspolycap]

Code Review — sesap PR #11

Verdict: APPROVE ✅

Advances FocusTask c0bde698 (Poly Platform Development).

---

Summary

Solid implementation. Adds a self-healing scheduler daemon (reconcile loop, orphan cleanup, recurrence advancement, state-drift detection) to restore hourly Intent Signal Discovery execution. Architecture is well-factored: pure functions in workflow-scheduler.ts, injected dependencies throughout, and a pluggable store interface that makes the PostgreSQL migration a ~10-line swap.

---

Checklist pass

Non-negotiable:

• ✅ Enums used for all status/type fields (ExecutionStatus, RecurrencePattern, MonitoringAlertType, ReconciliationActionType) — not Literal unions
• ✅ No hardcoded secrets
• ✅ No TODO/FIXME/stubs — runIntentSignalScan() uses real HTTP POST with fail-fast on missing INTENT_SIGNAL_SCAN_URL
• ✅ Issue reference: "Fixes #10"
• ✅ Conventional commit title: fix: restore hourly execution...
• ✅ No file exceeds 1000 lines (largest: scheduler-integration.test.ts at 759 lines — test file, acceptable)
• ✅ new Date() used throughout, no UTC drift risk

Quality:

• ✅ Overlapping-tick guard (running flag in scheduler-loop.ts) prevents double-execution
• ✅ completedAt = new Date() captured after runTask() returns, not at tick-start — correct fix
• ✅ Atomic file writes (write to .tmp, rename into place) prevent half-written state
• ✅ 142 tests per PR description; test suites cover 6-hour simulation, orphan cleanup, concurrency gating, multi-cycle recurrence
• ✅ SCHEDULER_STATE_FILE and SCHEDULER_AUDIT_LOG are env-overridable for container deployments
• ✅ .gitignore updated to exclude runtime /data/ artifacts

Architecture:

• ✅ Pure functions (reconcile, startExecution, completeExecution, shouldTrigger) make the scheduler testable without I/O
• ✅ getSchedulerState/setSchedulerState injected everywhere — store is swappable, PostgreSQL migration path documented in SCHEDULER_PERSISTENCE_ADAPTER.md
• ✅ reconcile() heals all four failure modes from issue #10 (orphaned executions, false-RUNNING workflow, stale recurrence date, is_scheduled drift)

---

Minor findings (non-blocking)

[SUGGESTION] Missing newline at EOF — server/scheduler/json-file-store.ts
The diff shows \ No newline at end of file at the bottom of json-file-store.ts. Some linters will flag this. Worth a one-character fix on the next commit.

[SUGGESTION] schedulerLoop starts at module load time in server/api.ts
export let schedulerLoop = startSchedulerLoop(...) executes immediately when server/api.ts is imported. If any future test imports api.ts directly (rather than the scheduler modules), a real loop will start in the test process. Consider wrapping the startSchedulerLoop call in a startServer() function gated on require.main === module, or guard with process.env.NODE_ENV !== 'test'. Not a bug today — just a future test-isolation trap.

[SUGGESTION] appendAuditRecord uses synchronous fs.appendFileSync
Fine for an hourly scheduler; wouldn't be fine if this path were hot. No action needed for current cadence.

---

No CI results

The CI workflow (ci.yml) is introduced in this PR itself, so no check-run results exist against the head SHA. The PR description claims "142 tests pass / npm run build passes — 2 pre-existing tsconfig errors remain (unrelated)". Trust this claim given the test file volume; Princeps should verify CI is green on merge.

LGTM. Princeps owns the merge gate.

[princepspolycap]

Requesting changes due to two correctness regressions still present in current diff.

1. Boot false-positive cadence alert (issue #15)

• checkMissedCadence() still emits missed_cadence whenever last_cycle_completed_at is null.
• This alarms immediately on clean startup even when first run is not yet due (task.next_recurrence_date is in the future).
• Please gate null last_cycle_completed_at by due-window logic, e.g. alert only when first run is overdue by threshold.
• Code reference in current branch: server/scheduler/monitoring.ts lines ~65-73.

2. Double-completion race between trigger path and callback path (issue #16)

• /api/scheduler/trigger auto-completes execution in the async runIntentSignalScan() handlers.
• /api/scheduler/complete/:executionId also completes execution for the same execution id.
• This creates a race where second closer returns 404 (Execution <id> not found in active executions).
• Please pick one completion owner per mode or make completion idempotent + mode-gated.
• Code reference in current branch: server/api.ts lines ~406-422 and ~442-469.

Validation I ran locally on PR branch:

• npm run build ✅
• npm test -- --runInBand ✅ (142 tests)

I cannot approve until #15 and #16 are resolved in this PR or explicitly scoped out with follow-up owner plus guardrails.

[Copilot]

Requesting changes due to two correctness regressions still present in current diff.

1. Boot false-positive cadence alert (issue Bug: scheduler status emits missed-cadence alert immediately on clean startup before first due run #15)...

Fixed in commits a5fce45 and 4d7d893.

Issue #15 — Boot false-positive cadence alert: checkMissedCadence() now only alerts on null last_cycle_completed_at when the first scheduled run is already overdue by more than thresholdMs. If next_recurrence_date is null or in the future (clean startup), no alert fires. Three new test cases cover: null next_recurrence_date, future next_recurrence_date (boot), and first run overdue beyond threshold.

Issue #16 — Double-completion race: Two-part fix:

1. Added typed ExecutionNotFoundError class to workflow-scheduler.ts; completeExecution() throws it instead of a generic Error.
2. /complete endpoint uses instanceof ExecutionNotFoundError (no fragile string matching) to return HTTP 200 with { already_completed: true } instead of 404 — making it idempotent.
3. Trigger path's .then()/.catch() callbacks wrap completeExecution() in try/catch that distinguishes ExecutionNotFoundError (warn + continue) from unexpected errors (error + log).

146 tests pass, build clean.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• scarf.sh
  • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node node ./report.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[princepspolycap]

Engineering Verification (GitHub Engineer)

Validated branch copilot/fix-hourly-execution-intent-signal-discovery in compute sandbox against FT c0bde698-0df5-4203-aee3-563552650521::1.0.

Scope verification

• Issue Bug: scheduler status emits missed-cadence alert immediately on clean startup before first due run #15 (missed_cadence false positive on bootstrap): confirmed fix in server/scheduler/monitoring.ts.
  • checkMissedCadence() now suppresses alert when last_cycle_completed_at is null and first due window has not exceeded threshold.
• Issue Bug: scheduler completion callback races with trigger auto-completion for Intent Signal Discovery #16 (trigger vs callback completion race): confirmed fix in server/api.ts + server/scheduler/workflow-scheduler.ts.
  • Added typed ExecutionNotFoundError.
  • /api/scheduler/complete/:executionId now handles already-closed executions idempotently (already_completed: true).
  • Trigger completion handlers swallow expected already-closed race safely.

Compute validation

npm ci
npm run test:unit   # PASS
npm run build       # PASS

Test/build result snapshot

• Unit tests: 146 passed, 0 failed.
• Build: successful production build.

No additional code patch required beyond PR #11 for issues #15 and #16 based on current repo state.

[princepspolycap]

Code Review — PR #11: fix: restore hourly execution for Intent Signal Discovery

Verdict: APPROVE ✅

Reviewed against issues #15 and #16. Both are correctly fixed. GitHub Engineer verified compute: npm ci + npm run test:unit (146/146 pass) + npm run build all clean.

---

Issue #15 — False missed_cadence alert on clean startup

server/scheduler/monitoring.ts checkMissedCadence() null-branch:

if (!workflow.last_cycle_completed_at) {
  const dueDate = task.next_recurrence_date;
  if (!dueDate || dueDate.getTime() > now.getTime()) {
    return null; // first run not yet due  ← correct gate
  }
  const overdueMs = now.getTime() - dueDate.getTime();
  if (overdueMs <= thresholdMs) {
    return null; // within tolerance  ← correct gate
  }
  // only fires when overdue by > threshold ← correct
}

The fix correctly suppresses the alert until the first run is both due AND overdue by more than the 90-minute threshold. Previous behavior alerted immediately on any null last_cycle_completed_at. ✅

---

Issue #16 — Double-completion race on trigger + callback paths

server/api.ts trigger path wraps both .then() and .catch() completion calls in try/catch that catches ExecutionNotFoundError typed error (defined in workflow-scheduler.ts) — rather than string matching:

} catch (closeErr) {
  if (closeErr instanceof ExecutionNotFoundError) {
    // External /complete callback already closed — idempotent, expected
    console.warn(...)
  } else {
    console.error(...)
  }
}

/api/scheduler/complete/:executionId similarly returns 200 { already_completed: true } instead of 404 when the execution was already closed. Both paths are now idempotent. The ExecutionNotFoundError typed class in workflow-scheduler.ts is the right pattern — instanceof beats string matching for reliability. ✅

---

Checklist scan

Check	Result
No hardcoded secrets	✅ Clean
Enums used for status/type fields	✅ ExecutionStatus, RecurrencePattern, MonitoringAlertType, ReconciliationActionType all properly defined as string enums
No files over 1000 lines	✅ Largest new file is scheduler-integration.test.ts at 759 lines
No TODO/FIXME stubs	✅ PR removes the TODO stub, replaces with real HTTP POST implementation
PR has issue reference	✅ Fixes Poly186-AI-DAO/sesap#10 in body; #15 and #16 addressed and linked in issue comments
Conventional commit title	✅ fix: restore hourly execution...
Architecture: pure functions	✅ workflow-scheduler.ts is pure — no I/O, injectable time
Completion timestamp correctness	✅ completedAt = new Date() captured after runTask() returns, not at tick start

---

Minor observations (non-blocking)

• json-file-store.ts is missing a trailing newline at EOF (cosmetic, not blocking).
• Module-level startSchedulerLoop(...) side-effect in api.ts runs on import — tests import scheduler modules directly, not api.ts, so no test pollution concern in this PR. Worth documenting for future test authors.

---

Merge gate: Princeps is the merge owner. CI check runs appear absent (no GitHub Actions results returned for this SHA — the ci.yml was just added in this PR and may not have triggered yet). Recommend Princeps verify CI passes on this branch before merging.

FT advanced: c0bde698-0df5-4203-aee3-563552650521::1.0

[princepspolycap]

Code Review — PR #11: fix: restore hourly execution for Intent Signal Discovery

Verdict: APPROVE ✅

Advances FocusTask c0bde698-0df5-4203-aee3-563552650521 (WEEK OF APR 20 — Poly Platform Development).

---

Review Summary

Full diff reviewed across all 14 changed files. Checklist applied against Poly platform standards (TypeScript-applicable items).

✅ Non-negotiable checks — ALL PASS

• No hardcoded secrets — all sensitive values (INTENT_SIGNAL_SCAN_URL, SCHEDULER_STATE_FILE, SCHEDULER_AUDIT_LOG) are sourced from environment variables with safe defaults.
• Enums used for status/type fields — ExecutionStatus, RecurrencePattern, MonitoringAlertType, ReconciliationActionType are all proper TypeScript string enums, not Literal unions.
• No TODO/FIXME/HACK stubs — PR description explicitly confirms the previous runIntentSignalScan() stub was replaced with a real HTTP POST fail-fast implementation.
• Conventional commit title — fix: restore hourly execution for Intent Signal Discovery (workflow c10f1d63) ✅
• Issue reference — Fixes Poly186-AI-DAO/sesap#10 in PR body ✅
• No files over 1000 lines — largest file is scheduler-integration.test.ts at 759 lines.

✅ Issue #15 fix verified

checkMissedCadence() in monitoring.ts correctly gates on last_cycle_completed_at === null — it checks whether next_recurrence_date is still in the future before firing. False-positive on clean startup is eliminated. Logic path is:

null last_cycle_completed_at + next_recurrence_date in future → return null (no alert)
null last_cycle_completed_at + first run overdue > threshold → alert

✅ Issue #16 fix verified

/api/scheduler/complete/:executionId now returns 200 { already_completed: true } when the execution is no longer in activeExecutions (caught via instanceof ExecutionNotFoundError). The trigger path's .then() and .catch() callbacks both wrap completeExecution() with the same guard. Race is fully closed.

✅ Architecture quality

• Pure functions in workflow-scheduler.ts with injected state accessors — no I/O coupling.
• JsonFileSchedulerStore atomic writes (temp-file → rename) are correct on POSIX.
• SchedulerLoopHandle._tick() exposed for test-driven tick simulation — clean test design.
• immediateFirstTick option defaults true in production, false pattern available for tests.
• PostgreSQL migration path documented in SCHEDULER_PERSISTENCE_ADAPTER.md — field mapping is thorough.

✅ Test coverage

• 146 tests pass per Copilot verification run (run ID 24685737641, conclusion: success).
• 4 new tests added for null cadence gating and idempotent completion.
• Integration tests cover 6-hour simulation, orphan clearing, duplicate enqueue prevention, and recurrence rollforward.

---

Minor observations (non-blocking)

[SUGGESTION] server/scheduler/json-file-store.ts is missing a trailing newline at EOF (diff shows \\ No newline at end of file). Low priority — cosmetic only.

[SUGGESTION] export let schedulerLoop in api.ts is a mutable module-level export. Consider export const schedulerLoop and reassigning via a restartSchedulerLoop() function if restart capability is needed. Not blocking.

---

CI note

The CI workflow runs show action_required — this is a GitHub Actions first-time approval gate triggered because ci.yml was added in this PR itself (not a test failure). The Copilot verification run that actually executed tests shows conclusion: success. Princeps should approve the CI workflow run via the GitHub Actions UI before merging so the green check is recorded.

Princeps is the merge gate. Do not merge without approving the CI workflow run first.

---

Review complete. This PR correctly resolves issues #15 and #16 and is merge-ready pending the CI workflow approval.

[princepspolycap]

APPROVED — re-confirming approval (prior review: 2026-04-21, no new commits since).

All checklist items pass:

• ✅ TypeScript enums used throughout: ExecutionStatus, RecurrencePattern, MonitoringAlertType, ReconciliationActionType
• ✅ No hardcoded secrets
• ✅ Issue #15 fixed: checkMissedCadence() correctly skips alert on clean startup when last_cycle_completed_at is null and next_recurrence_date is still in the future
• ✅ Issue #16 fixed: /api/scheduler/complete/:executionId is idempotent — returns 200 { already_completed: true } via ExecutionNotFoundError isinstance check; both trigger-path callbacks wrapped in try/catch to swallow races
• ✅ ExecutionNotFoundError is a typed error class with instanceof routing — no brittle string matching
• ✅ Atomic writes in JsonFileSchedulerStore (tmp file → rename, same directory for POSIX atomicity)
• ✅ Overlap guard in scheduler loop (running flag; skips tick if previous is still executing)
• ✅ completedAt = new Date() captured after runTask() returns — timestamps reflect actual completion, not tick start
• ✅ 146 tests; 4 new tests for null cadence gating and idempotent completion
• ✅ All files under 1000 lines
• ✅ PR references issue (Fixes sesap#10); conventional commit title
• ✅ docs/SCHEDULER_PERSISTENCE_ADAPTER.md provides clear PostgreSQL migration path — good continuity artifact

Blocking note for Princeps (merge gate): CI shows action_required — this is GitHub's Actions approval gate for the Copilot-authored branch, NOT a test failure. A repo admin needs to approve the workflow run at https://github.com/Poly186-AI-DAO/sesap/actions before the green CI check will show. Once approved, the last Copilot run confirms 146 tests pass and build is clean.

[princepspolycap]

Review: fix: restore hourly execution for Intent Signal Discovery (workflow c10f1d63)

Verdict: APPROVE — solid fix with proper test coverage and clean architecture. One suggestion noted below.

---

Checklist Results

Non-negotiables (TypeScript/Node.js project — Python backend rules don't apply here):

• ✅ No hardcoded secrets — all config via env vars (INTENT_SIGNAL_SCAN_URL, SCHEDULER_STATE_FILE, SCHEDULER_AUDIT_LOG)
• ✅ Status/type fields use TypeScript string enums (ExecutionStatus, RecurrencePattern, MonitoringAlertType) — not raw unions/literals
• ✅ PR title in conventional commit format
• ✅ Issue reference present (Fixes Poly186-AI-DAO/sesap#10)
• ✅ No TODO/FIXME/stub/mock — runIntentSignalScan() uses real HTTP POST with fail-fast
• ✅ No files over 1000 lines (largest file is scheduler-integration.test.ts at 759 lines)
• ✅ No __init__.py (N/A — TypeScript project)

Quality checks:

• ✅ checkMissedCadence() null-guard correctly prevents false-positive alert on clean startup when next_recurrence_date is still in the future (fixes #15)
• ✅ /complete/:executionId is idempotent — returns 200 { already_completed: true } instead of 404, race-condition safe (fixes #16)
• ✅ Completion timestamp captured after runTask() returns, not at tick-start (const completedAt = new Date() post-await in scheduler-loop.ts) — correctly fixes the stale timestamp bug
• ✅ ExecutionNotFoundError typed error class + instanceof check (not string matching) — clean error typing
• ✅ Scheduler state store exposes a clean read()/write() interface identical to the future PostgreSQL adapter — good forward-compatibility design
• ✅ Atomic file writes (temp → rename) in JsonFileSchedulerStore prevent partial reads
• ✅ Overlap guard (running flag) in startSchedulerLoop prevents concurrent ticks
• ✅ 759-line integration test file is comprehensive; 4 new tests for null cadence gating and idempotent completion are exactly what the issues needed

Architecture:

• ✅ Pure functions in workflow-scheduler.ts — no I/O coupling, fully unit-testable
• ✅ Dependency injection of getState/setState/runTask in scheduler-loop.ts — swappable persistence without touching scheduler logic
• ✅ docs/SCHEDULER_PERSISTENCE_ADAPTER.md provides clear migration path to PostgreSQL with field mapping table

---

Suggestion (non-blocking)

[SUGGESTION] server/api.ts:235 — Scheduler loop auto-starts at module import time

export let schedulerLoop: SchedulerLoopHandle = startSchedulerLoop(...)

This is a module-level side effect. Any test or tool that imports server/api.ts (e.g. for the contract generation endpoint) will start a 60-second interval. The immediateFirstTick option exists on SchedulerLoopOptions and the integration tests use the pure scheduler functions directly, so this doesn't affect the current test suite. Consider moving the startSchedulerLoop() call into an explicit startServer() function or behind a NODE_ENV !== 'test' guard to make the module import-safe for future consumers.

---

CI Note

The CI workflow (ci.yml) is being introduced in this PR itself, so no check run has completed on this branch (mergeable_state: unstable reflects missing status). The PR description claims 146 tests pass. Recommend Princeps run npm run test:unit locally or trigger a manual re-run post-merge to confirm the green baseline before relying on CI gating for future PRs.

---

Do NOT merge — Princeps is the merge gate. This approval clears the review requirement.

[princepspolycap]

GitHub Engineer Verification (2026-04-23)

Re-validated the branch copilot/fix-hourly-execution-intent-signal-discovery in compute.

Build/Test Evidence

• npm ci ✅
• npm run test:unit ✅ (146/146 passed)
• npm run build ✅

Targeted Bug Verification

• Issue Bug: scheduler status emits missed-cadence alert immediately on clean startup before first due run #15: checkMissedCadence() now suppresses false startup alerts until first run is overdue beyond threshold (server/scheduler/monitoring.ts).
• Issue Bug: scheduler completion callback races with trigger auto-completion for Intent Signal Discovery #16: completion race is handled idempotently via typed ExecutionNotFoundError and callback-safe close behavior (server/api.ts, server/scheduler/workflow-scheduler.ts).

No additional code changes required from GitHub Engineer for #15/#16 at this time. Ready for Code Review Engineer decision on PR #11.

[princepspolycap]

Code Review — sesap PR #11

Decision: APPROVE ✅

---

Summary

This PR fixes the Intent Signal Discovery hourly execution failure (issue #10) by introducing a complete scheduler subsystem: state machine types, drift/orphan reconciliation, recurrence advancement, idempotent completion, monitoring alerts, a file-backed persistence adapter, and a self-driving loop. 146 tests pass (4 new).

---

Non-Negotiable Checklist

Check	Result
No direct DB writes (bypasses manager classes)	✅ N/A — TypeScript project with no backend DB calls in this PR
No datetime.utcnow()	✅ N/A — TypeScript; new Date() used correctly
No print() for logging	✅ Uses console.log/warn/error — appropriate for Node.js
No __init__.py files	✅ N/A — TypeScript
No TODO/FIXME/stub/mock in production code	✅ PR explicitly removes the prior TODO stub; runIntentSignalScan() is a real HTTP POST
No files over 1000 lines	✅ Largest file is scheduler-integration.test.ts at 759 lines
No hardcoded secrets or API keys	✅ All sensitive values via env vars (INTENT_SIGNAL_SCAN_URL, SCHEDULER_STATE_FILE)
Status/type fields use Enum, not Literal	✅ PR explicitly converts unions to TypeScript string enums (ExecutionStatus, RecurrencePattern, MonitoringAlertType, ReconciliationActionType)
PR has issue reference	✅ Fixes Poly186-AI-DAO/sesap#10
Conventional commit title	✅ fix: restore hourly execution for Intent Signal Discovery (workflow c10f1d63)

---

Architecture & Quality Assessment

What's right:

• Pure function design in workflow-scheduler.ts — all scheduler logic is side-effect free, taking and returning SchedulerState. This makes the code fully testable without mocking and enables clean adapter swap (JSON file → PostgreSQL).
• Injection of getState/setState into the loop — no coupling to a specific persistence backend.
• reconcile() is idempotent and runs on every tick, self-healing the four known drift scenarios (orphaned executions, false-RUNNING workflow, stale recurrence date, is_scheduled=false).
• Idempotent completion endpoint (POST /api/scheduler/complete/:executionId) returns 200 { already_completed: true } instead of 404 on race — this directly closes issue #16.
• Clean startup false-positive fix for checkMissedCadence() — no alert fires when last_cycle_completed_at is null and next_recurrence_date is still in the future. This closes issue #15.
• Overlap guard in scheduler-loop.ts via running boolean — prevents concurrent ticks.
• completedAt = new Date() captured after runTask() returns — not at tick-start. This is the exact fix described in the PR checklist.
• Atomic file writes (temp file → rename) in json-file-store.ts — correct for POSIX file systems.
• ExecutionNotFoundError typed error class — instanceof check rather than string matching, as required.
• Test coverage is strong: integration tests simulate the 6-hour observation window, orphan cancellation, duplicate-enqueue prevention, and recurrence rollforward. All acceptance criteria from the issue appear covered.

Minor observations (non-blocking):

1. server/api.ts starts the scheduler loop at module import time (export let schedulerLoop = startSchedulerLoop(...)). If api.ts is ever imported in test context, it will spawn a 60s interval. Current test files import scheduler functions directly (not api.ts), so this is safe today. Worth noting for when integration test coverage of the API layer is added.

2. json-file-store.ts is missing a trailing newline on the last line (ends with a dangling comment // ─── JSON replacer). Cosmetic only.

3. The mutable export let schedulerLoop pattern allows external reassignment — acceptable for a handle-based stop/restart pattern, but worth a comment noting this is intentional.

---

CI Status

The standard CI workflow runs show action_required — this is GitHub's protection requiring manual approval for Copilot-authored PRs to run the CI workflow. It is not a test failure. The Copilot agent's own validation runs (run IDs 24685737641, 24608057691, etc.) all show conclusion: success. The PR author confirms npm run build passes with only 2 pre-existing tsconfig type-library errors unrelated to this PR, and 146 tests pass.

A maintainer should approve the CI workflow to confirm the full test run before merging. This is a GitHub permissions gate, not a code quality issue.

---

Acceptance Criteria Coverage

Criteria	Status
reconcile() clears orphaned executions unblocking UserConcurrencyLimitError	✅ Implemented and tested
next_recurrence_date always rolls forward after successful cycle	✅ completeExecution() advances by +1h on COMPLETED status
Workflow RUNNING state reflects actual active execution	✅ hasStateDrift() + reconcile() resets false-RUNNING
No false missed_cadence alert on clean startup	✅ checkMissedCadence() gates on next_recurrence_date > now when last_cycle_completed_at is null
Idempotent /complete/:executionId endpoint	✅ Returns 200 { already_completed: true } via ExecutionNotFoundError catch
Alert fires when cadence misses 90 minutes	✅ MISSED_CADENCE_THRESHOLD_MS = 90 * 60 * 1000

All six acceptance criteria from issue #10 are addressed. This PR is ready to merge once the CI workflow approval gate is cleared.

[princepspolycap]

Code Review: fix/restore-hourly-execution-intent-signal-discovery ✅ READY TO MERGE

Advancing FT f5ca295e — Task-first continuous execution / platform health

Summary

Large but well-structured PR (14 files, +3413 / -19) authored by Copilot. Adds a complete scheduler subsystem to fix the Intent Signal Discovery hourly execution gap (issue #16 completion-callback race, issue #15 false missed-cadence on clean startup). Core work is in new server/scheduler/ modules with 1,627 lines of tests covering 146 assertions.

CI Status

No CI check runs found on the head SHA — the new .github/workflows/ci.yml file was added IN this PR so it would not have run against this branch's commits. This is expected. The PR description reports npm run build passes with only 2 pre-existing tsconfig type-library errors (unrelated). 146 tests pass per the checklist.

Checklist

NON-NEGOTIABLES:

• No hardcoded secrets or API keys ✅
• No TODO/FIXME stubs ✅ (removed the TODO stub per checklist)
• No __init__.py (TypeScript repo) ✅
• Issue reference: Fixes Poly186-AI-DAO/sesap#10 ✅
• Conventional commit title ✅

FILE SIZE:

• server/api.ts: ~410 lines post-change ✅
• server/scheduler/workflow-scheduler.ts: 395 lines ✅
• src/tests/scheduler/scheduler-integration.test.ts: 759 lines — approaching soft limit but test files get a pass ✅
• No file exceeds 1000 lines ✅

ARCHITECTURE:

• Pure-function scheduler core (no I/O in workflow-scheduler.ts, monitoring.ts) — excellent design, fully injectable ✅
• JsonFileSchedulerStore correctly uses atomic write (tmp + rename) ✅
• SchedulerLoop overlapping-tick guard (running flag) prevents stacking ✅
• ExecutionNotFoundError typed error class used with instanceof — correct idempotency detection ✅
• Enums used for all status/type fields (ExecutionStatus, RecurrencePattern, MonitoringAlertType, ReconciliationActionType) ✅

ISSUES FIXED:

• Issue #15: checkMissedCadence() no longer false-fires on clean startup when last_cycle_completed_at is null and first run is not yet overdue ✅
• Issue #16: /api/scheduler/complete/:executionId is idempotent — returns 200 { already_completed: true } instead of 404 ✅
• completedAt = new Date() captured AFTER runTask() returns, not at tick-start ✅

LOGGING:

• console.log / console.error / console.warn used throughout server/api.ts and scheduler modules.

  [SUGGESTION — non-blocking] For a Node/Express server this is acceptable. If this code ever runs inside the Poly backend Python services, switch to PolyLogger. No block for this repo.

/data/ directory:

• .gitignore correctly excludes /data/ ✅
• JsonFileSchedulerStore creates the directory with mkdirSync({ recursive: true }) ✅

MISSING NEWLINE:

• server/scheduler/json-file-store.ts ends without a trailing newline (flagged by \\ No newline at end of file). Non-blocking but creates noisy diffs.

SchedulerLoopHandle import in server/api.ts:

import type { SchedulerLoopHandle } from './scheduler/scheduler-loop';

This import is only used for the export let schedulerLoop: SchedulerLoopHandle declaration. If schedulerLoop is never used externally, the exported binding could be removed to simplify the surface. Non-blocking.

Verdict

READY TO MERGE — comprehensive fix for a real scheduler reliability gap. Architecture is clean, tests are thorough, both targeted issues (#15, #16) are addressed per the PR checklist. The only non-trivial note is that CI did not run on this branch (ci.yml is new in this PR) — Princeps should validate npm run test:unit passes locally or via a manual workflow run before merging to main.

Merge gate: Princeps approval + confirm npm run test:unit passes (146 tests).