Skip to content

feat(version.lic): v1.3.0 add stale session tracking#2265

Merged
mrhoribu merged 11 commits intomasterfrom
feat/version_track_stale_sessions
Apr 3, 2026
Merged

feat(version.lic): v1.3.0 add stale session tracking#2265
mrhoribu merged 11 commits intomasterfrom
feat/version_track_stale_sessions

Conversation

@OSXLich-Doug
Copy link
Copy Markdown
Contributor

@OSXLich-Doug OSXLich-Doug commented Mar 8, 2026

What Changed

Integrated core snapshot module
Updated snapshot source from Lich::Common::SessionSettings to Lich::Common::SessionsSettings.
Preserved fallback behavior when core snapshot is unavailable.
Added user-facing session health summary
Introduced top-level computed summary fields:

  • Total ruby sessions
  • Potential stale sessions
  • Recommendation

Added helper methods to compute:

stale count (potential_stale_sessions_count)
duplicate session-name count (duplicate_session_name_count)
recommendation text (session_recommendation)
Reduced default output noise
Moved technical per-session rows to all/full/details mode only.
Default ;version now favors concise, actionable output.

Updated placement and emphasis

Moved session summary out of the top report body to a dedicated section near the end.
Added visual separators.
Added monsterbold recommendation label via Lich::Messaging.monsterbold('Recommendation..........').
Script metadata/versioning
Bumped script header version to 1.3.0.
Added changelog entries for the above behavior changes.

Why

Align version.lic with new core session summary infrastructure.
Provide non-technical users actionable guidance without overwhelming detail.
Keep deep diagnostics available when explicitly requested.

NOTE: Requires Lich core PR# 1247

Summary by CodeRabbit

  • New Features

    • Session diagnostics with health recommendations, per-session details in detailed report mode, and feature-flagged session summary
    • Automatic identification of active, stale, and duplicate sessions
  • Improvements

    • Expanded script discovery with improved recursive traversal and more robust custom-script detection
    • Unified autostart script loading via a reusable helper
    • Refined output formatting and stricter exception handling for increased robustness

Updated version to 1.3.0 and added new features including session snapshot integration and simplified reporting.
@OSXLich-Doug
Copy link
Copy Markdown
Contributor Author

new feature adds this (where appropriate!)

image

@mrhoribu mrhoribu changed the title feat(all): version now adds stale session tracking feat(version.lic): v1.3.0 add stale session tracking Mar 9, 2026
@mrhoribu
Copy link
Copy Markdown
Contributor

mrhoribu commented Mar 9, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 9, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 9, 2026

📝 Walkthrough

Walkthrough

Adds SessionsSettings-driven session snapshot integration, per-session normalization and health metrics (stale/duplicate/active), recommendation generation, autostart script loader, expanded report/detail rendering, and safer StandardError-based error handling in scripts/version.lic. (50 words)

Changes

Cohort / File(s) Summary
Session snapshot & feature gate
scripts/version.lic
Adds session_summary_enabled?, stub_session_snapshot, session_snapshot, normalize_session_snapshot and feature-flagged retrieval with fallback and error capture.
Session metrics & helpers
scripts/version.lic
Adds potential_stale_sessions_count, duplicate_live_session_count, active_session_count, session_recommendation, and helpers session_name_value, session_game_code_value, session_pid_value, session_os_seen? for normalization and diagnostics.
Reporting / detail output
scripts/version.lic
Extends main report to include total/stale/duplicate counts and recommendation; in detail/full modes appends per-session diagnostics (pid, name, state, marker) with conditional emphasis for issues.
Autostart script discovery
scripts/version.lic
Introduces load_autostart_scripts(scope) to recursively discover autostart scripts, filter actual files, standardize naming, and tolerate load errors.
Error handling & minor refactors
scripts/version.lic
Replaces broad rescues with StandardError, switches to File.read and each iterations, uses .any? for emptiness checks, and replaces regex-based custom path checks with include?('/custom/').

Sequence Diagram(s)

