Mapped drives existed only as a comma-joined line in the user context card, which is fine for three drives and unreadable for a dozen. They now get their own list card in Monitors: one row per letter, remote path as meta, health as a badge. Reuses the statecolor/levels vocabulary the tracked-services cards already declare rather than new CSS. Only unhealthy drives are coloured. Green on every working drive makes the broken one just another badge in a row of badges, and the card exists to make the exception findable at a glance. Unhealthy rows sort first for the same reason - a user with a dozen mappings should not hunt for the broken one - and the sort is on health alone, so drive-letter order survives within each group and the list does not reshuffle between refreshes. The merge logic is untouched. _mapped_drive_rows now returns structured rows and the user-context summary line is built from them, so the card and the line cannot drift apart about a drive's status. The two silent-failure paths are mutation-tested: forcing every badge green failed 7 tests, and the first sort test passed a mutation that added a secondary key because every broken drive in it said the same word - it now uses statuses that would reorder under a text sort. Also corrects a stale warning claiming the merge is untested; it has been covered since Phase 1.
586 lines
31 KiB
Markdown
586 lines
31 KiB
Markdown
# Endpoint telemetry and PC Details UI - phased working notes
|
|
|
|
Scratch/reference notes for a multi-phase effort. Not a contract document. When
|
|
a phase lands, its details belong in AGENT-CONTRACT.md, the csproj changelog, or
|
|
the panel declarations - not here. Delete this file when all phases are done.
|
|
|
|
Branch: wilm. Current versions at time of writing: agent 1.8.0, tray 1.1.0.
|
|
|
|
---
|
|
|
|
## Ground truth established before scoping
|
|
|
|
Verified in code, not assumed. These constrain everything below.
|
|
|
|
**Panel groups are a hardcoded frontend list.**
|
|
`frontend/src/components/PluginAssetPanels.vue:423`:
|
|
|
|
const RENDERED_GROUPS = ['details', 'monitors', 'software', 'activity']
|
|
|
|
Any panel whose `group` is not in that list silently falls back to `details`.
|
|
Nothing is logged. The comment directly above it records that this has already
|
|
caused two disappearing-card incidents (`actions` -> `monitors`, and the
|
|
`metrics` -> `monitors` rename). Tab display labels are separately hardcoded in
|
|
`plugins/computers/frontend/views/PCDetail.vue:441-449`.
|
|
|
|
Consequence: introducing a new tab is a THREE-file change that must land in one
|
|
commit - panel `group` in `plugins/endpoints/plugin.py`, `RENDERED_GROUPS`, and
|
|
the PCDetail label map. Split it and cards vanish with no error.
|
|
|
|
**There is no pinned/locked card concept today.**
|
|
Layout is a CSS grid ordered purely by the integer `position` on each panel.
|
|
There is no "top row", no span control except one special case: the Network
|
|
interfaces table sets full-width span because at half width it wrapped every
|
|
cell to three lines (`PluginAssetPanels.vue:693`).
|
|
|
|
Consequence: "lock this card top-left/top-right" does not exist yet and is
|
|
itself a feature, not a config value. See Phase 5.
|
|
|
|
**Two of the requested cards are not plugin panels at all.**
|
|
Both the Entra/Intune card (`PCDetail.vue:202-243`) and the Network/IP card
|
|
(`PCDetail.vue:249-265`) are hand-written Vue inside PCDetail's details grid,
|
|
not declarations in `plugin.py`. Moving them is a component refactor, not a
|
|
`position`/`group` edit.
|
|
|
|
**There is no LAPS retrieval feature in this codebase.**
|
|
Searched: no LAPS button, endpoint, or credential path exists. The only hit is
|
|
a historical incident note in `plugins/tools/frontend/views/SecureSendCreate.vue:254`
|
|
about LAPS passwords once being sent over a chat link. The request assumes a
|
|
click-to-retrieve LAPS button already exists on the Entra card; it does not.
|
|
This needs its own scoping conversation - it is a credential-disclosure path
|
|
with real audit/permission implications and must not be smuggled in as part of
|
|
a layout change. See Open Questions.
|
|
|
|
**Existing panel positions in `monitors`** (so new cards do not collide):
|
|
10 status, 12 tracked services, 13 tracked processes, 14 hardware, 15 osinfo,
|
|
16 interfaces, 17 storage, 18 operator reports, 19 listeners, 40 user context.
|
|
`activity` uses 20. `software` uses 30.
|
|
|
|
---
|
|
|
|
## Phase 1 - Mapped drive reachability (agent + tray)
|
|
|
|
STATUS: code complete, NOT fully validated. See "Where we left off" at the end
|
|
of this file before resuming.
|
|
|
|
Highest value, and the reason the user-context work exists. Extends the probe
|
|
already added in agent 1.8.0 / tray 1.1.0.
|
|
|
|
**No active reachability testing.** An earlier draft proposed touching each
|
|
share to see if it responded, with a per-mapping timeout. Rejected: Windows
|
|
already reports the status, so probing the share is wasted work and reintroduces
|
|
the hang risk the bounded probe exists to avoid. Read the status, do not test it.
|
|
|
|
**The two status sources disagree, and that disagreement is the signal.**
|
|
Captured live on the same box at the same moment, same two mappings:
|
|
|
|
Get-SmbMapping net use
|
|
OK Q: ...specdata Disconnected Q: ...specdata
|
|
OK S: ...shared Reconnecting S: ...shared
|
|
|
|
`Get-SmbMapping` reported OK for both while `net use` reported Disconnected and
|
|
Reconnecting. So neither source alone is trustworthy: collecting only
|
|
Get-SmbMapping (the nicer, more structured one) would have reported a healthy
|
|
Q: on a box whose Q: was actually down - exactly the failure this card exists
|
|
to catch.
|
|
|
|
Collect BOTH and keep them as separate fields per mapping. Do not silently
|
|
reconcile them in the agent; the server/UI decides presentation, and preserving
|
|
the disagreement is diagnostically useful in its own right. The card should
|
|
surface the WORSE of the two rather than the prettier one.
|
|
|
|
Status vocabulary must include `Reconnecting` - it appears in the real capture
|
|
above and an earlier draft's `Ok`/`Disconnected`/`Unknown` set would have
|
|
forced it into the wrong bucket. Keep `Unknown` for probe failure so a failed
|
|
check never renders as green Ok.
|
|
|
|
**Elevation matters and confirms the tray approach.** The capture was taken
|
|
from an elevated shell (`start powershell -verb runas`). Because of UAC token
|
|
separation, an elevated context sees a DIFFERENT mapped-drive table than the
|
|
normal user - often empty. The tray runs unelevated as the operator, which is
|
|
the correct context to enumerate from. Do not "improve" this later by elevating
|
|
the probe; that would silently empty the card.
|
|
|
|
Mechanism: the probe already writes JSON to the spool file, so this rides the
|
|
existing handoff - run both commands in-session, write the combined result,
|
|
service reads it. No new transport.
|
|
|
|
Server: persist as part of the existing `usercontext` payload key (no new
|
|
column, no migration needed - it rides `usercontextjson`).
|
|
|
|
Note in code, commented out, per request: a future one-click remediation
|
|
command will re-establish a Disconnected mapping. Do not implement now, but
|
|
leave the command-catalog seam obvious. It will be an allowlisted action in
|
|
`plugins/endpoints/commands.py` and must run in the USER session (a SYSTEM
|
|
remap creates a mapping the operator cannot see, and per the elevation note
|
|
above an elevated remap has the same problem).
|
|
|
|
## Phase 2 - Cheap system-side telemetry (agent service)
|
|
|
|
All service-side, no session needed. Group these because they share the
|
|
inventory-snapshot plumbing and one migration.
|
|
|
|
- **Time sync offset and source** - CUT. Do not collect, do not surface.
|
|
w32time is Stopped on provisioned endpoints, confirmed on more than one box
|
|
(`get-service w32time` -> Stopped). The service appears to be stopped by
|
|
server-side policy after provisioning, so `w32tm /query /status` returns
|
|
0x80070426 "service has not been started" fleet-wide and there is no offset to
|
|
read. Collecting it would return null everywhere and produce an alerting
|
|
signal that never fires - worse than nothing, because a permanently-null
|
|
health check reads as "no problem found".
|
|
|
|
Implemented and then removed in this phase. If time skew becomes a real
|
|
suspect later, the prerequisite is getting w32time RUNNING on the fleet; the
|
|
collection is the easy part. One implementation note worth keeping: parse the
|
|
offset by LABEL, not by "first value ending in s". w32tm prints Root Delay and
|
|
Root Dispersion in the same seconds format BEFORE Phase Offset, so a
|
|
first-match parser silently reports network latency as clock skew. That bug
|
|
was written and caught here; do not rewrite it.
|
|
- **Disk errors / SMART predictive failure** - DONE. Landed as its own "Disk
|
|
health" card rather than folded into the Storage card as originally planned:
|
|
Storage is per VOLUME (C:, D:) and this is per PHYSICAL device, and one drive
|
|
can back several volumes or none the operator sees, so the two do not share a
|
|
key and could not be one table.
|
|
The important finding: the classic ATA SMART class
|
|
(`MSStorageDriver_FailurePredictStatus`) returns ZERO instances on an NVMe
|
|
machine. Verified on a two-NVMe box where both drives were present and
|
|
healthy. An agent collecting only that class would have reported an empty
|
|
disk health list on every modern endpoint and read as "no problems" - the
|
|
exact silent-healthy failure mode this telemetry exists to prevent. So the
|
|
agent collects BOTH `MSFT_PhysicalDisk` (the only NVMe-aware source) and
|
|
`Win32_DiskDrive.Status` (ATA/SATA, and hosts where the Storage namespace is
|
|
unavailable), and the server does not reconcile them - same reasoning as the
|
|
Phase 1 mapped-drive merge, where two disagreeing sources was the signal.
|
|
Note `MSFT_PhysicalDisk` returns raw enum INTEGERS through
|
|
ManagementObjectSearcher, not the friendly strings `Get-PhysicalDisk` prints
|
|
(Health=0 Op=2 Media=4 Bus=17 for a healthy NVMe); the mapping lives in the
|
|
collector so the wire stays self-describing.
|
|
- **NIC link speed and duplex** - DONE. Paired onto the existing interfaces
|
|
entries by MAC from `MSFT_NetAdapter` (`Win32_NetworkAdapterConfiguration`
|
|
carries no link state, so it is a second query). Both fields are null when
|
|
unreadable: verified on the live box that a DISCONNECTED adapter returns
|
|
empty Speed and FullDuplex, so defaulting would have reported every unplugged
|
|
NIC as half-duplex. Not surfaced.
|
|
- **Boot type** - DONE. Two facts kept separate: `hiberbootenabled` from the
|
|
registry (fast startup configured) and `lastboottype` from Kernel-Boot event
|
|
27 (what the last boot did). Neither substitutes for the other - the registry
|
|
cannot say what the last boot did, and the event can age out of the log.
|
|
Verified live: BootType=0 with HiberbootEnabled=0. Not surfaced; placement
|
|
still undecided.
|
|
- **Windows Update history, last 5** - PINNED, deferred out of Phase 2. The COM
|
|
update searcher is not cheap: it routinely takes seconds and sometimes tens of
|
|
seconds, which would make it the most expensive thing in an otherwise
|
|
WMI-and-registry `Collect()` running on the heartbeat path.
|
|
|
|
The blocking concern is broader than cost. Some endpoints host COM server
|
|
applications, and driving the update searcher through COM on those boxes risks
|
|
interfering with collections - or with the hosted application itself. Needs
|
|
its own investigation before any implementation, and if it lands it wants its
|
|
own cadence (hourly/daily), never the every-cycle path.
|
|
- **Defender/AV health** - DONE, collected and persisted, NOT surfaced, exactly
|
|
as specified. Note the ages are in days and a real 0 means updated today, so
|
|
the collector sends null rather than 0 when Defender did not report. A box
|
|
with third-party AV or a stripped image has no Defender WMI provider at all,
|
|
which is normal here and not a fault.
|
|
|
|
Explicitly dropped after discussion, do not collect:
|
|
certificates (corporate-managed), BitLocker (network join already requires it),
|
|
display topology (single monitor on the floor, trained users), session
|
|
type/RDP (RDP-in not permitted on the shop floor), logon server (local users).
|
|
|
|
**Pending file-rename operations** - DONE, but NOT as a new collection. It was
|
|
already being read: `GetPendingReboot()` OR-s it together with the CBS and
|
|
Windows Update keys into one bool. The gap was that the bool cannot say WHICH
|
|
signal fired, so the "stronger signal" was already being flattened into the
|
|
weaker one. Shipped as `rebootreasons`, a list naming the signals that fired,
|
|
leaving `pendingreboot` untouched for compatibility. Empty list = checked,
|
|
nothing pending; null = the check failed.
|
|
|
|
Schema: these add columns to `endpointstates`. This landed as `0015`, not the
|
|
`0014` reserved here - `0014` was taken by the disk health column earlier in
|
|
Phase 2.
|
|
|
|
## Phase 3 - Service/Process Monitors tab split
|
|
|
|
Tab churn is exactly what has silently eaten cards twice before, so this phase
|
|
is deliberately isolated from any collection work.
|
|
|
|
- Rename the existing `Monitors` tab to `System Monitors`.
|
|
- Add a new `Service/Process Monitors` tab.
|
|
- Move `Tracked Services` (position 12) and `Tracked Processes` (13) into it.
|
|
|
|
Must land atomically across all three places:
|
|
1. `group` on those panels in `plugins/endpoints/plugin.py`
|
|
2. `RENDERED_GROUPS` in `PluginAssetPanels.vue`
|
|
3. the tab label map in `PCDetail.vue`
|
|
|
|
Also update the hardcoded panel-count assertion in
|
|
`tests/test_plugins/test_endpoints_plugin.py`
|
|
(`test_endpoints_asset_panels_are_computer_scoped`) - it has already needed
|
|
bumping once (7 -> 12) and will fail again here.
|
|
|
|
Printers: a location-based printer auto-detect collection is in progress
|
|
elsewhere. Reserve space and a position band for a printer card in the monitors
|
|
family so this work does not have to be redone when it lands. Do not build the
|
|
card now.
|
|
|
|
## Phase 4 - Card content additions
|
|
|
|
Depends on Phase 2 data existing.
|
|
|
|
- **Mapped drives card** - DONE. `list` render at position 21 in Monitors, one
|
|
row per drive letter with the remote path as meta and a status badge. Reuses
|
|
the existing status/badge vocabulary (`statecolor` + the ok/warning/critical
|
|
levels the tracked-services cards already declare) rather than new CSS.
|
|
Only UNHEALTHY drives are coloured. Green on every working drive makes the
|
|
broken one just another badge in a row of badges; the card exists to make the
|
|
exception findable. For the same reason unhealthy rows sort first - a user
|
|
with a dozen mappings should not hunt for the broken one - and the sort is on
|
|
health ALONE so drive-letter order survives within each group and the list
|
|
does not reshuffle between refreshes.
|
|
The merge itself was not touched. `_mapped_drive_rows` now returns structured
|
|
rows and `_mapped_drives` (the user-context summary line) is built on top of
|
|
it, so the card and the summary cannot drift apart about a drive's status.
|
|
Room reserved for the one-click reconnect: the `list` render already supports
|
|
a row action, so adding it is a map/commandendpoint change, not a new card.
|
|
- **Storage card** - superseded. Disk health shipped in Phase 2 as its own
|
|
"Disk health" card (position 18, directly under Storage) instead of being
|
|
folded in: Storage is per volume, disk health is per physical device, so they
|
|
do not share a key. Rows sort failure-first, then unknown, then OK - a failing
|
|
drive must not sit below the fold on a multi-disk box, and a drive that
|
|
stopped answering is a weaker claim of health than one that said OK.
|
|
Later merged into a single two-section card; see the entry below.
|
|
- **Windows Update card** - BLOCKED, not scheduled. Depends on Phase 2 data
|
|
that is pinned pending a COM-interference investigation. Do not build the card
|
|
until the collection question is settled.
|
|
- Boot type placement - DONE in Phase 3. Rendered in the OS card only when fast
|
|
startup applies, because that is the case where uptime understates how long
|
|
the kernel has been up. A permanent "Cold" row would be noise.
|
|
- **Storage + Disk health into one card with two collapsible sections** -
|
|
DONE. VISUAL GROUPING ONLY. These two tables are not merged into shared rows:
|
|
Storage is per volume, Disk health is per physical device, and one drive can
|
|
back several volumes or none the operator sees. This is emphatically NOT the
|
|
Phase 1 mapped-drive treatment, where two sources described the same drive and
|
|
the worse status won. Nothing here is reconciled.
|
|
Built as predicted: `tabs` was extended to support table sections (columns/
|
|
rows/empty per section) in the shared renderer, and the two routes collapsed
|
|
into one `/storage` returning `sections`. `/diskhealth` is kept as a route -
|
|
it is a stable read for non-panel callers, and removing it for a UI-only
|
|
change would break them.
|
|
Two things only found by building it:
|
|
- The empty-section filter had to grow a table branch. A table section
|
|
declares its columns up front and so must survive having no rows; dropping
|
|
it takes its TAB with it, and a missing tab reads as "this machine has no
|
|
disks" rather than "nothing was reported".
|
|
- Full-width span was keyed off `render === 'table'`. A `tabs` panel holding a
|
|
six-column table needs the same span, and whether it does is only knowable
|
|
from FETCHED DATA, not the declaration - hence `spansFullWidth()`.
|
|
Both are mutation-tested in `tabSectionsTable.spec.js`.
|
|
Checked and cleared: the "inline label/value pairs, not nested cards" density
|
|
rule in Phase 5 does NOT block this. That rule is scoped to the PC Details
|
|
HEADER block, where the constraint is adding zero height so the tab strip is
|
|
not pushed toward the fold. It says nothing about panel bodies in the
|
|
Monitors tab.
|
|
|
|
## Phase 5 - Header right-hand block (DEFERRED - revisit before building)
|
|
|
|
Status: on hold at user request. The original ask assumed pinned cards were a
|
|
config value; they are not, and building a pinning system was not what the user
|
|
wanted to sign up for. Not scheduled. Nothing below is committed.
|
|
|
|
The cascade worry that motivated pinning is explicitly parked - do not build
|
|
anything for it unless reflow problems actually surface.
|
|
|
|
**Revised direction: use the empty right-hand half of the existing header.**
|
|
The PC Details header is a full-width card whose right side is currently dead
|
|
space - the hostname, badges, PC type, OS and location all sit left, and the
|
|
Status / Remote access / Refresh / Restart row runs along the bottom. The block
|
|
to the right of the identity lines is empty at every width.
|
|
|
|
This is a better target than either pinned grid cells or extending the header
|
|
downward:
|
|
- It consumes space that already exists, so it costs ZERO additional vertical
|
|
height and cannot push the tab strip toward the fold. That was the hard
|
|
constraint, and this approach satisfies it structurally rather than by
|
|
budgeting.
|
|
- It sidesteps the Network interfaces width problem entirely (that table is
|
|
full-width by necessity and would fight a half-width pinned slot).
|
|
- Header content lives outside the panel grid, so it cannot be reflowed by
|
|
cards appearing or disappearing below it.
|
|
|
|
Candidates for the block, unresolved:
|
|
- Entra/Intune deep links (currently a Details-tab card). Strongest candidate:
|
|
they are navigation, not data, and the user has called out their outbound
|
|
usability value.
|
|
- IP / network summary (currently the hand-written Network card).
|
|
|
|
Density rules if this proceeds: inline label/value pairs, not nested cards. Do
|
|
not let the right block grow taller than the left identity block, or it starts
|
|
adding height after all and the zero-cost property is lost.
|
|
|
|
Scope note: that "not nested cards" rule is about THIS header block only, where
|
|
the whole value is adding zero height. It is not a general ban on sections
|
|
inside a panel body - the Phase 4 Storage + Disk health grouping was checked
|
|
against it and is not blocked.
|
|
|
|
Still true and still unbuilt if this proceeds:
|
|
- `Collectors` is not a current tab or group. If IP History moves to a new tab,
|
|
that is the same three-file atomic change as Phase 3.
|
|
- Entra empty state, requested: show `Please provide DeviceID` /
|
|
`Please provide ObjectID` when the backend has no value, rather than an
|
|
empty or broken card.
|
|
|
|
LAPS is NOT a feature request - clarified by the user as an illustration of why
|
|
these outbound portal links matter to usability. No LAPS work is scoped.
|
|
|
|
## Phase 6 - Intune/Entra link bug (independent, can be done anytime)
|
|
|
|
Reported: the INTUNE and ENTRA buttons sometimes do not appear until the page
|
|
is refreshed. Diagnosed, root cause found, NOT yet fixed.
|
|
|
|
**Cause: a cold-cache race, not a render problem.**
|
|
|
|
`frontend/src/utils/siteSettings.js:9` caches only the RESOLVED settings object:
|
|
|
|
let settingsCache = null
|
|
async function loadSettings() {
|
|
if (settingsCache) return settingsCache
|
|
...
|
|
|
|
The in-flight promise is not cached, so every caller during the first load
|
|
fires its own `settingsApi.list()`.
|
|
|
|
In `plugins/computers/frontend/views/PCDetail.vue:513`, `getEntraDeviceUrls()`
|
|
is deliberately fire-and-forget (`.then(...)`, not awaited) so a settings
|
|
failure cannot block the page. Meanwhile `computersApi.get()` on line 519 IS
|
|
awaited. On a cold cache the settings response can therefore land AFTER the
|
|
computer data has rendered. Until it does, `entraUrlTemplate` /
|
|
`intuneUrlTemplate` are still `''`, so `fillTemplate()` returns null
|
|
(line 426: `if (!template || !value) return null`), and the `v-if` on the
|
|
anchors hides both links. A later refresh hits the warm cache and they appear.
|
|
|
|
That also explains the intermittency: it depends on which response wins, so it
|
|
varies by network timing and by whether another view already warmed the cache.
|
|
|
|
**Fix: cache the in-flight promise, not just the result.** Assign the promise
|
|
to `settingsCache` immediately so concurrent callers await the same request.
|
|
This is a one-line-shaped change in `siteSettings.js` and fixes every consumer
|
|
of site settings, not just this card - the same race exists for any view that
|
|
reads settings during mount.
|
|
|
|
Watch for: the existing `catch` sets `settingsCache = {}` on failure, which
|
|
permanently caches the empty result for the page session. Confirm whether a
|
|
failed settings load should stay cached (currently it does) or be retryable.
|
|
Do not change that behavior silently while fixing the race.
|
|
|
|
Verify after fixing that the links appear on a hard reload with a cold cache
|
|
and throttled network, which is the condition that reproduces it.
|
|
|
|
DONE. Fixed in `siteSettings.js`: the in-flight promise is now cached alongside
|
|
the result, and a failed load is cached for 30s rather than the page session.
|
|
Covered by `frontend/src/utils/siteSettings.spec.js` (6 tests). Open question 5
|
|
resolved in favour of the short TTL.
|
|
|
|
Measured side benefit: the old code assigned `settingsCache` only AFTER the
|
|
response resolved, so each sequential `await` re-entered `loadSettings()` before
|
|
the cache existed. `applyBranding()` awaits five settings in a row at bootstrap,
|
|
`getServicenowUrls()` four, `getEntraDeviceUrls()` two - all of which were
|
|
separate GETs on a cold load. Now one.
|
|
|
|
## Phase 7 - PCForm "Failed to load data" (NOT the same bug)
|
|
|
|
Separate from Phase 6 and NOT caused by it - `PCForm.vue` does not import
|
|
`siteSettings.js`. Surfaced on the F3KKSGH4 edit page.
|
|
|
|
Partially addressed, root cause still UNKNOWN.
|
|
|
|
Evidence: the form renders with reference data present (printer list populated,
|
|
dropdowns built) but record fields empty. That means the eight-way
|
|
`Promise.all` of reference endpoints SUCCEEDED and the failure is in the
|
|
edit-only block, `PCForm.vue` lines 589-635. The only unprotected call there is
|
|
`computersApi.get(route.params.id)`; `ensureModelLoaded()` already swallows its
|
|
own errors, so it is not the source.
|
|
|
|
Two candidates remain, with different fixes and different layers:
|
|
- the `computersApi.get` request fails (backend/route problem), or
|
|
- a TypeError while mapping the response into `form.value` (frontend shape
|
|
mismatch, plausibly from recent schema work).
|
|
|
|
Both land in the same catch and both printed the same generic string, which is
|
|
why this could not be diagnosed from the UI.
|
|
|
|
Done so far: the catch now uses `apiError(err, ...)` so the real server message
|
|
surfaces instead of a generic one, and the stale comment claiming a call
|
|
"handles its own failure" was removed - no such call remains in that list.
|
|
|
|
NEXT: reload the edit page and read the now-specific error. An axios status
|
|
code points at the backend; a TypeError names the property and points at the
|
|
mapping block. Do not guess between them.
|
|
|
|
Also worth doing regardless: a single failed reference list currently blanks
|
|
the whole form via `Promise.all` rejection. Degrading one dropdown instead
|
|
would be the behaviour the removed comment falsely claimed.
|
|
|
|
---
|
|
|
|
## Open questions - answer before the relevant phase
|
|
|
|
1. ~~LAPS~~ - RESOLVED: not a feature request. The user was illustrating why the
|
|
outbound Intune/Entra links matter to usability. No LAPS work scoped. The
|
|
security concerns noted earlier stand if it is ever revisited.
|
|
2. ~~Pending file-rename operations~~ - RESOLVED: collect. Folded into Phase 2.
|
|
3. ~~Phase 5 pinning / cascade symptom~~ - RESOLVED: not worrying about the
|
|
cascade unless it surfaces. Pinned cards dropped in favour of the header's
|
|
empty right-hand block; Phase 5 deferred.
|
|
4. **`Collectors` tab** - only relevant if Phase 5 proceeds. New tab for one
|
|
moved card, or does IP History belong in an existing tab?
|
|
5. **Failed settings load caching** - Phase 6. Should a failed
|
|
`settingsApi.list()` stay cached for the page session (current behavior) or
|
|
be retryable? Decide while fixing the race, do not change it silently.
|
|
|
|
## Sequencing
|
|
|
|
1 -> 2 -> 3 -> 4. Phase 6 is independent and can be slotted in anytime; it is
|
|
small, self-contained, and fixes a user-visible bug, so it is a good candidate
|
|
to do early. Phase 5 is deferred and unscheduled.
|
|
|
|
Phases 1 and 2 are additive and independently deployable. Phase 3 is a
|
|
contained tab move. Phase 4 needs 2.
|
|
|
|
Agent/tray version bumps: Phase 1 touches the probe (tray behavior) and Phase 2
|
|
touches the service collector. Both need csproj changelog entries. Note the
|
|
existing coupling hazard - `Program.cs` has no unknown-arg guard, so an older
|
|
agent receiving a newer tray's verb starts a full second host rather than
|
|
no-oping. Fix that guard before any deployment where tray and agent could skew.
|
|
|
|
## Phase 8 - fetchAllPages returns a bare array (LAST, and it affects the above)
|
|
|
|
Deliberately last: it is a cross-cutting API-surface change, and every earlier
|
|
phase should land against the current shape rather than a moving one.
|
|
|
|
`frontend/src/api/index.js:131` - `fetchAllPages()` returns the ROWS:
|
|
|
|
return items
|
|
|
|
Every other method on every API object returns the axios RESPONSE. So callers
|
|
must remember, per call site, which of two shapes they are holding:
|
|
|
|
const rows = await vendorsApi.listAll() // array
|
|
const response = await vendorsApi.list() // { data: { data: [...] } }
|
|
|
|
Reading `.data.data` off the array yields `Cannot read properties of undefined
|
|
(reading 'data')`. Inside a `Promise.all` that rejects the whole batch, so one
|
|
wrong unwrap blanks an entire form - and if it throws before the record fetch,
|
|
NO request is made at all, which makes it look like a backend or schema fault.
|
|
That is exactly how the PC edit page presented, and why it was misdiagnosed
|
|
twice before the log showed `/api/computers/729` was never requested.
|
|
|
|
Known history - this class of bug has now hit at least four times:
|
|
- `MachineForm.vue` `pcsRes` (fixed previously, comment left in place)
|
|
- `PCForm.vue` `vendorRes` (fixed now)
|
|
- `MachineForm.vue` `vendorRes` and `buRes` (fixed now - the warning comment was
|
|
already sitting three lines away and the adjacent defects were still missed)
|
|
- `MeasuringToolForm.vue` `buResponse` (fixed now)
|
|
|
|
`ConfigurationPoliciesList.vue:187` defends with
|
|
`Array.isArray(value) ? value : ...`, which is evidence the shape is not
|
|
trustworthy at the call site.
|
|
|
|
Why this matters to the phases above: Phase 1/2 UI work adds call sites that
|
|
consume paged reference data, and Phase 5 (if revived) moves cards between
|
|
components. Any new `listAll()` caller is another chance to hit this. Landing
|
|
the phases first and this last means one migration pass, not a moving target.
|
|
|
|
Options, in order of preference:
|
|
1. Make `fetchAllPages` return `{ data: { data: items } }` so every API method
|
|
has ONE shape. Mechanical but touches every existing `listAll()` caller in
|
|
the same commit, or they break in the opposite direction. Highest value,
|
|
highest blast radius - needs the full grep and a single atomic change.
|
|
2. Rename to something the shape is obvious from (`fetchAllRows`,
|
|
`listAllRows`). Cheap, no behavior change, keeps the trap but makes it
|
|
visible at the call site. Good interim step if 1 is too large right now.
|
|
3. Leave it and rely on review. Rejected - four occurrences say review does not
|
|
catch it, and one of them happened directly beneath a comment warning about
|
|
it.
|
|
|
|
Whichever is chosen, do the full-codebase grep for `listAll(` first; there are
|
|
callers in core views, print views, and at least seven plugin frontends.
|
|
|
|
---
|
|
|
|
# Where we left off
|
|
|
|
Written mid-session when the test tooling stopped working. Read this first.
|
|
|
|
## Phase 1 is code complete but NOT fully validated
|
|
|
|
What landed, all uncommitted in the working tree:
|
|
|
|
**`agent/ShopDbEndpointAgent.Core/UserContextProbe.cs`** - added `Get-SmbMapping`
|
|
alongside the existing `Win32_NetworkConnection` capture, emitted as a separate
|
|
`smbmapping` key with its own `smbmappingerror`. The two sources are NOT
|
|
reconciled in the probe, deliberately - see the Phase 1 notes above. Still
|
|
unelevated, still in the operator session.
|
|
|
|
**`plugins/endpoints/api/routes.py`** - added `_drive_letter`,
|
|
`_DRIVE_STATUS_RANK`, `_worse_drive_status`, `_mapped_drives`, and pointed
|
|
`panel_usercontext()` at `_mapped_drives()`. Merges the two views by drive
|
|
letter and shows the WORSE status. Carries a long WARNING comment naming the
|
|
three load-bearing behaviours; that comment is doing real work, leave it.
|
|
|
|
**`plugins/endpoints/commands.py`** - commented-out `remap_drive` seam with the
|
|
user-session/elevation reasoning. Comment only, no behavior.
|
|
|
|
## Validation state - the important part
|
|
|
|
Last KNOWN-GOOD full run: `8 failed, 2489 passed, 11 skipped`. All 8 failures
|
|
are pre-existing and unrelated:
|
|
- 3x `test_endpoints_plugin.py` timestamp tests - these fail ONLY on Windows.
|
|
`_in_zone()` calls `time.tzset()`, which does not exist on Windows (Unix-only
|
|
in CPython), so it raises AttributeError before the assertions ever run.
|
|
`_parse_dt` itself is correct. These pass on CI. Consider a
|
|
`skipif(not hasattr(time, 'tzset'))` so the suite is honest on Windows.
|
|
- 5x docs / security-headers / plugin-guard, all pre-existing.
|
|
|
|
TWO CHANGES LANDED AFTER THAT RUN AND ARE UNVERIFIED:
|
|
1. `_mapped_drives()` now renders `(Unknown)` for a drive neither source gave a
|
|
status for. Previously it rendered blank, which reads as healthy - the exact
|
|
silent-green failure the warning comment warns about. Small but real,
|
|
behavioral, and untested.
|
|
2. The `commands.py` seam. Comment only, zero risk.
|
|
|
|
`get_errors` on both changed files is clean, but that only proves they parse.
|
|
|
|
## Tooling failure that stopped validation
|
|
|
|
Every `run_command_in_terminal` call began failing with "A task was canceled",
|
|
including a two-file pytest run that should finish in under a second, and a
|
|
background launch that never wrote its `Tee-Object` output file. Two full-suite
|
|
runs had succeeded earlier in the same session, so this is not slowness - the
|
|
execution path went down partway through.
|
|
|
|
Test Explorer discovers all 2508 tests but returns zero results for every
|
|
filter tried (`TypeName`, `MethodName`, `FullyQualifiedName`). Likely needs a VS
|
|
restart.
|
|
|
|
## Next actions, in order
|
|
|
|
1. Restart VS, then run `python -m pytest tests/test_plugins/test_endpoints_plugin.py -q`.
|
|
Expect 3 failed (the tzset ones) / 23 passed. Anything else is ours.
|
|
2. Consider adding tests for `_mapped_drives()` - currently zero coverage on
|
|
logic that decides whether a broken drive renders as broken. The cases that
|
|
matter: sources disagree (worse wins), drive in only one source (kept), no
|
|
status at all (Unknown, not blank), `Get-SmbMapping` absent entirely
|
|
(degrades to netuse alone).
|
|
3. Only then start Phase 2. Do not begin new collection work on an unverified
|
|
base - the untested `(Unknown)` delta would get buried under it.
|
|
|
|
## Deferred, agreed explicitly
|
|
|
|
Agent and tray republish happens at the END of all phases, not per-phase. The
|
|
server change is safe to deploy ahead of the agent: until the new probe ships,
|
|
`smbmapping` is simply absent from the payload and `_mapped_drives()` falls
|
|
through to `netuse` alone, rendering exactly as it does today. No ordering
|
|
constraint between them.
|
|
|