From 2217037d72a43485313f0b908ecac0b325435646 Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Sun, 1 Nov 2026 18:33:59 +0100 Subject: [PATCH] feat(cli): pick the app of the project the launcher belongs to Discovery is per user, so every project's launcher sees every debug app running, and with two of them up every command refused to guess and asked for -i, though the agent in one project meant its own app. Two copies of the example, both named basic, could only be told apart by id. The launcher now exports CRAFTER_DEBUG_PROJECT, its project's directory (flatpak-spawn gets it spelled out), and the client counts an app as the project's when its executable lies under it, as `-r` runs it from /bin//. With other projects' apps running, commands pick this project's; two copies of it still need -i. Another project's app is never picked by default: with this project's app not up yet, or crashed, the agent would otherwise drive, or quit, someone else's. It is listed instead, for -i. `ls` lists this project's apps first, marked, `--json` says which are, and `--wait` waits for one of them rather than any app; the exit code says whether one runs. A client started without the launcher has no project and picks the one app running, as before, and the command the protocol check prints for an older app's own client now names it with -i. Co-Authored-By: Claude Opus 5.5 --- implementations/Crafter.Debug.Cli-Client.cpp | 75 +++++++++++++++++--- implementations/main.cpp | 2 + interfaces/Crafter.Debug.Cli-Client.cppm | 5 ++ tests/ShouldRunClientCommands/main.cpp | 45 +++++++++++- tooling/interfaces/Crafter.Debug.Build.cppm | 15 ++-- 5 files changed, 126 insertions(+), 16 deletions(-) diff --git a/implementations/Crafter.Debug.Cli-Client.cpp b/implementations/Crafter.Debug.Cli-Client.cpp index bc4d041..921f7cd 100644 --- a/implementations/Crafter.Debug.Cli-Client.cpp +++ b/implementations/Crafter.Debug.Cli-Client.cpp @@ -230,7 +230,13 @@ namespace { class Client { public: - Client(Environment& environment, Options options) : env(environment), opts(std::move(options)) {} + Client(Environment& environment, Options options) : env(environment), opts(std::move(options)) { + if (!env.project.empty()) { + std::error_code ec; + project = fs::weakly_canonical(env.project, ec); + if (ec) project = env.project.lexically_normal(); + } + } std::int32_t Execute() { const std::string& c = opts.command; @@ -265,6 +271,8 @@ namespace { private: Environment& env; Options opts; + // env.project, resolved like the executables it is compared with. + fs::path project; std::optional instance; std::unique_ptr connection; @@ -278,13 +286,42 @@ namespace { // ---- choosing the instance ---- + // Whether `record` is an app of this client's project: crafter-build + // runs it from /bin//, so its executable lies under + // the project's directory. + bool Ours(const InstanceRecord& record) const { + if (project.empty() || record.exe.empty()) return false; + std::error_code ec; + fs::path exe = fs::weakly_canonical(fs::path(record.exe), ec); + if (ec) exe = fs::path(record.exe).lexically_normal(); + const fs::path relative = exe.lexically_relative(project); + return !relative.empty() && *relative.begin() != ".."; + } + std::int32_t Select() { std::vector live = env.list(); - if (!opts.instance.empty()) { + const bool named = !opts.instance.empty(); + if (named) { std::erase_if(live, [&](const Instance& candidate) { return candidate.record.name != opts.instance && !candidate.record.instanceId.starts_with(opts.instance); }); } + // This project's apps, when it runs any: an agent working here + // means one of its own. Another project's app only through -i, + // never by default: with this project's app not up yet, or + // crashed, the agent would otherwise drive, or quit, someone + // else's. + std::vector ours; + std::ranges::copy_if(live, std::back_inserter(ours), [&](const Instance& candidate) { return Ours(candidate.record); }); + if (!ours.empty()) { + live = std::move(ours); + } else if (!named && !project.empty() && !live.empty()) { + Err() << std::format("crafter-debug: nothing from this project ({}) is running. Start its app in the background (e.g. crafter-build --debug-api -r), " + "or pick one of these with -i :\n", + project.string()); + for (const Instance& other : live) Err() << std::format(" {} id {} up {} {}\n", other.record.name, other.record.instanceId, Age(other.record.startedAt), other.record.exe); + return exitNoInstance; + } if (live.empty()) { if (!opts.instance.empty()) { std::string running; @@ -310,7 +347,7 @@ namespace { } else { Err() << " Restart the app from the current build (crafter-build -r), or talk to it with its own client:\n"; } - Err() << std::format(" flatpak run --die-with-parent --command={} net.catcrafts.CrafterBuild {}\n", record.client, opts.command); + Err() << std::format(" flatpak run --die-with-parent --command={} net.catcrafts.CrafterBuild -i {} {}\n", record.client, record.instanceId, opts.command); return exitVersion; } if (!record.sourceId.empty() && record.sourceId != Detail::SourceId()) { @@ -436,21 +473,29 @@ namespace { return Usage(std::format("unknown argument '{}'", opts.rest[i])); } } + // With a project, what counts is one of its apps: --wait waits for + // one, and the exit code says whether one runs. The others are + // listed after it all the same, for -i. + const auto found = [&](const std::vector& list) { + return project.empty() ? !list.empty() : std::ranges::any_of(list, [&](const Instance& entry) { return Ours(entry.record); }); + }; const auto deadline = std::chrono::steady_clock::now() + wait.value_or(0ms); std::vector live = env.list(); - while (live.empty() && wait && std::chrono::steady_clock::now() < deadline) { + while (!found(live) && wait && std::chrono::steady_clock::now() < deadline) { std::this_thread::sleep_for(200ms); live = env.list(); } + std::ranges::stable_partition(live, [&](const Instance& entry) { return Ours(entry.record); }); if (opts.json) { Value list = Value::MakeArray(); for (const Instance& entry : live) { Value record = Encode(entry.record); record.Erase("token"); + if (!project.empty()) record.Set("thisProject", Value(Ours(entry.record))); list.Push(std::move(record)); } Out() << list.Dump() << '\n'; - return live.empty() ? exitNoInstance : exitOk; + return found(live) ? exitOk : exitNoInstance; } if (live.empty()) { Err() << "no running instances. Start the app from its debug build in the background (e.g. crafter-build --debug-api -r)" @@ -459,9 +504,15 @@ namespace { } for (const Instance& entry : live) { const InstanceRecord& r = entry.record; - Out() << std::format("{} id {} pid {}{} up {} port {}{}\n", r.name, r.instanceId, r.pid, + Out() << std::format("{} id {} pid {}{} up {} port {}{}{}\n", r.name, r.instanceId, r.pid, r.hostInitPid > 0 ? std::format(" (sandbox {})", r.hostInitPid) : std::string(), Age(r.startedAt), r.port, - r.protocol != protocolVersion ? std::format(" protocol {} (this client {})", r.protocol, protocolVersion) : std::string()); + r.protocol != protocolVersion ? std::format(" protocol {} (this client {})", r.protocol, protocolVersion) : std::string(), + Ours(r) ? std::string(" (this project)") : std::string()); + } + if (!found(live)) { + Err() << std::format("none of these is from this project ({}); start its app in the background (e.g. crafter-build --debug-api -r){}\n", + project.string(), wait ? ", and wait longer if it is still starting" : std::string(", then: crafter-debug ls --wait 60s")); + return exitNoInstance; } return exitOk; } @@ -987,7 +1038,8 @@ memory are a debugger's job; `crafter-debug debugger` prints how to attach one. ## Commands - ls [--wait 60s] + ls [--wait 60s] running apps, this project's first; --wait + waits for one of this project's status cmds [glob] name(args = defaults) -> result -- description help @@ -1015,8 +1067,11 @@ memory are a debugger's job; `crafter-debug debugger` prints how to attach one. debugger how to attach lldb or gdb version -Global: -i picks an instance when several run; --json prints raw -JSON (never truncated); --timeout 30s bounds the request (waits included). +Global: commands pick this project's app; when another project's apps run +too they are left alone, and `ls` marks this project's. -i picks +one by name or id: another project's app, or one of several copies of this +one. --json prints raw JSON (never truncated); --timeout 30s bounds the +request (waits included). ## Conditions diff --git a/implementations/main.cpp b/implementations/main.cpp index d0e4f57..c432f09 100644 --- a/implementations/main.cpp +++ b/implementations/main.cpp @@ -17,6 +17,8 @@ int main(int argc, char** argv) { { Cli::Environment environment; environment.connect = Cli::ConnectQuic; + // Set by the project's launcher, build/crafter-debug. + if (const char* project = std::getenv("CRAFTER_DEBUG_PROJECT"); project != nullptr && *project != '\0') environment.project = project; environment.lldb = [] { for (const char* path : { "/app/bin/lldb", "/usr/bin/lldb" }) { if (std::filesystem::exists(path)) return std::string(path); diff --git a/interfaces/Crafter.Debug.Cli-Client.cppm b/interfaces/Crafter.Debug.Cli-Client.cppm index 02d56d4..a0b14e0 100644 --- a/interfaces/Crafter.Debug.Cli-Client.cppm +++ b/interfaces/Crafter.Debug.Cli-Client.cppm @@ -58,6 +58,11 @@ export namespace Crafter::Debug::Cli { std::ostream* err = &std::cerr; // Where a screenshot without -o goes, relative paths included. std::filesystem::path cwd = std::filesystem::current_path(); + // The project the launcher that started this client was written for + // (CRAFTER_DEBUG_PROJECT). Its apps, the ones whose executable lies + // under it, are picked without -i when other apps run too, and no + // other app is. Empty: whichever app runs, as long as only one does. + std::filesystem::path project; // Where lldb is here, for the attach command `debugger` prints: its // absolute path, since `flatpak enter` runs a command without the // sandbox's PATH, which /app/bin is on. Empty for none: gdb then. diff --git a/tests/ShouldRunClientCommands/main.cpp b/tests/ShouldRunClientCommands/main.cpp index 2817e04..603ee39 100644 --- a/tests/ShouldRunClientCommands/main.cpp +++ b/tests/ShouldRunClientCommands/main.cpp @@ -6,7 +6,8 @@ // while a thread plays the app's frame loop (Advance, Pump). Covers what an // agent sees: typed key=value calls and the usage error for a bad value, // suggestions for typos, get/set, pause/step/resume, waits, the log, traces, -// screenshots written where asked, --json, instance selection, the protocol +// screenshots written where asked, --json, instance selection (this +// project's apps first, another project's only through -i), the protocol // and source checks against the discovery record, a server-side timeout, // and a client-side one that diagnoses an app no longer calling Pump. #include @@ -76,10 +77,12 @@ namespace { std::string err; }; - Ran Run(std::vector args, const std::vector& instances) { + // `project` as the launcher would pass it; empty, as without one. + Ran Run(std::vector args, const std::vector& instances, const fs::path& project = {}) { std::ostringstream out; std::ostringstream err; Cli::Environment environment; + environment.project = project; environment.connect = [](const InstanceRecord&) -> std::unique_ptr { return std::make_unique(); }; environment.list = [instances] { return instances; }; environment.out = &out; @@ -212,6 +215,44 @@ int main() { r = Run({ "guide" }, instances); Check(r.code == 0 && Has(r.out, "# crafter-debug"), "guide"); + // Apps of two projects at once. `debugger` prints the pid of the record + // it picked, which is how these tell which one that was. + const fs::path here = "/projects/game"; + InstanceRecord mine = record; + mine.instanceId = "aaa111"; + mine.pid = 11; + mine.exe = (here / "bin" / "variant" / "game").string(); + InstanceRecord copy = mine; + copy.instanceId = "aaa222"; + copy.pid = 12; + InstanceRecord theirs = record; + theirs.name = "tool"; + theirs.instanceId = "bbb111"; + theirs.pid = 21; + theirs.exe = "/projects/game-tools/bin/variant/tool"; + r = Run({ "debugger" }, { { theirs, {} }, { mine, {} } }, here); + Check(r.code == 0 && Has(r.out, "-p 11"), std::format("with another project's app running too, this project's is picked ({})", r.out + r.err)); + r = Run({ "debugger" }, { { theirs, {} } }, here); + Check(r.code == 3 && Has(r.err, "nothing from this project") && Has(r.err, "bbb111"), + std::format("another project's app alone is not picked, but listed for -i ({})", r.err)); + r = Run({ "-i", "tool", "debugger" }, { { theirs, {} }, { mine, {} } }, here); + Check(r.code == 0 && Has(r.out, "-p 21"), "-i picks another project's app"); + r = Run({ "debugger" }, { { mine, {} }, { copy, {} } }, here); + Check(r.code == 2 && Has(r.err, "several instances"), "two copies of this project's app still need -i"); + r = Run({ "-i", "aaa2", "debugger" }, { { mine, {} }, { copy, {} } }, here); + Check(r.code == 0 && Has(r.out, "-p 12"), "a start of the id is enough"); + r = Run({ "debugger" }, { { theirs, {} } }); + Check(r.code == 0 && Has(r.out, "-p 21"), "without a launcher's project, the one app running is picked as before"); + r = Run({ "ls" }, { { theirs, {} }, { mine, {} } }, here); + Check(r.code == 0 && r.out.starts_with("game id aaa111") && Has(r.out, "(this project)") && Has(r.out, "tool id bbb111"), + std::format("ls lists this project's apps first, marked, then the others ({})", r.out)); + const auto waited = std::chrono::steady_clock::now(); + r = Run({ "ls", "--wait", "300ms" }, { { theirs, {} } }, here); + Check(r.code == 3 && std::chrono::steady_clock::now() - waited >= 300ms && Has(r.out, "tool id bbb111") && Has(r.err, "none of these is from this project"), + std::format("ls --wait waits for this project's app, not any ({})", r.err)); + r = Run({ "--json", "ls" }, { { theirs, {} }, { mine, {} } }, here); + Check(r.code == 0 && Has(r.out, R"("thisProject":true)") && Has(r.out, R"("thisProject":false)"), "ls --json says which apps are this project's"); + pumping = false; app.join(); r = Run({ "--timeout", "100ms", "get", "hp" }, instances); diff --git a/tooling/interfaces/Crafter.Debug.Build.cppm b/tooling/interfaces/Crafter.Debug.Build.cppm index 00b8b43..cf53a6e 100644 --- a/tooling/interfaces/Crafter.Debug.Build.cppm +++ b/tooling/interfaces/Crafter.Debug.Build.cppm @@ -80,17 +80,24 @@ namespace Crafter::Debug { // against; from another app's sandbox it goes through flatpak-spawn. // `--die-with-parent` ends the client when whatever started the launcher // is killed, a timed-out `wait` or a Monitor-stopped `log --follow`. - std::string LauncherText(const fs::path& client, const Sandbox& sandbox, std::string_view flag) { + // + // It tells the client which project it belongs to: with apps of other + // projects running too, the client picks this project's and leaves the + // others to -i. flatpak run passes the environment on; flatpak-spawn + // needs it spelled out. + std::string LauncherText(const fs::path& client, const Sandbox& sandbox, std::string_view flag, const fs::path& project) { std::string text = std::format( "#!/bin/sh\n" "# Written by Crafter.Debug (Debug::Use) each time crafter-build loads this\n" "# project with {0}: runs the crafter-debug client that build compiled.\n" "client={1}\n" + "CRAFTER_DEBUG_PROJECT={2}\n" + "export CRAFTER_DEBUG_PROJECT\n" "if [ ! -x \"$client\" ]; then\n" " echo \"crafter-debug: the client is not built yet; let crafter-build {0} finish first, which builds it with the app\" >&2\n" " exit 2\n" "fi\n", - flag, ShellQuote(client.string())); + flag, ShellQuote(client.string()), ShellQuote(project.string())); if (sandbox.app.empty()) { text += "exec \"$client\" \"$@\"\n"; return text; @@ -100,7 +107,7 @@ namespace Crafter::Debug { text += std::format( "if grep -qsx {0} /.flatpak-info; then exec \"$client\" \"$@\"; fi\n" "if command -v flatpak >/dev/null 2>&1; then exec {1}; fi\n" - "if command -v flatpak-spawn >/dev/null 2>&1; then exec flatpak-spawn --host {1}; fi\n" + "if command -v flatpak-spawn >/dev/null 2>&1; then exec flatpak-spawn --host --env=CRAFTER_DEBUG_PROJECT=\"$CRAFTER_DEBUG_PROJECT\" {1}; fi\n" "echo \"crafter-debug: needs flatpak to start $client\" >&2\n" "exit 1\n", ShellQuote(std::format("name={}", sandbox.app)), run); @@ -189,7 +196,7 @@ export namespace Crafter::Debug { if (client == library->dependencies.end()) { throw std::runtime_error(std::format("Crafter.Debug: {} declares no crafter.debug-cli client", (Checkout() / "project.cpp").string())); } - WriteIfChanged(launcher, LauncherText((*client)->BinDir() / (*client)->outputName, CurrentSandbox(), options.flag)); + WriteIfChanged(launcher, LauncherText((*client)->BinDir() / (*client)->outputName, CurrentSandbox(), options.flag, launcher.parent_path().parent_path())); return true; } }