Skip to content

Commit 690a4fa

Browse files
nullmastermindclaude
nullmastermind
and
claude
committed
fix(acp): omp no longer runs on past an ask_user form
omp's model also `read`s xd://mcp__notepadai_ask_user to see the schema. That call was taken for ask_user, failed to parse, and the host sent "The form could not be shown", which started a new turn: the agent carried on instead of waiting for the answers. - A device call counts only when it is a write (has `content`). - While a form is pending, an unparseable ask_user call sends nothing. - History keeps ask_user's rawInput (other tools stay slim) so a restored session can rebuild the expired form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 2a0f3b5 commit 690a4fa

6 files changed

Lines changed: 67 additions & 2 deletions

File tree

‎src/AcpSessionModel.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
#include "AcpSessionModel.h"
2020

21+
#include "AskUserForm.h"
22+
2123
#include <QDateTime>
2224
#include <QDir>
2325
#include <QFile>
@@ -333,7 +335,10 @@ QJsonObject AcpSessionModel::toHistoryJson() const
333335
if (tc.durationMs >= 0) {
334336
to.insert(QStringLiteral("durationMs"), static_cast<double>(tc.durationMs));
335337
}
336-
if (!tc.rawInput.isEmpty()) {
338+
if (AskUserForm::isAskUserToolCall(tc.name, tc.title, tc.rawInput)) {
339+
// Small (validated: <= 30 questions) and needed to restore the form.
340+
to.insert(QStringLiteral("rawInput"), tc.rawInput);
341+
} else if (!tc.rawInput.isEmpty()) {
337342
QJsonObject slim;
338343
auto copyField = [&](const QLatin1String &key) {
339344
if (tc.rawInput.contains(key))

‎src/AskUserForm.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,11 @@ constexpr QLatin1String kDevicePrefix("xd://");
365365

366366
// omp mounts MCP tools as devices and calls them through its `write` tool:
367367
// rawInput = {path: "xd://mcp__notepadai_ask_user", content: "<args JSON>"}.
368+
// A `read` of the same path (the device's schema) has no content: not a call.
368369
bool isAskUserDevice(const QJsonObject &rawInput)
369370
{
371+
if (!rawInput.contains(QStringLiteral("content")))
372+
return false;
370373
const QString path = rawInput.value(QStringLiteral("path")).toString().trimmed();
371374
return path.startsWith(kDevicePrefix, Qt::CaseInsensitive) && nameMatches(path.mid(kDevicePrefix.size()));
372375
}

‎src/widgets/AcpSessionView.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2154,6 +2154,10 @@ void AcpSessionView::showAskUserToolCallForm(const AcpProtocol::AcpToolCall &tc)
21542154
QString error;
21552155
const auto spec = AskUserForm::parseAskUserInput(AskUserForm::askUserArguments(tc.rawInput, tc.rawOutput), &error);
21562156
if (!spec) {
2157+
// A form is already waiting on the user: a stray, unparseable call must
2158+
// not start another turn (the agent would run on without the answers).
2159+
if (m_pendingForm)
2160+
return;
21572161
// The agent believes the form is on screen and waits: tell it.
21582162
sendHostPrompt(QLatin1String(AskUserForm::kAnswersHeader)
21592163
+ QStringLiteral("\nThe form could not be shown: %1. Fix the input and call "

‎tests/CMakeLists.txt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,8 @@ target_sources(test_acp_session_model PRIVATE
409409
${CMAKE_SOURCE_DIR}/src/AcpProtocol.cpp
410410
${CMAKE_SOURCE_DIR}/src/AcpProtocol.h
411411
${CMAKE_SOURCE_DIR}/src/AcpSessionModel.cpp
412+
${CMAKE_SOURCE_DIR}/src/AskUserForm.cpp
413+
${CMAKE_SOURCE_DIR}/src/AskUserForm.h
412414
${CMAKE_SOURCE_DIR}/src/AcpSessionModel.h
413415
${CMAKE_SOURCE_DIR}/src/DataPaths.cpp
414416
${CMAKE_SOURCE_DIR}/src/DataPaths.h
@@ -425,6 +427,8 @@ target_sources(test_acp_transcript_truncation PRIVATE
425427
${CMAKE_SOURCE_DIR}/src/AcpProtocol.cpp
426428
${CMAKE_SOURCE_DIR}/src/AcpProtocol.h
427429
${CMAKE_SOURCE_DIR}/src/AcpSessionModel.cpp
430+
${CMAKE_SOURCE_DIR}/src/AskUserForm.cpp
431+
${CMAKE_SOURCE_DIR}/src/AskUserForm.h
428432
${CMAKE_SOURCE_DIR}/src/AcpSessionModel.h
429433
${CMAKE_SOURCE_DIR}/src/AcpTranscriptTruncation.cpp
430434
${CMAKE_SOURCE_DIR}/src/AcpTranscriptTruncation.h
@@ -446,6 +450,8 @@ target_sources(test_acp_session_model_persistence_integration PRIVATE
446450
${CMAKE_SOURCE_DIR}/src/AcpProtocol.cpp
447451
${CMAKE_SOURCE_DIR}/src/AcpProtocol.h
448452
${CMAKE_SOURCE_DIR}/src/AcpSessionModel.cpp
453+
${CMAKE_SOURCE_DIR}/src/AskUserForm.cpp
454+
${CMAKE_SOURCE_DIR}/src/AskUserForm.h
449455
${CMAKE_SOURCE_DIR}/src/AcpSessionModel.h
450456
${CMAKE_SOURCE_DIR}/src/DataPaths.cpp
451457
${CMAKE_SOURCE_DIR}/src/DataPaths.h
@@ -2231,6 +2237,8 @@ target_sources(test_acp_turn_folding PRIVATE
22312237
${CMAKE_SOURCE_DIR}/src/AcpProtocol.cpp
22322238
${CMAKE_SOURCE_DIR}/src/AcpProtocol.h
22332239
${CMAKE_SOURCE_DIR}/src/AcpSessionModel.cpp
2240+
${CMAKE_SOURCE_DIR}/src/AskUserForm.cpp
2241+
${CMAKE_SOURCE_DIR}/src/AskUserForm.h
22342242
${CMAKE_SOURCE_DIR}/src/AcpSessionModel.h
22352243
${CMAKE_SOURCE_DIR}/src/AcpTranscriptTruncation.cpp
22362244
${CMAKE_SOURCE_DIR}/src/AcpTranscriptTruncation.h
@@ -2253,6 +2261,8 @@ target_sources(test_acp_conversation_copy PRIVATE
22532261
${CMAKE_SOURCE_DIR}/src/AcpProtocol.cpp
22542262
${CMAKE_SOURCE_DIR}/src/AcpProtocol.h
22552263
${CMAKE_SOURCE_DIR}/src/AcpSessionModel.cpp
2264+
${CMAKE_SOURCE_DIR}/src/AskUserForm.cpp
2265+
${CMAKE_SOURCE_DIR}/src/AskUserForm.h
22562266
${CMAKE_SOURCE_DIR}/src/AcpSessionModel.h
22572267
${CMAKE_SOURCE_DIR}/src/DataPaths.cpp
22582268
${CMAKE_SOURCE_DIR}/src/DataPaths.h

‎tests/test_acp_session_model.cpp‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class TestAcpSessionModel : public QObject
3333
private slots:
3434
void initTestCase();
3535
void emptySessionDoesNotPersist();
36+
void askUserRawInputSurvivesHistory();
3637
void streamingConcatenation();
3738
void messageIdChange_startsNewBubble();
3839
void promptEndedClosesStreaming();
@@ -80,6 +81,44 @@ void TestAcpSessionModel::initTestCase()
8081
DataPaths::init(QDir::tempPath(), DataPaths::Source::Default);
8182
}
8283

84+
void TestAcpSessionModel::askUserRawInputSurvivesHistory()
85+
{
86+
QTemporaryDir tmp;
87+
QVERIFY(tmp.isValid());
88+
AcpSessionModel model(QStringLiteral("au1"), QStringLiteral("proj"), tmp.path());
89+
model.appendUserMessage(QStringLiteral("grill me"), {});
90+
AcpProtocol::AcpToolCall ask;
91+
ask.id = QStringLiteral("t-ask");
92+
ask.title = QStringLiteral("mcp__notepadai__ask_user");
93+
ask.status = QStringLiteral("completed");
94+
ask.rawInput = QJsonObject{{QStringLiteral("questions"), QJsonArray{QJsonObject{
95+
{QStringLiteral("id"), QStringLiteral("Q1")}, {QStringLiteral("title"), QStringLiteral("What?")},
96+
{QStringLiteral("type"), QStringLiteral("text")}}}}};
97+
model.onToolCallReceived(ask);
98+
AcpProtocol::AcpToolCall other;
99+
other.id = QStringLiteral("t-write");
100+
other.title = QStringLiteral("Write");
101+
other.status = QStringLiteral("completed");
102+
other.rawInput = QJsonObject{{QStringLiteral("path"), QStringLiteral("a.txt")},
103+
{QStringLiteral("content"), QStringLiteral("big body")}};
104+
model.onToolCallReceived(other);
105+
106+
const QJsonArray calls = model.toHistoryJson().value(QStringLiteral("toolCalls")).toArray();
107+
QJsonObject askJson;
108+
QJsonObject writeJson;
109+
for (const QJsonValue &v : calls) {
110+
const QJsonObject o = v.toObject();
111+
if (o.value(QStringLiteral("toolCallId")).toString() == QLatin1String("t-ask"))
112+
askJson = o;
113+
else
114+
writeJson = o;
115+
}
116+
// ask_user keeps its arguments (the form is rebuilt from them)...
117+
QCOMPARE(askJson.value(QStringLiteral("rawInput")).toObject(), ask.rawInput);
118+
// ...other tools stay slim.
119+
QVERIFY(!writeJson.value(QStringLiteral("rawInput")).toObject().contains(QStringLiteral("content")));
120+
}
121+
83122
void TestAcpSessionModel::emptySessionDoesNotPersist()
84123
{
85124
QTemporaryDir tmp;

‎tests/test_ask_user_form.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,9 @@ void TestAskUserForm::recognisesOmpDeviceWrite()
363363
QJsonObject otherDevice = raw;
364364
otherDevice.insert(QStringLiteral("path"), QStringLiteral("xd://mcp__other_ask_user"));
365365
QVERIFY(!isAskUserToolCall(QString(), intent, otherDevice));
366+
// Reading the device (its schema/help) is not a call: no `content`.
367+
const QJsonObject read{{QStringLiteral("path"), QStringLiteral("xd://mcp__notepadai_ask_user")}};
368+
QVERIFY(!isAskUserToolCall(QStringLiteral("read"), QStringLiteral("Read the ask_user device"), read));
366369
QJsonObject file = raw;
367370
file.insert(QStringLiteral("path"), QStringLiteral("notepadai_ask_user.json"));
368371
QVERIFY(!isAskUserToolCall(QStringLiteral("write"), intent, file));
@@ -374,7 +377,8 @@ void TestAskUserForm::recognisesOmpDeviceWrite()
374377
QCOMPARE(spec->questions.size(), 1);
375378

376379
// Completion carries the parsed arguments in rawOutput.details.xdev.
377-
const QJsonObject noContent{{QStringLiteral("path"), QStringLiteral("xd://mcp__notepadai_ask_user")}};
380+
const QJsonObject noContent{{QStringLiteral("path"), QStringLiteral("xd://mcp__notepadai_ask_user")},
381+
{QStringLiteral("content"), QStringLiteral("not json")}};
378382
const QJsonObject rawOutput{{QStringLiteral("details"), QJsonObject{{QStringLiteral("xdev"), QJsonObject{
379383
{QStringLiteral("tool"), QStringLiteral("mcp__notepadai_ask_user")},
380384
{QStringLiteral("args"), QJsonDocument::fromJson(argsJson.toUtf8()).object()}}}}}};

0 commit comments

Comments
 (0)