sequenceDiagram
  participant CLI as CLI/Invoker
  participant VS as VersionScript
  participant SS as SessionsSettings
  participant FS as Filesystem
  participant R as Reporter

  CLI->>VS: invoke report
  VS->>SS: request session snapshot (if feature enabled)
  SS-->>VS: snapshot or error
  VS->>VS: normalize snapshot (or stub) and compute metrics (active, stale, duplicate)
  VS->>FS: load_autostart_scripts(scope)
  FS-->>VS: discovered scripts list
  VS->>R: assemble summary, recommendation, and (detail mode) per-session diagnostics
  R-->>CLI: render output
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(version.lic): v1.3.0 add stale session tracking' directly corresponds to the main changes in the PR: version bump to 1.3.0 and implementation of stale session detection logic with SessionsSettings integration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/version_track_stale_sessions

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/version.lic`:
- Around line 331-335: session_summary currently sets total: active_sessions
which mislabels the total count; update the session_summary assignment (symbol:
session_summary) so that total uses the real total value (session_state[:total])
instead of active_sessions, and keep stale and issue logic the same (stale:
stale_sessions, issue: stale_sessions.positive? ||
duplicate_sessions.positive?); alternatively, if the intent is to report active
sessions, change the external label that prints the value (where it reads "Total
ruby sessions") to "Active ruby sessions" to match active_sessions—pick the
correct semantics and make the corresponding change consistently (references:
session_summary, active_sessions, session_state[:total],
session_recommendation).
- Around line 265-272: active_session_count and the detail rendering assume
symbol keys; update them to tolerate string keys by normalizing or
dual-accessing keys: use Array(session_state[:sessions] ||
session_state['sessions']) in active_session_count, and for each session (s)
check marker via s[:marker] || s['marker']; if marker exists compare to
'active'; otherwise read state via s[:state] || s['state'] and stale via
s[:stale] || s['stale'] to decide active-ness; apply the same normalization
pattern to the detail output code paths referenced around lines 468-473 so
`"marker"=>"stale"`/`"state"=>"exited"` are handled correctly.
- Around line 199-205: The potential_stale_sessions_count method currently
returns session_state[:stale].to_i whenever the :stale key exists (via
session_state.key?(:stale)), but normalization always injects that key so the
row-level fallback never runs; change the guard to detect a real value instead
of mere presence — replace the session_state.key?(:stale) check with a nil-check
(e.g. return session_state[:stale].to_i if !session_state[:stale].nil?) so that
when :stale is not set (nil) the method falls back to iterating
Array(session_state[:sessions]) and applying the marker/heartbeat_age logic.
- Around line 176-179: The current mapping keeps non-Hash entries by returning
them from the block; change the normalization to drop malformed rows by first
filtering to only Hashes and then mapping: replace the
Array(snapshot[:sessions]).map { |session| next session unless
session.is_a?(Hash); ... } logic with a two-step filter-then-map (e.g.
Array(snapshot[:sessions]).select { |s| s.is_a?(Hash) }.map { |session|
session.key?(:game_code) ? session : session.merge(game_code: session[:game]) })
so only Hash session rows are kept and malformed entries are removed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 634bdf4f-614f-4e35-b04a-bc9e4fc84715

📥 Commits

Reviewing files that changed from the base of the PR and between a0ba95a and 8d3de85.

📒 Files selected for processing (1)
  • scripts/version.lic

Comment thread scripts/version.lic Outdated
Comment thread scripts/version.lic
Comment thread scripts/version.lic Outdated
Comment thread scripts/version.lic Outdated
@mrhoribu
Copy link
Copy Markdown
Contributor

mrhoribu commented Mar 9, 2026

@coderabbitai summary

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 9, 2026

✅ Actions performed

Summary regeneration triggered.

## What was fixed:
 - Dropped malformed non-Hash session rows during snapshot normalization.
 - Made stale-count fallback reachable by switching from key?(:stale) to nil-aware value checks.
 - Added symbol/string key tolerance in:
 -- active session counting
 -- stale fallback logic
 -- detail rendering rows
 - Kept active-session semantics and relabeled output from Total ruby sessions to Active ruby sessions for consistency.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
scripts/version.lic (1)

238-239: Consider adding string-key fallback for consistency.

Line 239 accesses session_state[:sessions] without a string-key fallback, unlike potential_stale_sessions_count (line 208) and active_session_count (line 276) which use session_state[:sessions] || session_state['sessions'].

While this works correctly because normalize_session_snapshot always outputs symbol keys, adding the fallback would maintain defensive consistency across all methods.

♻️ Suggested fix
   def self.duplicate_live_session_count(session_state)
-    live_sessions = Array(session_state[:sessions]).select { |s| session_os_seen?(s) == true }
+    live_sessions = Array(session_state[:sessions] || session_state['sessions']).select { |s| session_os_seen?(s) == true }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/version.lic` around lines 238 - 239, The method
duplicate_live_session_count currently accesses session_state[:sessions] without
a string-key fallback; change the retrieval to mirror the other helpers (e.g.,
use session_state[:sessions] || session_state['sessions'] wrapped in Array(...))
so it defensively handles both symbol and string keys in
duplicate_live_session_count, leaving the rest of the logic (including the
session_os_seen? predicate) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@scripts/version.lic`:
- Around line 238-239: The method duplicate_live_session_count currently
accesses session_state[:sessions] without a string-key fallback; change the
retrieval to mirror the other helpers (e.g., use session_state[:sessions] ||
session_state['sessions'] wrapped in Array(...)) so it defensively handles both
symbol and string keys in duplicate_live_session_count, leaving the rest of the
logic (including the session_os_seen? predicate) unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bef05c7b-f142-4b34-95d3-47050f6f5b6c

📥 Commits

Reviewing files that changed from the base of the PR and between 8d3de85 and c31f326.

📒 Files selected for processing (1)
  • scripts/version.lic

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
scripts/version.lic (1)

163-178: ⚠️ Potential issue | 🟠 Major

Preserve stale=nil when the snapshot omits the top-level stale count.

This still normalizes “missing” to 0, so potential_stale_sessions_count never reaches the row-level marker/heartbeat fallback. On cores that only populate per-session stale data, the new summary will always report 0 stale sessions. This hunk is also the RuboCop failure from CI.

Suggested fix
   def self.normalize_session_snapshot(snapshot)
     base = stub_session_snapshot
     return base unless snapshot.is_a?(Hash)
+    stale = snapshot[:stale]
+    stale = snapshot['stale'] if stale.nil?
 
     base.merge(
       supported: true,
       source: snapshot[:source] || snapshot['source'] || 'SessionSettings',
       total: (snapshot[:total] || snapshot['total']).to_i,
       idle_over_30m: (snapshot[:idle_over_30m] || snapshot['idle_over_30m']).to_i,
-      stale: (snapshot[:stale] || snapshot['stale']).to_i,
+      stale: stale.nil? ? nil : stale.to_i,
       running: (snapshot[:running] || snapshot['running']).to_i,
       sleeping: (snapshot[:sleeping] || snapshot['sleeping']).to_i,
       hidden: (snapshot[:hidden] || snapshot['hidden']).to_i,
       sessions: Array(snapshot[:sessions] || snapshot['sessions'])
-                  .select { |session| session.is_a?(Hash) }
-                  .map { |session|
+        .select { |session| session.is_a?(Hash) }
+        .map { |session|
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/version.lic` around lines 163 - 178, The normalize_session_snapshot
method currently coerces missing top-level counts (e.g., stale) to 0 by calling
.to_i on (snapshot[:stale] || snapshot['stale']), which prevents preserving nil
when the snapshot omits that key; update the merge logic for stale (and
similarly for any other counts that must distinguish "missing" vs zero) to
detect whether the snapshot actually provided the key (using
snapshot.key?(:stale) or snapshot.key?('stale') or fetch with nil) and only call
.to_i when a value exists, otherwise set stale to nil so
potential_stale_sessions_count can fall back to per-session markers. Target
normalize_session_snapshot and the stale/total/etc. merge entries in your
change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/version.lic`:
- Around line 241-250: The grouping logic treats blank/nil PIDs and mixed
String/Integer PID types as distinct and also drops rows that only have a name;
update the pipeline around keys/grouped to normalize session_pid_value and
session_name_value before grouping: coerce PIDs to a single canonical type
(e.g., integer when numeric, otherwise nil) and normalize blank or nil PIDs to a
consistent sentinel (or nil) so that identical processes compare equal, and
ensure you don't drop rows solely because game is empty if name should still
count; apply the same normalization for the other occurrences noted (the blocks
around lines producing keys at 255-257 and 265-267) so grouped.count uses
normalized pid uniqueness checks rather than raw mixed types/blank values.
- Around line 305-307: The ALL/full/details branch only globs top-level files so
scripts/custom/*.lic are skipped; update the scripts_to_check assignment (where
Script.current.vars[1] is checked and show_report is set) to use a recursive
glob that includes subdirectories (e.g. a "**" pattern under SCRIPT_DIR) and
keep the existing /\.lic$/i filter so custom/*.lic files are included in the
all/full/details report.
- Around line 482-489: The marker fallback incorrectly synthesizes "active" for
rows where state == "exited"; update the marker assignment around session_info
(where stale and state are read) to avoid producing "active" for exited sessions
— e.g., when computing marker (currently using session_info[:marker] || ... ||
(stale ? 'stale' : 'active')), change the fallback logic to check state first
and return 'exited' (or a non-active marker) when state == 'exited', otherwise
return 'stale' if stale, otherwise 'active'; ensure the updated logic still
respects explicit session_info[:marker]/session_info['marker'] and that
session_report["Session #{pid}"] uses the new marker.

---

Duplicate comments:
In `@scripts/version.lic`:
- Around line 163-178: The normalize_session_snapshot method currently coerces
missing top-level counts (e.g., stale) to 0 by calling .to_i on
(snapshot[:stale] || snapshot['stale']), which prevents preserving nil when the
snapshot omits that key; update the merge logic for stale (and similarly for any
other counts that must distinguish "missing" vs zero) to detect whether the
snapshot actually provided the key (using snapshot.key?(:stale) or
snapshot.key?('stale') or fetch with nil) and only call .to_i when a value
exists, otherwise set stale to nil so potential_stale_sessions_count can fall
back to per-session markers. Target normalize_session_snapshot and the
stale/total/etc. merge entries in your change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a357f7ed-12f9-40bf-bc85-25c19bd4c0c2

📥 Commits

Reviewing files that changed from the base of the PR and between c31f326 and 3cc51a7.

📒 Files selected for processing (1)
  • scripts/version.lic

Comment thread scripts/version.lic
Comment thread scripts/version.lic Outdated
Comment thread scripts/version.lic
Changes made:

Preserved missing stale as nil in snapshot normalization (instead of forcing 0): version.lic (line 163)
Normalized duplicate-session PID handling ("123" and 123 now canonicalize the same; blank/non-numeric -> nil), and made session array read tolerant of symbol/string keys: version.lic (line 241), version.lic (line 268)
Switched all/full/details scan to recursive glob so nested custom/*.lic are included: version.lic (line 317)
Fixed detail marker fallback so exited sessions are not synthesized as active: version.lic (line 500)

NB: Anything about whether rows should exist / be filtered needs to be addressed at source: Redirect to core.  This script is a reporting script only and does not maintenance / lifecycle ativities on the data itself.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
scripts/version.lic (1)

258-260: session_name_value should fall back to :name for consistency.

The detail output at Line 496 falls back to :name/'name' when :session_name is absent, but this helper does not. Sessions from older core versions that only expose :name would return empty strings here, causing duplicate detection to skip them entirely (via the reject on Line 250).

Proposed fix
   def self.session_name_value(session)
-    (session[:session_name] || session['session_name']).to_s.strip.downcase
+    value = session[:session_name] || session['session_name'] ||
+            session[:name] || session['name']
+    value.to_s.strip.downcase
   end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/version.lic` around lines 258 - 260, The session_name_value helper
