Skip to content

feat(dgw): search Active Directory Console logs - #2150

Merged
irvingouj@Devolutions (irvingoujAtDevolution) merged 4 commits into
masterfrom
feat/recording-search-all-slogs
Oct 28, 2026
Merged

irvingouj@Devolutions (irvingoujAtDevolution) merged 4 commits into
masterfrom
feat/recording-search-all-slogs

Conversation

@irvingoujAtDevolution

@irvingoujAtDevolution irvingouj@Devolutions (irvingoujAtDevolution) commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Search read only the .slog files listed under artifacts in recording.json. Active Directory Console pushes its log as a recording, listed under files, so none of its logs, stored ones included, could be found.

Search now reads the .slog files under files too, before those under artifacts, in the order the manifest and the pull ZIP use. A name listed in both sections is searched once, so a cursor still resumes at a single place. Other file types under files, such as .webm, and unsafe names are still skipped.

api/jrec/search.intent.md states the new rules.

Issue: DGW-463

Active Directory Console pushes its log as a recording, so it sits
under `files` in `recording.json`, where search does not look.

The search intent now covers the `.slog` files listed under `files`
as well as under `artifacts`, `files` first, and defers choosing
other file types.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Search read only the `.slog` files listed under `artifacts` in
`recording.json`. Active Directory Console pushes its log as a
recording, listed under `files`, so none of its logs, stored ones
included, could be found.

Search now reads the `.slog` files under `files` too, before those
under `artifacts`, in the order the manifest and the pull ZIP use. A
name listed in both sections is searched once, so a cursor still
resumes at a single place. Other file types under `files`, such as
`.webm`, and unsafe names are still skipped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 23, 2026 22:47
@irvingoujAtDevolution

irvingouj@Devolutions (irvingoujAtDevolution) commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Lines changed, against master:

Added Removed
Production code 17 8
Tests 44 8
Other (intent file) 5 4

The behaviour change is read_recording_logs in search.rs; the rest of the production lines are doc comments.

#2114 copies the old search doc ("Only the files listed under artifacts …") into the OpenAPI document and the clients, so whichever of the two merges second regenerates them.

Note

LLM-assisted content (no human feedback).

@irvingoujAtDevolution

Copy link
Copy Markdown
Contributor Author

Intent drafts: handoff

The interview read devolutions-gateway/src/api/jrec/search.intent.md at 0467406c7 (master with cursor paging), and an intent review of the first draft added three more rounds.

Provenance

Levels: 1 = my words verbatim, 2 = my words tidied, 3 = agent wording I adopted, 4 = agent fill-in, not intent.

# Commitment as written My words (if tidied) Level
1 Search covers the .slog files listed in recording.json, under files as well as under artifacts. "To keep it backward compatiable, with the exception of Active Directory logging, which is pushed as a recording instead of artifacts, we extend the search to all content that is .slog, no matter it is an artifact or it is a file"; the review split it into a rule and a reason and dropped "every", which the safe-name rule falsified: "I agree" 3
2 files is searched so that Active Directory Console logs, which are pushed as recordings, are searchable, the ones already stored included, with no change to how they are pushed. Same answer as 1; the reason "backward compatible" named no party, so the review spelled it out: "I agree" 3
3 Within a recording, the .slog files listed under files are searched first, then those listed under artifacts, by convention, in the order recording.json lists them and a recording pull zips them. "file first"; "no reason, done by convention, in sync with order in manifest and zipping" 2
4 Any other line is matched case-insensitively against its raw text, so that any kind of log, present or future, can be searched. The existing reason, "artifact" widened to "log" since logs under files are searched too: "I agree" 3
5 Choosing which file types to search, such as .cast besides .slog, is wanted later; the request shape must not preclude it either. "choosing which file kinds to search is a wanted later"; of two readings, log kinds or file types: "2" 2

