fix(session): resolve Greptile review findings

- status honesty: TraceConfig::has_sink() (feeds status "tracing") returns false
  once the writer has failed, so status no longer reports active tracing while
  emits are being dropped.
- gc data race: re-check recent activity (snapshot/trace mtime, created-recently)
  AFTER acquiring the session write lock, so a trace segment written between the
  pre-check and remove is not deleted.
- session end honors the resolved session: `--session X session end` (no id) now
  ends X, not whatever the current_session pointer names.
- degrade precision: manifest/pointer reads degrade only symlinked/corrupt files
  (the structural attack) but let genuine I/O/permission errors surface, instead
  of masking every failure as "absent".
- strict batch override: a batch item overriding to a valid no-trace/ended
  session no longer fails INVALID_ARGS under a --trace-strict parent
  (clone_with_session_segment(None) yields a sinkless config, not a strict error).

Kept: BatchSessionArgs deny_unknown_fields (session batch args are new — no
legacy generators to break — and it catches the no_trce typo the prior review
required).

Guards added: failed-writer-reports-no-sink, strict-parent-allows-no-trace-
batch-override.

crates/core/src/context_tests.rs

@@ -321,3 +321,25 @@ fn batch_item_session_override_uses_its_own_segment_dir() {
         .trace_dir();
     assert!(child_trace.is_dir());
 }
+
+#[test]
+fn strict_parent_allows_no_trace_batch_override() {
+    let _guard = crate::refs_test_support::HomeGuard::new();
+    let traced = start_session(StartSessionOptions {
+        name: None,
+        trace: SessionTraceMode::On,
+        force: true,
+    })
+    .unwrap();
+    let untraced = start_session(StartSessionOptions {
+        name: None,
+        trace: SessionTraceMode::Off,
+        force: true,
+    })
+    .unwrap();
+    let parent = CommandContext::new(Some(traced.id.clone()), None, true).unwrap();
+    let child = parent
+        .for_batch_item(Some(untraced.id.clone()))
+        .expect("a no-trace session override must not fail under a strict parent");
+    assert_eq!(child.session_id(), Some(untraced.id.as_str()));
+}

crates/core/src/session/gc.rs

