Shadow task fixes, asset link ids, backup verdict

plugins/geenforce/api/routes.py

@@ -955,9 +955,12 @@ def _asset_facts(hostnames):
             'mapx': asset.mapx,
             'mapy': asset.mapy,
             'machinenumber': None, 'machineassetid': None,
+            'machinepluginid': None,
             'toolassetnumber': None, 'toolassetid': None,
+            'toolpluginid': None,
             'displayrole': None,
             'backupkind': None, 'backuplastseen': None,
+            'backuplastseenat': None,
         }
 
     _attach_controlled_assets(facts, assetidtohost)
@@ -993,11 +996,24 @@ def _attach_controlled_assets(facts, assetidtohost):
                     AssetRelationship.sourceassetid.in_(assetidtohost))
             .all())
 
-    toolassetids = set()
+    # assetid -> the PLUGIN extension id the detail routes key on. /machines/:id
+    # and /measuringtools/:id take machineid / measuringtoolid, NOT the core
+    # assetid; linking with the assetid lands on whichever record happens to
+    # share that number, which looks plausible and is wrong. Frontend
+    # getAssetRoute resolves the same way.
+    toolids = {}
     try:
         from plugins.measuringtools.models import MeasuringTool
-        toolassetids = {row.assetid for row in
-                        MeasuringTool.query.with_entities(MeasuringTool.assetid)}
+        toolids = {row.assetid: row.measuringtoolid for row in
+                   MeasuringTool.query.with_entities(
+                       MeasuringTool.assetid, MeasuringTool.measuringtoolid)}
+    except ImportError:
+        pass
+    machineids = {}
+    try:
+        from plugins.machines.models import Machine
+        machineids = {row.assetid: row.machineid for row in
+                      Machine.query.with_entities(Machine.assetid, Machine.machineid)}
     except ImportError:
         pass
 
@@ -1005,12 +1021,17 @@ def _attach_controlled_assets(facts, assetidtohost):
         entry = facts.get(assetidtohost.get(sourceassetid))
         if not entry:
             continue
-        if asset.assetid in toolassetids:
+        if asset.assetid in toolids:
             entry['toolassetnumber'] = asset.assetnumber
             entry['toolassetid'] = asset.assetid
+            entry['toolpluginid'] = toolids.get(asset.assetid)
         else:
             entry['machinenumber'] = asset.assetnumber
             entry['machineassetid'] = asset.assetid
+            # None when the machines plugin is not installed or the asset has no
+            # extension row: the frontend then renders the number without a link
+            # rather than a link that goes somewhere wrong.
+            entry['machinepluginid'] = machineids.get(asset.assetid)
 
 
 def _attach_display_roles(facts, hostnames):
@@ -1059,6 +1080,51 @@ def _attach_backup_state(facts, hostnames):
             entry['backuplastseen'] = (
                 revision.lastseenat.isoformat() + 'Z'
                 if revision.lastseenat else None)
+            # Raw value kept beside the wire string so the verdict compares
+            # datetimes rather than re-parsing its own output.
+            entry['backuplastseenat'] = revision.lastseenat
+
+
+def _backup_ok(lastseenat, cutoff):
+    """Tri-state verdict for one PC's backup: True good, False stale, None unknown.
+
+    A BOOLEAN rather than a date, because the question is "is this backed up?"
+    and every backup kind answers it the same way. Adding a new kind needs no
+    change here - it reports a revision like the others and inherits the badge.
+
+    None means there is nothing to judge: no backup revision for this host at
+    all, or the staleness check is disabled. The caller shows no badge rather
+    than a green one, since "never seen" must not read as healthy.
+    """
+    if cutoff is None or lastseenat is None:
+        return None
+    return lastseenat >= cutoff
+
+
+def _backup_stale_cutoff():
+    """(cutoff datetime, threshold days) for 'this PC's backup has gone quiet'.
+
+    A raw date makes the reader do the arithmetic, and the arithmetic is not
+    obvious: the collector only ATTEMPTS every backups_intervalhours (24 by
+    default), so a confirmation from yesterday is the healthy steady state, not
+    a warning. What answers the question is the verdict.
+
+    The threshold is the backups plugin's own backups_staledays, read through
+    its service so there is ONE definition of stale rather than a second one
+    drifting here. Returns (None, 0) when the plugin is absent or the check is
+    disabled, and callers then show the date alone.
+    """
+    try:
+        from plugins.backups.services import staleness
+    except ImportError:
+        return None, 0
+    try:
+        days = int(staleness.staledays())
+    except (TypeError, ValueError):
+        return None, 0
+    if days <= 0:
+        return None, 0
+    return service._utcnow() - timedelta(days=days), days
 
 
 def _report_stale_cutoff():