Left out on purpose

  • A .slog added under files while the cursor is inside an artifact of the same recording is not searched by that paging run. No current producer writes a .slog under both sections of one recording.
  • A manifest without files, or with a files entry missing fileName: Gateway always writes both, so search keeps reading the first and skips the second, as a pull does.
  • Access: §Access already makes the search scope the only check for any content, Active Directory Console logs included.
  • Choosing by log kind, such as only Active Directory Console logs: not committed.
  • Anchors: the file has none while tests cite its sections by heading; that predates this change and goes in a pull request of its own.
  • AD pushing an artifact-only session, and a separate endpoint to push any .slog: not part of this change.

Placement

All in devolutions-gateway/src/api/jrec/search.intent.md: 1 to 3 in §What is searched, 4 in its matching rules, 5 in §Later.

Pending experiments

None.

Note

Human-tuned, LLM-assisted content.

@github-actions

Copy link
Copy Markdown

Let maintainers know that an action is required on their side

  • Add the label release-required Please cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module) when you request a maintainer to cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module)

  • Add the label release-blocker Follow-up is required before cutting a new release if a follow-up is required before cutting a new release

  • Add the label publish-required Please publish libraries (`Devolutions.Gateway.Utils`, OpenAPI clients, etc) when you request a maintainer to publish libraries (Devolutions.Gateway.Utils, OpenAPI clients, etc.)

  • Add the label publish-blocker Follow-up is required before publishing libraries if a follow-up is required before publishing libraries

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused change retains existing filtering and deduplication, adds regression coverage, and has no identified blocking issues.

No open issues 🎉

What changed in this PR

This PR extends recording search to include Active Directory Console .slog logs, including existing recordings, without changing how they are uploaded.

Changes:

  • Search manifest files before artifacts, deduplicating names while retaining filename safety and file-type filtering.
  • Update regression tests for expanded results, pagination order, and duplicate log entries.
  • Document the expanded search scope and ordering in the search intent.
File Description
testsuite/​tests/​cli/​dgw/​recording_search.rs Updates expected results and pagination to include recorded logs.
devolutions-gateway/​src/​api/​jrec/​search.rs Adds manifest files to log discovery and tests cross-section deduplication.
devolutions-gateway/​src/​api/​jrec/​search.intent.md Defines the expanded search scope and file ordering.

Review effort: Balanced

Within a recording, search reads the `.slog` files under `files`
first, then those under `artifacts`, each in the order
`recording.json` lists them. No test listed two logs in one section,
so reversing or sorting either section went unnoticed.

Issue: DGW-463

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@irvingoujAtDevolution

Copy link
Copy Markdown
Contributor Author

Test pruner (editing mode)

dependency-pruner does not apply: this PR changes no Cargo.toml or Cargo.lock.

Result: one coverage gap, closed by c7edac9 (lists_logs_in_manifest_order_files_first). Nothing deleted, consolidated, or strengthened, since every test in scope is independent and the two repeat cases are already one parametrized test.

Scope: the tests this PR adds or changes, against read_recording_logs in devolutions-gateway/src/api/jrec/search.rs. Each mutant ran against two filters: api::jrec::search::tests:: (60 unit tests, 61 with the new one) and cli::dgw::recording_search:: (14 testsuite tests).

Oracle table

All sections are in devolutions-gateway/src/api/jrec/search.intent.md.

Test Class Source
lists_a_log_once_when_the_manifest_repeats_it::twice_in_artifacts independent §What is searched: each log is searched once, even when the manifest lists it twice
lists_a_log_once_when_the_manifest_repeats_it::in_files_and_in_artifacts independent same sentence; a name under both files and artifacts is listed twice
lists_logs_in_manifest_order_files_first (new) independent §What is searched: files first, then artifacts, in the order recording.json lists them; the test picks a non-alphabetical order and checks it is kept
searches_the_slog_files_of_every_recording_newest_first independent §What is searched: .slog under files and artifacts, files first, ../outside.slog and .webm not searched, a missing log skipped; §Request: "all" puts the latest start first; §Pages: no nextCursor once every log is read
searches_each_listed_recording_once independent §Request: a repeated ID is searched once; §What is searched for which hits
pages_through_listed_recordings_in_their_order independent §Request: a list keeps its order; §What is searched: files first; §Pages: with maxHits 1, each further hit opens the next page and the last page has no cursor, so 3 hits give 3 pages

