Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions src/chat/agent-loop.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,14 +282,23 @@ class AgentLoop {
// ctx.registerBgJob into bg-shell so each freshly started jobId is
// recorded here; we clear entries on end so memory is bounded.
run._sessionStartedBgJobs = new Set();
// Bg jobs the model has actively inspected via read_terminal in this run.
// Populated by tool-executor; consumed by the BG_WAIT_SKIPPED_MODEL_DONE
// guard so the turn refuses to end while a monitored job is still alive
// (e.g. user asked the model to watch a training that was started in an
// earlier turn — `_sessionStartedBgJobs` alone wouldn't catch this).
run._monitoredBgJobs = new Set();
// Only accept job-end events for jobs that belong to THIS session.
// Require an exact sessionId match so events without a sessionId (orphaned
// terminals not registered via addActiveBgJob) are also dropped — this
// ensures complete isolation between concurrent sessions.
const _bgJobEndHandler = (payload) => {
if (!payload || payload.sessionId !== sid) return;
run._pendingBgJobEvents.push(payload);
if (payload.jobId) run._sessionStartedBgJobs.delete(payload.jobId);
if (payload.jobId) {
run._sessionStartedBgJobs.delete(payload.jobId);
run._monitoredBgJobs.delete(payload.jobId);
}
};
onBgJobEnded(_bgJobEndHandler);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

新增的 run._monitoredBgJobs 集合用于跟踪模型在当前运行中主动检查的后台作业,但未对其进行初始化检查。在使用前应确保其存在且为 Set 类型,以避免潜在的空指针异常。此外,建议在注释中详细说明该集合的用途和生命周期,以提高可维护性。

Expand Down Expand Up @@ -642,10 +651,21 @@ class AgentLoop {
const _activeJobsNow = myBgJobs();
const _hasOwnRunningJob = [...run._sessionStartedBgJobs]
.some(j => _activeJobsNow.has(j));
if (!_hasPendingEvents && !_hasOwnRunningJob) {
// Watchdog scenario: model is actively monitoring a
// pre-existing bg job (e.g. training started in an
// earlier turn). Detected when the model has called
// read_terminal(terminal: "deepseek-job-X") in this
// run. If any such monitored job is still active,
// refuse to end the turn — the model committed to
// watching it through completion.
const _monitored = (run._monitoredBgJobs instanceof Set) ? run._monitoredBgJobs : null;
const _hasMonitoredRunningJob = !!_monitored && [..._monitored]
.some(j => _activeJobsNow.has(j));
if (!_hasPendingEvents && !_hasOwnRunningJob && !_hasMonitoredRunningJob) {
Logger.info('BG_WAIT_SKIPPED_MODEL_DONE', {
jobs: [..._activeJobsNow],
sessionStartedJobs: [...run._sessionStartedBgJobs],
monitoredJobs: _monitored ? [..._monitored] : [],
});
break;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

在检查 run._monitoredBgJobs 时,虽然进行了类型检查,但建议在使用前确保该集合已被正确初始化。此外,建议在条件判断中添加日志记录,以便在出现问题时能够追踪到具体的作业状态。

Expand Down
99 changes: 59 additions & 40 deletions src/chat/session-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
const vscode = require('vscode');
const { randomBytes } = require('crypto');
const { t, tf } = require('../utils/i18n');
const { Logger } = require('../logger');

// ─── Orphan tool_calls sanitizer ───────────────────────────────────────────
// Removes ANY incomplete assistant{tool_calls} group from a message array,
Expand Down Expand Up @@ -102,6 +103,47 @@ class SessionStore {
this._getBusy = getBusy; // (id) => bool (is that session's run busy?)
this._onDeleteRun = onDeleteRun; // (id) => void (abort + remove from _runs)
this.sessionId = null; // currently displayed session id (null = empty view)

// Issue #169: archive semantics changed from "soft-hide + export" to
// "pure export". Sessions that were previously hidden by the old
// archive action are stuck invisible in globalState. Run a one-shot
// idempotent migration that flips every `archived: true` back to
// `false` so those records reappear in the sidebar after upgrade.
// Guarded by a globalState boolean so we only do this once per user.
// Fire-and-forget: the migration runs asynchronously and triggers
// postList() itself once it finishes, so the sidebar refreshes as
// soon as the migrated data is persisted (not necessarily on the
// very next tick).
this._migrateArchivedFlagIfNeeded();
}

/**
* One-time migration for issue #169. Idempotent: subsequent runs no-op
* because the `archiveSemanticsV2Migrated` flag is set on first success.
* Errors are swallowed (logged via Logger) — failure here must not
* block extension activation; the next launch will retry automatically
* because the flag was never written.
*/
async _migrateArchivedFlagIfNeeded() {
try {
if (this._gs.get('deepseekAgent.archiveSemanticsV2Migrated', false)) return;
const list = this.all();
let touched = false;
for (const s of list) {
if (s.archived) { s.archived = false; touched = true; }
}
if (touched) await this.set(list);
await this._gs.update('deepseekAgent.archiveSemanticsV2Migrated', true);
if (touched) this.postList();
} catch (err) {
// Non-fatal: next launch will retry. Route through Logger so the
// diagnostic respects deepseekAgent.enableDebugLog and lands in
// the "Deep Copilot Debug" output channel/log file.
Logger.info('ARCHIVE_V2_MIGRATION_FAILED', {
message: (err && err.message) || String(err),
stack: (err && err.stack) || undefined,
});
}
Comment on lines +138 to +146
Comment on lines +138 to +146
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

在捕获异常时,建议对错误进行更详细的处理,尤其是在日志记录时。虽然使用 Logger 记录错误信息是一个好的改进,但在某些情况下,可能需要对错误进行更细致的分类或处理,以便后续的调试和维护。此外,确保 Logger 的实现是线程安全的,以避免潜在的竞态条件。

// ─── Raw storage ────────────────────────────────────────────────────────
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

_migrateArchivedFlagIfNeeded 方法中,虽然错误被捕获并记录,但没有对错误进行进一步处理。建议在捕获错误后,考虑是否需要进行重试或记录更多上下文信息,以便后续排查。同时,建议确保 this._gs 的获取和使用是安全的,避免潜在的空指针异常。

Expand Down Expand Up @@ -361,61 +403,38 @@ class SessionStore {
}

/**
* "Archive" a session — issue #165.
* "Archive" a session — issues #165, #169.
*
* Original behaviour (pre-#165) was a soft-hide toggle: flip `archived`,
* disappear from the sidebar list, data stays in globalState. Users
* reported that this didn't match the menu label's promise: nothing was
* actually *archived* anywhere they could see, find, or grep.
* Behaviour evolution:
* pre-#165 : soft-hide toggle (flip `archived`, disappear from list).
* #165/#166: render to Markdown + soft-hide (export AND hide).
* #169 : pure export. Render to Markdown, leave session state
* completely untouched — it stays in the sidebar, stays
* the active session, can be archived again to produce
* another snapshot.
*
* New behaviour: render the session to Markdown and write it under
* <workspace>/.deep-copilot/archives/yyyyMMdd-HHmmss-<title>.md
* Then perform the original soft-hide so the session leaves the sidebar.
* Contract:
* - exportSessionToMarkdown returns absolute path → toast + done.
* - returns null (user cancelled folder picker / save dialog) → silent.
* - throws → toast error; no state change.
*
* The "un-archive" gesture (clicking again on an already-archived item)
* is still supported via the boolean toggle — it simply un-hides the
* record without touching the Markdown file on disk.
* Session state (`archived` flag, current sessionId, list ordering) is
* never mutated by this method.
*/
async archive(id) {
const list = this.all();
const s = list.find(x => x.id === id);
const s = this.all().find(x => x.id === id);
if (!s) return;

// Un-archive: just flip back to visible. No file I/O needed.
if (s.archived) {
s.archived = false;
await this.set(list);
this.postList();
return;
}

// Archive: render to Markdown FIRST. Only hide the session from the
// sidebar if we actually produced a file on disk — otherwise setting
// `archived = true` on export failure (or on a user-cancelled save
// dialog) would leave the session invisible in the list while nothing
// was actually archived. Contract:
// - savedPath is a string → write succeeded, hide the session.
// - savedPath is null → user cancelled the save dialog; keep
// session visible, do nothing more.
// - throw caught below → fs error; surface message, keep visible.
let savedPath = null;
try {
const { exportSessionToMarkdown } = require('./archive-export');
savedPath = await exportSessionToMarkdown(s);
} catch (err) {
const msg = (err && err.message) || String(err);
vscode.window.showErrorMessage(tf('archiveFailed', { msg }));
return; // do not toggle archived — keep state consistent
}
if (!savedPath) return; // user cancelled; keep state consistent

s.archived = true;
if (this.sessionId === id) {
this.sessionId = null;
this._post({ type: 'sessionLoaded', id: null, messages: [] });
return;
}
await this.set(list);
this.postList();
if (!savedPath) return; // user cancelled

this._notifyArchived(savedPath);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

archive 方法中,虽然进行了错误处理,但在调用 exportSessionToMarkdown 时,如果发生错误,应该确保 s.archived 状态不被改变。建议在 try-catch 块中明确处理状态的变化,确保在任何情况下都能保持状态一致性。此外,建议对 this.all() 的调用进行空值检查,以防止潜在的空指针异常。

Expand Down
14 changes: 14 additions & 0 deletions src/chat/tool-executor.js
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,20 @@ class ToolExecutor {
if (!(run._sessionStartedBgJobs instanceof Set)) run._sessionStartedBgJobs = new Set();
run._sessionStartedBgJobs.add(jobId);
};
// Watchdog/monitor detection: when the model inspects an existing
// deepseek-job-* terminal via read_terminal in this run, treat that
// as an active monitoring commitment. agent-loop's
// BG_WAIT_SKIPPED_MODEL_DONE guard refuses to end the turn while a
// monitored job is still alive — preventing the model from
// "promising to keep watching" and then immediately stopping when
// the underlying job was spawned in a previous turn.
if (name === 'read_terminal' && run && args && typeof args.terminal === 'string') {
const _jobId = args.terminal;
if (/^deepseek-job-/.test(_jobId)) {
if (!(run._monitoredBgJobs instanceof Set)) run._monitoredBgJobs = new Set();
run._monitoredBgJobs.add(_jobId);
}
}
Comment on lines +535 to +548
if (tcId) {
ctx.onStreamDelta = (delta) => {
if (!delta) return;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

此段代码引入了对终端的监控机制,但在处理 args.terminal 时缺乏有效的输入验证,可能导致命令注入风险。建议在使用 args.terminal 之前,增加对其内容的严格校验,确保其符合预期格式。此外,run._monitoredBgJobs 的初始化逻辑应考虑并发访问的安全性,避免潜在的竞态条件。

Expand Down
Loading