@@ -1108,6 +1174,7 @@ def list_reports():
                           if report.hostname})
 
     stalecutoff, stalafter = _report_stale_cutoff()
+    backupcutoff, backupafterdays = _backup_stale_cutoff()
     latest_cache = {}
     data = []
     for report in reports:
@@ -1142,13 +1209,23 @@ def list_reports():
             'mapy': known.get('mapy'),
             'machinenumber': known.get('machinenumber'),
             'machineassetid': known.get('machineassetid'),
+            'machinepluginid': known.get('machinepluginid'),
             'toolassetnumber': known.get('toolassetnumber'),
             'toolassetid': known.get('toolassetid'),
+            'toolpluginid': known.get('toolpluginid'),
             # Reported by the device wins; the DashboardDefault mapping is a
             # fallback for hosts on an older client that does not send it.
             'displayrole': report.subtype or known.get('displayrole'),
             'backupkind': known.get('backupkind'),
             'backuplastseen': known.get('backuplastseen'),
+            # VERIFIED, not "last backup taken". Dedup means an unchanged config
+            # writes no new revision, so lastseenat is the last time the
+            # collector CHECKED and found the backup intact - which is the
+            # question. backupok says whether that check is recent enough to
+            # trust; the date alone reads as "nothing has happened since", which
+            # is the healthy steady state and looks like neglect.
+            'backupok': _backup_ok(known.get('backuplastseenat'), backupcutoff),
+            'backupstaleafterdays': backupafterdays,
             'scopename': report.scopename,
             'phase': report.phase,
             'appliedversion': report.appliedversion,

plugins/geenforce/client/Register-ShopdbShadow.ps1

@@ -20,12 +20,28 @@ $ErrorActionPreference = 'Continue'
 $TaskName   = 'ShopDB GE-Enforce (shadow)'
 $InstallDir = 'C:\Program Files\GE\Shopfloor'
 $BaseUrl    = 'https://tsgwp00525.wjs.geaerospace.net/shopdb'
-$Scope      = 'gea-shopfloor-nocollections'
+# Scope is NOT hardcoded: this script is shipped by more than one scope
+# (collections and nocollections today), and a wrong value here would shadow the
+# wrong manifest silently. It runs from <drive>:\<scope>\shopdb-client, so the
+# directory it sits under IS the scope name - the same derivation used below for
+# the share manifest, so the two cannot disagree.
+$Scope      = Split-Path -Leaf (Split-Path -Parent $PSScriptRoot)
 
 function Write-ShadowLog {
+    # Write-Host ALONE is not enough: the engine records only "ps1: <path>" and
+    # the exit code for a PS1 entry, so nothing this script says reaches the
+    # enforce log. Combined with the fail-safe `exit 0` on every path, a silent
+    # early-out was indistinguishable from success - which is exactly how the
+    # never-firing task went unnoticed. Write to a file as well so the next
+    # failure is answerable from disk.
     param([string]$Message)
     $line = "[{0}] [shadow-setup] {1}" -f (Get-Date -Format 'yyyy-MM-dd HH:mm:ss'), $Message
     Write-Host $line
+    try {
+        $dir = 'C:\Logs\Shopfloor'
+        if (-not (Test-Path $dir)) { New-Item -ItemType Directory -Path $dir -Force | Out-Null }
+        Add-Content -LiteralPath (Join-Path $dir ('shadow-setup-{0}.log' -f (Get-Date -Format yyyyMMdd))) -Value $line
+    } catch { }
 }
 
 try {
@@ -60,6 +76,37 @@ try {
     $arguments = '-NoProfile -ExecutionPolicy Bypass -File "{0}" -Scope "{1}" -EnginePath "{2}" -ShadowMode -ShareManifestPath "{3}"' `
         -f $runner, $Scope, $engine, $shareManifest
 
+    # ONLY register when it is missing or its arguments changed.
+    #
+    # Registering unconditionally is what stopped this working the first time.
+    # A `-Once -At (Get-Date)` trigger does NOT fire immediately: the first run
+    # is start-boundary PLUS the repetition interval, so 15 minutes out. This
+    # entry is DetectionMethod=Always and runs every enforce cycle, 5 minutes
+    # apart, and each Register-ScheduledTask -Force reset the start boundary to
+    # "now" - pushing the first run back to +15 before the previous +15 could
+    # elapse. 5 < 15, so the task sat at Ready with LastTaskResult 267011
+    # (SCHED_S_TASK_HAS_NOT_RUN) indefinitely. Measured on the win11 VM.
+    $existing = Get-ScheduledTask -TaskName $TaskName -ErrorAction SilentlyContinue
+    if ($existing) {
+        $currentArgs = ($existing.Actions | Select-Object -First 1).Arguments
+        if ($currentArgs -eq $arguments) {
+            # Correct arguments, but a bay provisioned by the BROKEN version of
+            # this script carries a task that was reset every cycle and so has
+            # never run. Leaving it alone would strand exactly the bays that hit
+            # the bug. Kick it once; after that LastRunTime is set and this is a
+            # no-op forever.
+            $info = $existing | Get-ScheduledTaskInfo
+            $neverran = ($null -eq $info.LastRunTime) -or
+                        ($info.LastRunTime -lt (Get-Date '2000-01-01'))
+            if ($neverran) {
+                Write-ShadowLog ("task exists but has never run (LastTaskResult $($info.LastTaskResult)) - starting it once.")
+                Start-ScheduledTask -TaskName $TaskName -ErrorAction SilentlyContinue
+            }
+            return   # schedule is correct; do not reset the start boundary
+        }
+        Write-ShadowLog 'task arguments changed - re-registering.'
+    }
+
     $action = New-ScheduledTaskAction -Execute 'powershell.exe' -Argument $arguments
     # RepetitionInterval ALONE - passing RepetitionDuration serializes to a
     # Duration the Task Scheduler schema rejects.
@@ -69,6 +116,9 @@ try {
 
     Register-ScheduledTask -TaskName $TaskName -Action $action -Trigger $trigger `
         -Principal $principal -Settings $settings -Force | Out-Null
+    # Kick it once rather than waiting out the first 15-minute interval, so a
+    # freshly provisioned bay reports on this cycle instead of the next.
+    Start-ScheduledTask -TaskName $TaskName -ErrorAction SilentlyContinue
     Write-ShadowLog "registered '$TaskName' (every 15 min), shadowing $shareManifest"
     exit 0
 }

plugins/geenforce/frontend/views/EnforcementReports.vue

@@ -61,23 +61,33 @@
                 <!-- What this PC IS or DRIVES, whichever applies. A part-marker
                      or bay PC shows its machine; a measuring-tool PC its tool;
                      a display its role. -->
-                <router-link v-if="report.machineassetid" class="asset-chip"
-                             :to="`/machines/${report.machineassetid}`"
+                <!-- /machines/:id and /measuringtools/:id key on the PLUGIN
+                     extension id, NOT the core assetid. Linking with the assetid
+                     lands on whichever record happens to share that number - a
+                     wrong page that looks right. No plugin id means the plugin
+                     is not installed or the asset has no extension row, so show
+                     the number as plain text rather than a link that misleads. -->
+                <router-link v-if="report.machinepluginid" class="asset-chip"
+                             :to="`/machines/${report.machinepluginid}`"
                              title="Machine this PC controls">
                   {{ report.machinenumber }}
                 </router-link>
-                <router-link v-if="report.toolassetid" class="asset-chip"
-                             :to="`/measuringtools/${report.toolassetid}`"
+                <span v-else-if="report.machinenumber" class="asset-chip muted"
+                      title="Machine this PC controls">{{ report.machinenumber }}</span>
+                <router-link v-if="report.toolpluginid" class="asset-chip"
+                             :to="`/measuringtools/${report.toolpluginid}`"
                              title="Measuring tool this PC controls">
                   {{ report.toolassetnumber }}
                 </router-link>
+                <span v-else-if="report.toolassetnumber" class="asset-chip muted"
+                      title="Measuring tool this PC controls">{{ report.toolassetnumber }}</span>
                 <span v-if="report.displayrole" class="asset-chip muted"
                       title="Display role">{{ report.displayrole }}</span>
                 <!-- Deliberately NOT the PC's own asset number: the collector
                      stores a PC's hostname AS its assetnumber, so echoing it
                      here would just repeat the Host column. Blank means this PC
                      drives nothing and is not a display. -->
-                <span v-if="!report.machineassetid && !report.toolassetid && !report.displayrole"
+                <span v-if="!report.machinenumber && !report.toolassetnumber && !report.displayrole"
                       class="muted">-</span>
                 <div v-if="report.location" class="asset-location muted">{{ report.location }}</div>
               </td>
@@ -99,15 +109,19 @@
               <td>{{ report.installed }}</td>
               <td class="muted">{{ report.skipped }}</td>
               <td :class="{ 'fail-count': report.failed }">{{ report.failed }}</td>
-              <!-- Last CONFIRMED, not last changed: an unchanged config writes
-                   no revision, so a stable machine is healthy with an old
-                   revision. What matters is whether the backup still runs. -->
-              <td class="muted">
-                <template v-if="report.backuplastseen">
-                  {{ formatDate(report.backuplastseen) }}
-                  <div class="backup-kind">{{ report.backupkind }}</div>
-                </template>
-                <span v-else>-</span>
+              <!-- A VERDICT, not a date. An unchanged config writes no new
+                   revision, so the timestamp is the last time the collector
+                   CHECKED and found the backup intact - which read as neglect
+                   when shown raw, because the healthy steady state is a
+                   day-old confirmation. The badge names the kind and colours
+                   it good/stale; the date moves to the tooltip. Any backup
+                   kind added later inherits this with no change here. -->
+              <td>
+                <span v-if="report.backupkind"
+                      class="badge"
+                      :class="backupClass(report)"
+                      :title="backupTitle(report)">{{ report.backupkind }}</span>
+                <span v-else class="muted">-</span>
               </td>
               <td class="muted">{{ formatDate(report.lastcheckin || report.receivedat) }}</td>
               <td class="actions">
@@ -191,6 +205,27 @@ function statusClass(status) {
   return { ok: 'badge-success', selfhealed: 'badge-info', failed: 'badge-danger' }[status] || ''
 }
 
+// null verdict = nothing to judge (no revision, or the check is disabled).
+// Deliberately NOT green: "never seen" must not read as healthy.
+function backupClass(report) {
+  if (report.backupok === true) return 'badge-success'
+  if (report.backupok === false) return 'badge-danger'
+  return ''
+}
+
+function backupTitle(report) {
+  const seen = report.backuplastseen ? formatDate(report.backuplastseen) : 'never'
+  if (report.backupok === true) {
+    return `${report.backupkind}: checked and verified ${seen}.`
+      + ' Config unchanged since, which is why the date does not move.'
+  }
+  if (report.backupok === false) {
+    return `${report.backupkind}: last confirmed ${seen}, more than`
+      + ` ${report.backupstaleafterdays} day(s) ago - the backup has stopped running.`
+  }
+  return `${report.backupkind}: last confirmed ${seen}.`
+}
+
 function staleTitle(report) {
   const received = report.receivedat ? formatDate(report.receivedat) : 'never'
   const checkin = report.lastcheckin ? formatDate(report.lastcheckin) : 'not reported'