searches_the_slog_files_of_every_recording_newest_first asserts that nextCursor is absent, which §Pages decides, but its oracle: markers name only §What is searched and §Request.

Kill matrix

Mutants are hand-written patches (domain mutants), applied in a separate clone and reverted after each run. x = caught, . = survived, – = not run.

Mutant T1 T2 T3 T4 T5 T6 Other scoped tests that caught it
pc-0 positive control: no manifest is read x x – x x x 2 unit, 10 testsuite
dm-1 artifacts before files . . – x x x none
dm-2 files in reverse order . . x . . . none
dm-3 files ignored . . – x x x none
dm-4 duplicates dropped only within one section . x – . . . none
dm-5 safe-name check skipped for files . . – x x x does_not_follow_a_cursor_out_of_its_recording
dm-6 every file type under files listed, not only .slog . . – x x x none
dm-7 artifacts in reverse order . . x . . . none
dm-8 names sorted within each section . . x . . . none

T1 lists_a_log_once_when_the_manifest_repeats_it::twice_in_artifacts, T2 …::in_files_and_in_artifacts, T3 lists_logs_in_manifest_order_files_first (new), T4 searches_the_slog_files_of_every_recording_newest_first, T5 searches_each_listed_recording_once, T6 pages_through_listed_recordings_in_their_order.

Unique kills: dm-4 (T2) and dm-2, dm-7, dm-8 (T3). Under dm-2, dm-7 and dm-8, T3 fails on its order assertion, for example dm-2 gives ["recording-0.slog", "recording-1.slog", …] where ["recording-1.slog", "recording-0.slog", …] is expected.

Survivors

Before the new test, dm-2 survived both suites, and dm-7 and dm-8 survived the testsuite. The new test was written after dm-2 survived; dm-7 and dm-8 were added to check it.
All three are coverage gaps: §What is searched requires the manifest order, but the testsuite fixture leaves only one readable .slog in each section of a recording (recording-0.slog under files, ai-analysis-0.slog under artifacts), so reordering a section changes no hit.
With the new test, no mutant survives.

What this run covered

Covered:

  • Clean tree, 3 runs of both filters, all green, no flaky test seen. The new test passed on 3 more clean runs of the unit filter.
  • 1 positive control and 8 domain mutants, each run against both filters. dm-2 ran against the testsuite before the new test only, since a unit test cannot change testsuite results.

Not covered:

  • No cargo-mutants run (generic mutants) on this diff.
  • 3 clean repeats instead of the skill's example of 15.
  • T3 did not run under pc-0, dm-1, dm-3, dm-4, dm-5 or dm-6.
  • No oracle review of the scoped tests this PR does not change. They only count in the "other scoped tests" column.
  • No challenge step: it applies only to tests of unknown oracle, and none are in scope.

Note

LLM-assisted content (no human feedback).

The test asserts that a fully read search returns no `nextCursor`, which
is a rule of search.intent.md §Pages.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@CBenoit Benoît Cortier (CBenoit) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you Irving.

So the idea is to search every .slog files by default, and to support more granuler filters later on?

My suggestion would be to think about some query syntax such that the caller can express "just .slog files for ai-analysis", or "all .slog files including from files and all the artifacts", and eventually other kinds of more powerful queries.

That being said, the current default seems good enough, and any more complex query system can wait.

LGTM

@irvingoujAtDevolution
irvingouj@Devolutions (irvingoujAtDevolution) merged commit 2639960 into master Oct 28, 2026
53 checks passed
@irvingoujAtDevolution
irvingouj@Devolutions (irvingoujAtDevolution) deleted the feat/recording-search-all-slogs branch October 28, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants