Skip to content

fix: subprocess task execution #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: subprocess task execution #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db2d2c3
fix: subprocess task execution #103
Yohe-Am Sep 20, 2025
0210455
Merge branch 'main' into fix/posix-working-dir
Yohe-Am Sep 21, 2025
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
37 changes: 1 addition & 36 deletions .ghjk/lock.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,6 @@
"ghjkEnvProvInstSet____rust": {
"installs": [
"bciqikjfnbntvagpghawbzlfp2es6lnqzhba3qx5de7tdrmvhuzhsjqa",
"bciqfrfun7z7soj7yxzziyvmt2jnebqvneeoozk5vynmg5pa6wqynhvi",
"bciqlmoqot4jk2lb2b34pldr5iiwsfm3biuipzesjkkwmc2n2o6nlw4q",
"bciqe72molvtvcuj3tuh47ziue2oqd6t4qetxn3rsoa764ofup6uwjmi",
"bciqe4zlekl4uqqbhxunac7br24mrf6cdpfrfblahqa4vrgaqjujcl4i",
Expand All @@ -552,7 +551,6 @@
"bciqhsrsmayibhhhcp3jmay4tnrsyhz5od6ngtaazymx3o64xxzbqiha",
"bciqasgft3ee3shnt2sivmethcza5cebrudgfyw5ntqetu5jnuniqblq",
"bciqikjfnbntvagpghawbzlfp2es6lnqzhba3qx5de7tdrmvhuzhsjqa",
"bciqfrfun7z7soj7yxzziyvmt2jnebqvneeoozk5vynmg5pa6wqynhvi",
"bciqlmoqot4jk2lb2b34pldr5iiwsfm3biuipzesjkkwmc2n2o6nlw4q",
"bciqe72molvtvcuj3tuh47ziue2oqd6t4qetxn3rsoa764ofup6uwjmi",
"bciqe4zlekl4uqqbhxunac7br24mrf6cdpfrfblahqa4vrgaqjujcl4i",
Expand All @@ -579,6 +577,7 @@
"test": {
"ty": "denoFile@v1",
"key": "test",
"workingDir": "./src",
"envKey": "bciqonerdglss7wgwanl5kmaobd65m62hhjbttwhi6kdy5sqf6us6b4a"
},
"lock-sed": {
Expand Down Expand Up @@ -1156,40 +1155,6 @@
"moduleSpecifier": "file:///ports/protoc.ts"
}
},
"bciqfrfun7z7soj7yxzziyvmt2jnebqvneeoozk5vynmg5pa6wqynhvi": {
"port": {
"ty": "denoWorker@v1",
"name": "pipi_pypi",
"platforms": [
"x86_64-linux",
"aarch64-linux",
"x86_64-darwin",
"aarch64-darwin",
"x86_64-windows",
"aarch64-windows",
"x86_64-freebsd",
"aarch64-freebsd",
"x86_64-netbsd",
"aarch64-netbsd",
"x86_64-aix",
"aarch64-aix",
"x86_64-solaris",
"aarch64-solaris",
"x86_64-illumos",
"aarch64-illumos",
"x86_64-android",
"aarch64-android"
],
"version": "0.1.0",
"buildDeps": [
{
"name": "cpy_bs_ghrel"
}
],
"moduleSpecifier": "file:///ports/pipi.ts"
},
"packageName": "cmake"
},
"bciqlmoqot4jk2lb2b34pldr5iiwsfm3biuipzesjkkwmc2n2o6nlw4q": {
"version": "v2.4.0",
"port": {
Expand Down
12 changes: 6 additions & 6 deletions flake.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions flake.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
buildInputs = with pkgs; [
rustChannel
clang
cmake
llvmPackages.libclang
pkg-config
sqlite
Expand Down
9 changes: 6 additions & 3 deletions ghjk.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ ghjk.env("_rust")
.install(
// install rust using rustup or nix (there's a flake.nix file)
ports.protoc(),
ports.pipi({ packageName: "cmake" })[0],
...(Deno.build.os == "linux" && !Deno.env.has("NO_MOLD")
? [ports.mold({
version: "v2.4.0",
Expand Down Expand Up @@ -208,6 +207,10 @@ ghjk.task(
{ inherit: false },
);

ghjk.task(function test($) {
console.log($.argv);
ghjk.task(async function test($) {
console.log($.workingDir, "custom");
await $`echo $PWD`;
console.log(Deno.cwd(), "posix");
}, {
workingDir: "./src",
});
7 changes: 5 additions & 2 deletions install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
# shellcheck disable=SC2016
# shellcheck disable=SC2028

# FIXME: move to .local/bin only after install.ts succeeds
# FIXME: when detecting nixos, advise appropriately

set -e -u

if ! command -v curl >/dev/null; then
Expand Down Expand Up @@ -126,7 +129,7 @@ EOF
fi
fi

GHJK_INSTALLER_URL="${GHJK_INSTALLER_URL:-https://raw.github.com/$ORG/$REPO/$VERSION/install.ts}"
GHJK_INSTALLER_URL="${GHJK_INSTALLER_URL:-https://raw.githubusercontent.com/$ORG/$REPO/$VERSION/install.ts}"
"$GHJK_INSTALL_EXE_DIR/$EXE" deno run -A "$GHJK_INSTALLER_URL"
# Print shell-specific commands for the user to run manually, with the
# current shell shown last for convenience. We do not modify any files.
Expand Down Expand Up @@ -200,4 +203,4 @@ for sh in $ordered_shells; do
done
echo
echo "ghjk has been installed to $GHJK_INSTALL_EXE_DIR"
echo "Add $GHJK_INSTALL_EXE_DIR to your PATH by running one of the commands shown above."
echo "Add $GHJK_INSTALL_EXE_DIR to your PATH by running one of the commands shown above."
46 changes: 46 additions & 0 deletions src/ghjk/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ pub async fn cli() -> Res<std::process::ExitCode> {
};
let gcx = Arc::new(gcx);

// Handle plumbing fast-path before any dynamic CLI/system init
if let QuickCliResult::ExecDenoTask(json_path) = &quick_res {
let res = crate::systems::tasks::exec::exec_deno_task_from_json(&gcx, json_path).await;
deno_cx.terminate().await?;
return res.map(|_| ExitCode::SUCCESS);
}
Comment on lines +86 to +91
Copy link

@coderabbitai coderabbitai bot Sep 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

🧩 Analysis chain

Fix panic when ghjkdir is None by handling ExecDenoTask before the ghjkdir unwrap.

If quick_res is ExecDenoTask and config.ghjkdir is None, Line 41 returns quick_res.exit(None) which hits unreachable! (Line 320) and panics. Move the ghjkdir guard below the fast‑path branch.

Apply:

-    let Some(ghjkdir_path) = config.ghjkdir.clone() else {
-        return Ok(quick_res.exit(None));
-    };
+    // (moved below to avoid panics when ExecDenoTask is requested and no ghjkdir is present)
+    // NOTE: this guard can only run after the ExecDenoTask fast-path is handled.

And insert after the fast‑path (right after Line 91):

+    let Some(ghjkdir_path) = gcx.config.ghjkdir.clone() else {
+        return Ok(quick_res.exit(None));
+    };

Also verify whether skipping write_lockfile_or_log() in the fast‑path is intentional (dynamic path writes it).

Also applies to: 41-44

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== repo root: $(pwd) ==="
echo
echo "=== Searching for ExecDenoTask and QuickCliResult ==="
rg -n --hidden --no-ignore-vcs "ExecDenoTask" || true
rg -n --hidden --no-ignore-vcs "QuickCliResult" || true
echo
echo "=== Searching for ghjkdir and quick_res.exit ==="
rg -n --hidden --no-ignore-vcs "ghjkdir" || true
rg -n --hidden --no-ignore-vcs "quick_res.exit" || true
echo
echo "=== Searching for write_lockfile_or_log and exec_deno_task_from_json ==="
rg -n --hidden --no-ignore-vcs "write_lockfile_or_log" || true
rg -n --hidden --no-ignore-vcs "exec_deno_task_from_json" || true
echo
echo "=== Locate unreachable! occurrences ==="
rg -n --hidden --no-ignore-vcs "unreachable!" || true
echo
echo "=== Dump src/ghjk/cli.rs (first 400 lines) with numbers ==="
nl -ba src/ghjk/cli.rs | sed -n '1,400p' || true
echo
echo "=== Dump any QuickCliResult impl/definition matches ==="
rg -n --hidden --no-ignore-vcs "enum QuickCliResult" -S || true
rg -n --hidden --no-ignore-vcs "impl QuickCliResult" -S || true
echo
echo "=== Done ==="

Length of output: 6768

Fix panic: handle ExecDenoTask fast-path before ghjkdir unwrap

Move the ghjkdir guard below the ExecDenoTask fast‑path so quick_res.exit(None) is not called for ExecDenoTask (it hits an unreachable! arm and panics).

Apply:

-    let Some(ghjkdir_path) = config.ghjkdir.clone() else {
-        return Ok(quick_res.exit(None));
-    };
+    // (moved below to avoid panics when ExecDenoTask is requested and no ghjkdir is present)
+    // NOTE: this guard can only run after the ExecDenoTask fast-path is handled.

Insert after the ExecDenoTask fast‑path branch:

+    let Some(ghjkdir_path) = gcx.config.ghjkdir.clone() else {
+        return Ok(quick_res.exit(None));
+    };

Also verify whether skipping systems.write_lockfile_or_log() in the ExecDenoTask fast‑path is intentional.
[src/ghjk/cli.rs: ~41 (remove) → after ~91 (insert)]

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Handle plumbing fast-path before any dynamic CLI/system init
if let QuickCliResult::ExecDenoTask(json_path) = &quick_res {
let res = crate::systems::tasks::exec::exec_deno_task_from_json(&gcx, json_path).await;
deno_cx.terminate().await?;
return res.map(|_| ExitCode::SUCCESS);
}
// Handle plumbing fast-path before any dynamic CLI/system init
if let QuickCliResult::ExecDenoTask(json_path) = &quick_res {
let res = crate::systems::tasks::exec::exec_deno_task_from_json(&gcx, json_path).await;
deno_cx.terminate().await?;
return res.map(|_| ExitCode::SUCCESS);
}
let Some(ghjkdir_path) = gcx.config.ghjkdir.clone() else {
return Ok(quick_res.exit(None));
};
🤖 Prompt for AI Agents 
In src/ghjk/cli.rs around lines 86 to 91, the ghjkdir guard/unwrapping currently
runs before the ExecDenoTask fast-path which causes quick_res.exit(None) to be
invoked for ExecDenoTask and hit an unreachable! arm leading to a panic; move
the ghjkdir guard and any code that calls quick_res.exit(None) to after the
ExecDenoTask branch (i.e., insert the ghjkdir check immediately after the
ExecDenoTask fast-path return) so ExecDenoTask returns early without executing
the ghjkdir-dependent code, and while making this change, confirm whether the
ExecDenoTask fast-path should also call systems.write_lockfile_or_log() — either
add that call into the fast-path if required or explicitly document/leave it out
if skipping is intentional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fantastic catch.

Copy link

Choose a reason for hiding this comment

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

@Yohe-Am Thank you! I'm glad I could help identify that potential panic scenario. The ordering issue between the ghjkdir guard and the ExecDenoTask fast-path could definitely cause problems when someone tries to use the plumbing command without a proper ghjk setup.

                /|   /|  
               ( :v:  )
                |(_)|
               /     \
              /  ___  \
             (  /   \  )
              \_\___/_/


// ready system contexts
let (system_manifests, envs_ctx, deno_sys_cx) = {
let (sys_envs, envs_ctx) = systems::envs::system(gcx.clone(), &ghjkdir_path).await?;
Expand Down Expand Up @@ -169,6 +176,8 @@ pub async fn cli() -> Res<std::process::ExitCode> {
root_cmd = root_cmd.subcommand(cmd);
}

// (plumbing command now lives in QuickCommands and is handled earlier)

// Register CLI completion reducer on envs with the fully-built root_cmd
// FIXME: this means completions are always generated even if we're not
// writing activator scripts
Expand Down Expand Up @@ -222,6 +231,19 @@ pub async fn cli() -> Res<std::process::ExitCode> {
Ok(QuickCommands::Deno { .. }) => {
unreachable!("deno_quick_cli will prevent this")
}
Ok(QuickCommands::Plumbing { commands }) => match commands {
PlumbingCommands::ExecDenoTask { json } => {
let res = crate::systems::tasks::exec::exec_deno_task_from_json(
&gcx,
std::path::Path::new(&json),
)
.await;
systems.write_lockfile_or_log().await;
deno_sys_cx.terminate().await?;
deno_cx.terminate().await?;
return res.map(|_| ExitCode::SUCCESS);
}
},
Err(err) => {
let kind = err.kind();
use clap::error::ErrorKind;
Expand Down Expand Up @@ -268,6 +290,7 @@ enum QuickCliResult {
ClapErr(clap::Error),
Completions(CompletionShell),
Exit(ExitCode),
ExecDenoTask(std::path::PathBuf),
}
impl QuickCliResult {
fn exit(self, cmd: Option<&mut clap::Command>) -> ExitCode {
Expand All @@ -293,6 +316,7 @@ impl QuickCliResult {
ExitCode::SUCCESS
}
QuickCliResult::Exit(_) => unreachable!("can't happen"),
QuickCliResult::ExecDenoTask(_) => unreachable!("handled earlier"),
}
}
}
Expand Down Expand Up @@ -333,6 +357,11 @@ async fn try_quick_cli(config: &Config) -> Res<QuickCliResult> {
}
QuickCommands::Init { commands } => commands.action(config).await?,
QuickCommands::Deno { .. } => unreachable!("deno quick cli will have prevented this"),
QuickCommands::Plumbing { commands } => match commands {
PlumbingCommands::ExecDenoTask { json } => {
return Ok(QuickCliResult::ExecDenoTask(std::path::PathBuf::from(json)));
}
},
}

Ok(QuickCliResult::Exit(ExitCode::SUCCESS))
Expand Down Expand Up @@ -400,6 +429,23 @@ enum QuickCommands {
#[arg(raw(true))]
args: String,
},
/// Internal commands (subject to change)
#[clap(hide = true)]
Plumbing {
#[command(subcommand)]
commands: PlumbingCommands,
},
}

#[derive(clap::Subcommand, Debug)]
enum PlumbingCommands {
/// Execute a Deno-backed task from a JSON request file
#[clap(hide = true, name = "exec-deno-task")]
ExecDenoTask {
/// Path to the JSON request file
#[arg(value_name = "JSON_FILE", value_hint = clap::ValueHint::FilePath)]
json: String,
},
}

/// TODO: keep more of this in deno next time it's updated
Expand Down
2 changes: 1 addition & 1 deletion src/ghjk/systems/tasks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::interlude::*;

mod exec;
pub mod exec;
mod reducers;
pub mod types;

Expand Down
Loading
Loading