@@ -1,6 +1,6 @@
 use super::{
-    TRACE_LIVENESS_WINDOW, list_sessions, now_millis, read_current_session_pointer, read_manifest,
-    session_dir,
+    SessionManifest, TRACE_LIVENESS_WINDOW, list_sessions, now_millis,
+    read_current_session_pointer, read_manifest,
 };
 use crate::{
     context::validate_session_id,
@@ -31,22 +31,23 @@ pub fn pointer_references_live_session() -> Result<bool, AppError> {
 pub fn is_live(session_id: &str) -> Result<bool, AppError> {
     validate_session_id(session_id)?;
     let store = RefStore::for_session(Some(session_id))?;
-    let base = store.base_dir();
-    if lock_holder_is_live(&base.join("refstore.lock")) {
+    if lock_holder_is_live(&store.base_dir().join("refstore.lock")) {
         return Ok(true);
     }
-    if path_recently_modified(&base.join("snapshots"))
+    let manifest = read_manifest(session_id)?;
+    Ok(has_recent_activity(&store, manifest.as_ref()))
+}
+
+fn has_recent_activity(store: &RefStore, manifest: Option<&SessionManifest>) -> bool {
+    if path_recently_modified(&store.base_dir().join("snapshots"))
         || trace_dir_recently_written(&store.trace_dir())
     {
-        return Ok(true);
+        return true;
     }
-    if let Some(manifest) = read_manifest(session_id)? {
+    manifest.is_some_and(|manifest| {
         let age = Duration::from_millis(now_millis().saturating_sub(manifest.created_at));
-        if manifest.ended_at.is_none() && age < TRACE_LIVENESS_WINDOW {
-            return Ok(true);
-        }
-    }
-    Ok(false)
+        manifest.ended_at.is_none() && age < TRACE_LIVENESS_WINDOW
+    })
 }
 
 pub fn gc(options: GcOptions) -> Result<GcReport, AppError> {
@@ -71,11 +72,16 @@ pub fn gc(options: GcOptions) -> Result<GcReport, AppError> {
         } else if manifest.ended_at.is_none() {
             continue;
         }
-        let dir = session_dir(&manifest.id)?;
+        let store = RefStore::for_session(Some(&manifest.id))?;
+        let dir = store.base_dir().to_path_buf();
         let lock = match RefStoreLock::acquire(&dir.join("refstore.lock")) {
             Ok(lock) => lock,
             Err(_) => continue,
         };
+        if has_recent_activity(&store, Some(&manifest)) {
+            drop(lock);
+            continue;
+        }
         let did_remove = remove_session_dir(&dir)?;
         drop(lock);
         if did_remove {

crates/core/src/session/mod.rs

@@ -71,22 +71,17 @@ pub fn read_current_session_pointer() -> Result<Option<String>, AppError> {
     let mut file = match open_session_file(&path) {
         Ok(file) => file,
         Err(err) if err.kind() == ErrorKind::NotFound => return Ok(None),
-        Err(err) => {
+        Err(err) if is_symlinked(&path) => {
             tracing::warn!(
-                "ignoring unreadable session pointer {}: {err}",
+                "ignoring symlinked session pointer {}: {err}",
                 path.display()
             );
             return Ok(None);
         }
+        Err(err) => return Err(err.into()),
     };
     let mut id = String::new();
-    if let Err(err) = file.read_to_string(&mut id) {
-        tracing::warn!(
-            "ignoring unreadable session pointer {}: {err}",
-            path.display()
-        );
-        return Ok(None);
-    }
+    file.read_to_string(&mut id)?;
     let id = id.trim().to_string();
     if id.is_empty() || validate_session_id(&id).is_err() {
         return Ok(None);
@@ -112,18 +107,23 @@ pub fn read_manifest(session_id: &str) -> Result<Option<SessionManifest>, AppErr
     let mut file = match open_session_file(&path) {
         Ok(file) => file,
         Err(err) if err.kind() == ErrorKind::NotFound => return Ok(None),
-        Err(err) => return Ok(ignore_unreadable_manifest(&path, &err)),
+        Err(err) if is_symlinked(&path) => return Ok(ignore_unreadable_manifest(&path, &err)),
+        Err(err) => return Err(err.into()),
     };
     let mut json = String::new();
-    if let Err(err) = file.read_to_string(&mut json) {
-        return Ok(ignore_unreadable_manifest(&path, &err));
-    }
+    file.read_to_string(&mut json)?;
     match serde_json::from_str(&json) {
         Ok(manifest) => Ok(Some(manifest)),
         Err(err) => Ok(ignore_unreadable_manifest(&path, &err)),
     }
 }
 
+fn is_symlinked(path: &Path) -> bool {
+    std::fs::symlink_metadata(path)
+        .map(|meta| meta.file_type().is_symlink())
+        .unwrap_or(false)
+}
+
 fn ignore_unreadable_manifest<E: std::fmt::Display>(
     path: &Path,
     err: &E,

crates/core/src/trace.rs

@@ -144,7 +144,13 @@ impl TraceConfig {
     }
 
     pub(crate) fn has_sink(&self) -> bool {
-        !matches!(self.state.pending, TracePending::None)
+        if matches!(self.state.pending, TracePending::None) {
+            return false;
+        }
+        match self.state.writer.lock() {
+            Ok(writer) => !matches!(*writer, WriterState::Failed),
+            Err(_) => true,
+        }
     }
 
     pub(crate) fn pending_file_path(&self) -> Option<&Path> {
@@ -161,7 +167,13 @@ impl TraceConfig {
         if self.pending_file_path().is_some() {
             return Ok(self.clone());
         }
-        Self::build(None, session_segment_dir, self.strict)
+        match session_segment_dir {
+            Some(dir) => Self::build(None, Some(dir), self.strict),
+            None => Ok(Self {
+                strict: self.strict,
+                state: Arc::new(TraceState::default()),
+            }),
+        }
     }
 }
 

crates/core/src/trace_tests.rs

@@ -273,3 +273,15 @@ fn segment_open_rejects_symlinked_trace_dir() {
     let _ = fs::remove_file(&base);
     let _ = fs::remove_dir_all(&real);
 }
+
+#[test]
+fn failed_writer_reports_no_sink() {
+    let missing = std::env::temp_dir()
+        .join("agent-desktop-nodir-hassink")
+        .join("trace.jsonl");
+    let config = TraceConfig::build(Some(missing), None, false).unwrap();
+    assert!(
+        !config.has_sink(),
+        "a trace writer whose open failed must not report an active sink"
+    );
+}

src/dispatch/mod.rs

@@ -372,7 +372,9 @@ pub(crate) fn dispatch(
                 no_trace: s.no_trace,
                 force: s.force,
             }),
-            SessionAction::End(e) => session::execute(session::SessionAction::End { id: e.id }),
+            SessionAction::End(e) => session::execute(session::SessionAction::End {
+                id: e.id.or_else(|| context.session_id().map(str::to_string)),
+            }),
             SessionAction::List => session::execute(session::SessionAction::List),
             SessionAction::Gc(g) => session::execute(session::SessionAction::Gc {
                 older_than_secs: g.older_than,