currently only checks session[:session_name] and session['session_name']; update
it to also fall back to session[:name] and session['name'] so it returns the
older key when session_name is absent. Modify the logic in def
self.session_name_value(session) (used by the duplicate detection path) to check
keys in order [:session_name, 'session_name', :name, 'name'], then
to_s.strip.downcase before returning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@scripts/version.lic`:
- Around line 258-260: The session_name_value helper currently only checks
session[:session_name] and session['session_name']; update it to also fall back
to session[:name] and session['name'] so it returns the older key when
session_name is absent. Modify the logic in def self.session_name_value(session)
(used by the duplicate detection path) to check keys in order [:session_name,
'session_name', :name, 'name'], then to_s.strip.downcase before returning.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ad5669e8-e238-412e-aa60-69c3d405beed

📥 Commits

Reviewing files that changed from the base of the PR and between 3cc51a7 and 4579856.

📒 Files selected for processing (1)
  • scripts/version.lic

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
scripts/version.lic (1)

244-250: ⚠️ Potential issue | 🟠 Major

Normalize duplicate-session identities before grouping.

Line 250 still keeps rows with pid=nil, and Line 259 ignores rows that only expose name. That means the reboot recommendation can still false-positive on incomplete rows and miss real duplicates from older snapshot shapes.

Possible fix
     keys = live_sessions.map { |s|
       [
         session_name_value(s),
         session_game_code_value(s),
         session_pid_value(s)
       ]
-    }.reject { |name, game, _pid| name.empty? || game.empty? }
+    }.reject { |name, game, pid| name.empty? || game.empty? || pid.nil? }
@@
   def self.session_name_value(session)
-    (session[:session_name] || session['session_name']).to_s.strip.downcase
+    value = session[:session_name] || session['session_name'] ||
+            session[:name] || session['name']
+    value.to_s.strip.downcase
   end

Also applies to: 258-259

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/version.lic` around lines 244 - 250, Normalize session identity
triples before filtering and grouping: when building keys from live_sessions
(the live_sessions.map that produces [session_name_value(s),
session_game_code_value(s), session_pid_value(s)]), normalize each element
(strip and downcase name and game, convert pid to a string or empty string when
nil) and then reject only fully-empty rows (e.g., where name.empty? &&
game.empty? && pid.to_s.empty?), so rows that only expose name (or name+game)
are preserved and pid=nil is normalized rather than left as nil.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/version.lic`:
- Around line 179-181: The chained call starting with Array(snapshot[:sessions]
|| snapshot['sessions']) should have each method on its own indented line to
satisfy RuboCop: place .select on the next line aligned under the opening of the
receiver expression and .map on the following line aligned with .select, e.g.
start with "sessions: Array(snapshot[:sessions] || snapshot['sessions'])" then
newline with ".select { |session| session.is_a?(Hash) }" and another newline
with ".map { |session| ... }", keeping the block braces/contents unchanged so
the Array(...).select.map chain is properly indented.

---

Duplicate comments:
In `@scripts/version.lic`:
- Around line 244-250: Normalize session identity triples before filtering and
grouping: when building keys from live_sessions (the live_sessions.map that
produces [session_name_value(s), session_game_code_value(s),
session_pid_value(s)]), normalize each element (strip and downcase name and
game, convert pid to a string or empty string when nil) and then reject only
fully-empty rows (e.g., where name.empty? && game.empty? && pid.to_s.empty?), so
rows that only expose name (or name+game) are preserved and pid=nil is
normalized rather than left as nil.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a046e98c-74da-4ab4-b85c-78614e551bc0

📥 Commits

Reviewing files that changed from the base of the PR and between 4579856 and 1a3ff07.

📒 Files selected for processing (1)
  • scripts/version.lic

Comment thread scripts/version.lic Outdated
OSXLich-Doug and others added 4 commits March 11, 2026 13:43
Add a method to check if session summary output is enabled based on feature flags.

If feature flags not available, or if feature flag for session summary is set to false, behavior is exactly the same as version 1.2.2

If feature flags is available AND the feature flag for session summary is set to true, new reporting behavior on sessions is avaialble.
@mrhoribu
Copy link
Copy Markdown
Contributor

@coderabbitai hide all previous comments and review again

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 31, 2026

@mrhoribu Sure! I'll resolve all my previous comments and perform a fresh full review of the PR now.

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
scripts/version.lic (1)

256-266: ⚠️ Potential issue | 🟡 Minor

Add nil PID check to avoid false duplicate positives.

Sessions with unknown PIDs (when session_pid_value returns nil) are currently counted as duplicates if paired with a session having a known PID. Since the function aims to detect the same character+game running on multiple resident processes, sessions without valid PIDs should be excluded from duplicate detection:

    }.reject { |name, game, _pid| name.empty? || game.empty? }
+   }.reject { |name, game, pid| name.empty? || game.empty? || pid.nil? }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/version.lic` around lines 256 - 266, The duplicate-detection
currently treats sessions with nil PIDs as valid and can produce false
duplicates; when building keys (the keys array from live_sessions) filter out
entries where session_pid_value(s) is nil (e.g., extend the existing reject to
also exclude rows with nil pid) before grouping, so grouped (and the subsequent
count on grouped) only considers entries with a valid pid; update the keys
construction or add a .compact/.reject { |_name, _game, pid| pid.nil? } prior to
grouped to exclude nil PIDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/version.lic`:
- Around line 270-272: session_name_value currently only checks
session[:session_name] and session['session_name'], causing sessions that only
provide :name/'name' to return empty and be excluded from duplicate counting;
update session_name_value to follow the same multi-level fallback as
session_game_code_value by also checking session[:name] and session['name'],
then convert the selected value with to_s.strip.downcase so the detail output
and duplicate counting use the same normalized name.

---

Duplicate comments:
In `@scripts/version.lic`:
- Around line 256-266: The duplicate-detection currently treats sessions with
nil PIDs as valid and can produce false duplicates; when building keys (the keys
array from live_sessions) filter out entries where session_pid_value(s) is nil
(e.g., extend the existing reject to also exclude rows with nil pid) before
grouping, so grouped (and the subsequent count on grouped) only considers
entries with a valid pid; update the keys construction or add a .compact/.reject
{ |_name, _game, pid| pid.nil? } prior to grouped to exclude nil PIDs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 503fd403-9657-4901-bc80-300209205c72

📥 Commits

Reviewing files that changed from the base of the PR and between 6621c2a and e2e9e9a.

📒 Files selected for processing (1)
  • scripts/version.lic

Comment thread scripts/version.lic
Comment on lines +270 to +272
def self.session_name_value(session)
(session[:session_name] || session['session_name']).to_s.strip.downcase
end
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.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if core SessionsSettings always provides :session_name or if :name fallback is needed
ast-grep --pattern 'session_name'
rg -n ':session_name|:name' --type ruby -C 2 | head -50

Repository: elanthia-online/scripts

Length of output: 3385


🏁 Script executed:

# First, locate and read the relevant parts of scripts/version.lic
wc -l scripts/version.lic

Repository: elanthia-online/scripts

Length of output: 90


🏁 Script executed:

# Read lines 260-280 to see session_name_value method
sed -n '260,280p' scripts/version.lic

Repository: elanthia-online/scripts

Length of output: 893


🏁 Script executed:

# Read lines 495-515 to see detail output reference at line 503
sed -n '495,515p' scripts/version.lic

Repository: elanthia-online/scripts

Length of output: 1287


🏁 Script executed:

# Search for where sessions are created and what keys they contain
rg -n 'session\[' scripts/version.lic | head -20

Repository: elanthia-online/scripts

Length of output: 520


🏁 Script executed:

# Look for the reject clause mentioned in the scratchpad
rg -n '\.reject' scripts/version.lic -A 2 -B 2 | head -30

Repository: elanthia-online/scripts

Length of output: 251


🏁 Script executed:

# Check what method contains the reject clause at line 262
sed -n '240,270p' scripts/version.lic

Repository: elanthia-online/scripts

Length of output: 1136


🏁 Script executed:

# Verify all uses of session_name_value
rg -n 'session_name_value' scripts/version.lic

Repository: elanthia-online/scripts

Length of output: 144


🏁 Script executed:

# Check if there are any other extractors to understand the pattern
rg -n 'def self\.session_' scripts/version.lic

Repository: elanthia-online/scripts

Length of output: 404


Align session_name_value fallback logic with detail output.

session_name_value (line 270) only checks :session_name/'session_name', but the detail output (line 505) also falls back to :name/'name'. If a session only provides :name, this method returns an empty string, which causes the session to be excluded from duplicate counting (line 262 rejects empty names) while still appearing in the detail output. Other extractors like session_game_code_value follow the multi-level fallback pattern.

Add the missing fallback:

Suggested change
 def self.session_name_value(session)
-  (session[:session_name] || session['session_name']).to_s.strip.downcase
+  value = session[:session_name] || session['session_name'] ||
+          session[:name] || session['name']
+  value.to_s.strip.downcase
 end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/version.lic` around lines 270 - 272, session_name_value currently
only checks session[:session_name] and session['session_name'], causing sessions
that only provide :name/'name' to return empty and be excluded from duplicate
counting; update session_name_value to follow the same multi-level fallback as
session_game_code_value by also checking session[:name] and session['name'],
then convert the selected value with to_s.strip.downcase so the detail output
and duplicate counting use the same normalized name.

@mrhoribu mrhoribu merged commit 0de1be1 into master Apr 3, 2026
4 checks passed
@mrhoribu mrhoribu deleted the feat/version_track_stale_sessions branch April 3, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants