From 2fd0c8ca8637fa0fd57d8fb8fce888317fbac7ea Mon Sep 17 00:00:00 2001
From: Will Crichton <wcrichto@cs.stanford.edu>
Date: Wed, 11 Sep 2024 15:57:34 -0700
Subject: [PATCH] Fix new quest errors, fix Windows errors. Release v0.1.2
 (#19)

* Fix error when initializing quest. Improve time to show initial state

* Improve cross-platform handling of github token

* Show errors in filing issues/PRs

* Release 0.1.2
---
 README.md                            |  2 +-
 js/packages/repo-quest/package.json  |  2 +-
 js/packages/repo-quest/src/index.tsx | 53 +++++++++++++++++++++++-----
 rs/Cargo.lock                        | 21 ++++++++++-
 rs/crates/repo-quest/Cargo.toml      |  3 +-
 rs/crates/repo-quest/Tauri.toml      |  4 +--
 rs/crates/repo-quest/src/github.rs   | 38 ++++++++++----------
 rs/crates/repo-quest/src/lib.rs      | 21 +++++++----
 rs/crates/repo-quest/src/quest.rs    | 38 ++++++++++++++------
 9 files changed, 132 insertions(+), 50 deletions(-)

diff --git a/README.md b/README.md
index 31386e3..bc741c2 100644
--- a/README.md
+++ b/README.md
@@ -28,7 +28,7 @@ We have prebuilt binaries for MacOS (x86-64 and ARM64), Linux (x86-64), and Wind
 
 #### Windows
 
-1. Download the installer (either `.msi` or `.exe) for your platform (arm64 for ARM chips, x64 otherwise).
+1. Download the installer (either `.msi` or `.exe`) for your platform (arm64 for ARM chips, x64 otherwise).
 2. Run the installer.
 3. Launch RepoQuest, e.g. by searching for "RepoQuest" in your applications list.
 
diff --git a/js/packages/repo-quest/package.json b/js/packages/repo-quest/package.json
index 6b4cbe9..5ade6a9 100644
--- a/js/packages/repo-quest/package.json
+++ b/js/packages/repo-quest/package.json
@@ -1,6 +1,6 @@
 {
   "name": "repo-quest",
-  "version": "0.1.1",
+  "version": "0.1.2",
   "type": "module",
   "depot": {
     "platform": "browser",
diff --git a/js/packages/repo-quest/src/index.tsx b/js/packages/repo-quest/src/index.tsx
index 1d17661..502f351 100644
--- a/js/packages/repo-quest/src/index.tsx
+++ b/js/packages/repo-quest/src/index.tsx
@@ -9,6 +9,7 @@ import {
   events,
   type QuestConfig,
   type QuestState,
+  type Result,
   type Stage,
   type StageState,
   type StateDescriptor,
@@ -88,6 +89,17 @@ let ErrorView: React.FC<{ message: string; action: string }> = ({
   return null;
 };
 
+async function tryAwait<T>(
+  promise: Promise<Result<T, string>>,
+  action: string,
+  setMessage: (message: ErrorMessage) => void
+) {
+  let result = await promise;
+  if (result.status === "error") {
+    setMessage({ action, message: result.error });
+  }
+}
+
 let GithubLoader = () => (
   <Await promise={commands.getGithubToken()}>
     {token =>
@@ -114,7 +126,7 @@ let GithubLoader = () => (
           </div>
         </>
       ) : (
-        <pre>ERROR: {token.value}</pre>
+        <ErrorView action="Loading Github token" message={token.value} />
       )
     }
   </Await>
@@ -129,7 +141,10 @@ let LoaderEntry = () => {
     <Await promise={promise}>
       {quest_res =>
         quest_res.status === "ok" ? (
-          <QuestView quest={quest_res.data} />
+          <QuestView
+            quest={quest_res.data[0]}
+            initialState={quest_res.data[1]}
+          />
         ) : (
           <InitForm />
         )
@@ -163,7 +178,10 @@ let InitForm = () => {
     <Await promise={commands.loadQuest(selected.dir)}>
       {quest_res =>
         quest_res.status === "ok" ? (
-          <QuestView quest={quest_res.data} />
+          <QuestView
+            quest={quest_res.data[0]}
+            initialState={quest_res.data[1]}
+          />
         ) : (
           <ErrorView action="Creating new quest" message={quest_res.error} />
         )
@@ -229,7 +247,10 @@ let NewQuest = () => {
     <Await promise={commands.newQuest(dir!, quest!)}>
       {quest_res =>
         quest_res.status === "ok" ? (
-          <QuestView quest={quest_res.data} />
+          <QuestView
+            quest={quest_res.data[0]}
+            initialState={quest_res.data[1]}
+          />
         ) : (
           <ErrorView action="Creating new quest" message={quest_res.error} />
         )
@@ -238,9 +259,12 @@ let NewQuest = () => {
   );
 };
 
-let QuestView: React.FC<{ quest: QuestConfig }> = ({ quest }) => {
+let QuestView: React.FC<{
+  quest: QuestConfig;
+  initialState: StateDescriptor;
+}> = ({ quest, initialState }) => {
   let loader = useContext(Loader.context)!;
-  let [state, setState] = useState<StateDescriptor | undefined>(undefined);
+  let [state, setState] = useState<StateDescriptor | undefined>(initialState);
   let setTitle = useContext(TitleContext)!;
   useEffect(() => setTitle(quest.title), [quest.title]);
 
@@ -331,6 +355,7 @@ let StageView: React.FC<{
   state: QuestState;
 }> = ({ index, stage, state }) => {
   let loader = useContext(Loader.context)!;
+  let setMessage = useContext(ErrorContext)!;
   return (
     <li>
       <div>
@@ -342,7 +367,13 @@ let StageView: React.FC<{
               <button
                 type="button"
                 onClick={() =>
-                  loader.loadAwait(commands.fileFeatureAndIssue(index))
+                  loader.loadAwait(
+                    tryAwait(
+                      commands.fileFeatureAndIssue(index),
+                      "Filing issue or feature PR",
+                      setMessage
+                    )
+                  )
                 }
               >
                 {stage.stage["no-starter"]
@@ -372,7 +403,13 @@ let StageView: React.FC<{
                   <button
                     type="button"
                     onClick={() =>
-                      loader.loadAwait(commands.fileSolution(index))
+                      loader.loadAwait(
+                        tryAwait(
+                          commands.fileSolution(index),
+                          "Filing solution PR",
+                          setMessage
+                        )
+                      )
                     }
                   >
                     File reference solution
diff --git a/rs/Cargo.lock b/rs/Cargo.lock
index fbe6265..3cd5a1c 100644
--- a/rs/Cargo.lock
+++ b/rs/Cargo.lock
@@ -3049,7 +3049,7 @@ checksum = "7a66a03ae7c801facd77a29370b4faec201768915ac14a721ba36f20bc9c209b"
 
 [[package]]
 name = "repo-quest"
-version = "0.1.1"
+version = "0.1.2"
 dependencies = [
  "anyhow",
  "futures-util",
@@ -3071,6 +3071,7 @@ dependencies = [
  "tokio-retry",
  "toml 0.8.19",
  "tracing",
+ "which",
 ]
 
 [[package]]
@@ -4972,6 +4973,18 @@ dependencies = [
  "windows-core 0.58.0",
 ]
 
+[[package]]
+name = "which"
+version = "6.0.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b4ee928febd44d98f2f459a4a79bd4d928591333a494a10a868418ac1b39cf1f"
+dependencies = [
+ "either",
+ "home",
+ "rustix",
+ "winsafe",
+]
+
 [[package]]
 name = "winapi"
 version = "0.3.9"
@@ -5360,6 +5373,12 @@ dependencies = [
  "windows-sys 0.48.0",
 ]
 
+[[package]]
+name = "winsafe"
+version = "0.0.19"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d135d17ab770252ad95e9a872d365cf3090e3be864a34ab46f48555993efc904"
+
 [[package]]
 name = "wry"
 version = "0.43.1"
diff --git a/rs/crates/repo-quest/Cargo.toml b/rs/crates/repo-quest/Cargo.toml
index da2c110..6bdef13 100644
--- a/rs/crates/repo-quest/Cargo.toml
+++ b/rs/crates/repo-quest/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "repo-quest"
-version = "0.1.1"
+version = "0.1.2"
 edition = "2021"
 default-run = "repo-quest"
 
@@ -27,3 +27,4 @@ tauri-plugin-shell = "2.0.0-rc"
 specta = "=2.0.0-rc.20"
 specta-typescript = "0.0.7"
 tauri-specta = { version = "=2.0.0-rc.19", features = ["derive", "typescript"] }
+which = "6.0.3"
diff --git a/rs/crates/repo-quest/Tauri.toml b/rs/crates/repo-quest/Tauri.toml
index f946716..e645d44 100644
--- a/rs/crates/repo-quest/Tauri.toml
+++ b/rs/crates/repo-quest/Tauri.toml
@@ -1,5 +1,5 @@
 productName = "RepoQuest"
-version = "0.1.1"
+version = "0.1.2"
 identifier = "com.repo-quest.app"
 
 [build]
@@ -9,7 +9,7 @@ dev-url = "http://localhost:5173"
 frontend-dist = "../../../js/packages/repo-quest/dist"
 
 [app]
-windows = [{title = "RepoQuest", width = 1200, height = 800}]
+windows = [{title = "RepoQuest", width = 1000, height = 600}]
 
 [bundle]
 active = true
diff --git a/rs/crates/repo-quest/src/github.rs b/rs/crates/repo-quest/src/github.rs
index 0984b76..2d325eb 100644
--- a/rs/crates/repo-quest/src/github.rs
+++ b/rs/crates/repo-quest/src/github.rs
@@ -20,7 +20,7 @@ use regex::Regex;
 use serde::{Deserialize, Serialize};
 use serde_json::json;
 use specta::Type;
-use std::{env, fs, process::Command, sync::Arc, time::Duration};
+use std::{fs, process::Command, sync::Arc, time::Duration};
 use tokio::{time::timeout, try_join};
 use tracing::warn;
 
@@ -52,7 +52,8 @@ impl GithubRepo {
     }
   }
 
-  pub async fn fetch(&self) -> Result<()> {
+  /// Returns true if repo
+  pub async fn fetch(&self) -> Result<bool> {
     let (pr_handler, issue_handler) = (self.pr_handler(), self.issue_handler());
     let res = try_join!(
       pr_handler.list().state(octocrab::params::State::All).send(),
@@ -69,7 +70,7 @@ impl GithubRepo {
           ..
         },
         ..
-      }) => return Ok(()),
+      }) => return Ok(false),
       Err(e) => return Err(e.into()),
     };
     let (prs, mut issues) = (pr_page.take_items(), issue_page.take_items());
@@ -79,7 +80,8 @@ impl GithubRepo {
 
     *self.prs.lock() = Some(prs);
     *self.issues.lock() = Some(issues);
-    Ok(())
+
+    Ok(true)
   }
 
   pub fn remote(&self) -> String {
@@ -236,7 +238,7 @@ impl GithubRepo {
     let is_reset = matches!(merge_type, MergeType::HardReset);
     if is_reset {
       body.push_str(r#"
-      
+
 Note: due to a merge conflict, this PR is a hard reset to the reference solution, and may have overwritten your previous changes."#);
     }
 
@@ -409,22 +411,18 @@ fn read_github_token_from_fs() -> GithubToken {
 }
 
 fn generate_github_token_from_cli() -> GithubToken {
-  let shell = env::var("SHELL").unwrap_or_else(|_| "sh".into());
-  let which_status = Command::new(&shell).args(["-c", "which gh"]).status();
-  match which_status {
-    Ok(status) => {
-      if status.success() {
-        let token_output = token_try!(Command::new(shell)
-          .args(["-c", "gh auth token"])
-          .output()
-          .context("Failed to run `gh auth token`"));
-        let token = token_try!(String::from_utf8(token_output.stdout));
-        let token_clean = token.trim_end().to_string();
-        GithubToken::Found(token_clean)
-      } else {
-        GithubToken::NotFound
-      }
+  let gh_path_res = which::which("gh");
+  match gh_path_res {
+    Ok(gh_path) => {
+      let token_output = token_try!(Command::new(gh_path)
+        .args(["auth", "token"])
+        .output()
+        .context("Failed to run `gh auth token`"));
+      let token = token_try!(String::from_utf8(token_output.stdout));
+      let token_clean = token.trim_end().to_string();
+      GithubToken::Found(token_clean)
     }
+    Err(which::Error::CannotFindBinaryPath) => GithubToken::NotFound,
     Err(err) => GithubToken::Error(format!("{err:?}")),
   }
 }
diff --git a/rs/crates/repo-quest/src/lib.rs b/rs/crates/repo-quest/src/lib.rs
index 98dbf29..92b24fe 100644
--- a/rs/crates/repo-quest/src/lib.rs
+++ b/rs/crates/repo-quest/src/lib.rs
@@ -5,7 +5,7 @@ use std::{env, path::PathBuf, sync::Arc};
 
 use self::quest::{Quest, QuestConfig};
 use github::GithubToken;
-use quest::StateEvent;
+use quest::{StateDescriptor, StateEvent};
 use tauri::{AppHandle, Manager, State};
 use tauri_specta::collect_events;
 
@@ -58,19 +58,28 @@ async fn load_quest_core(
 
 #[tauri::command]
 #[specta::specta]
-async fn load_quest(dir: PathBuf, app: AppHandle) -> Result<QuestConfig, String> {
+async fn load_quest(
+  dir: PathBuf,
+  app: AppHandle,
+) -> Result<(QuestConfig, StateDescriptor), String> {
   let config = fmt_err(QuestConfig::load(&dir))?;
-  load_quest_core(dir, &config, app).await?;
-  Ok(config)
+  let quest = load_quest_core(dir, &config, app).await?;
+  let state = fmt_err(quest.state_descriptor().await)?;
+  Ok((config, state))
 }
 
 #[tauri::command]
 #[specta::specta]
-async fn new_quest(dir: PathBuf, quest: String, app: AppHandle) -> Result<QuestConfig, String> {
+async fn new_quest(
+  dir: PathBuf,
+  quest: String,
+  app: AppHandle,
+) -> Result<(QuestConfig, StateDescriptor), String> {
   let config = fmt_err(quest::load_config_from_remote("cognitive-engineering-lab", &quest).await)?;
   let quest = load_quest_core(dir.join(quest), &config, app).await?;
   fmt_err(quest.create_repo().await)?;
-  Ok(config)
+  let state = fmt_err(quest.state_descriptor().await)?;
+  Ok((config, state))
 }
 
 #[tauri::command]
diff --git a/rs/crates/repo-quest/src/quest.rs b/rs/crates/repo-quest/src/quest.rs
index b224d5a..0e0e08a 100644
--- a/rs/crates/repo-quest/src/quest.rs
+++ b/rs/crates/repo-quest/src/quest.rs
@@ -3,6 +3,7 @@ use std::{
   env::{self, set_current_dir},
   path::{Path, PathBuf},
   process::Command,
+  sync::atomic::{AtomicBool, Ordering},
   time::Duration,
 };
 
@@ -77,6 +78,7 @@ pub struct Quest {
   upstream: GithubRepo,
   origin: GithubRepo,
   origin_git: GitRepo,
+  origin_exists: AtomicBool,
   stage_index: HashMap<String, usize>,
   dir: PathBuf,
   app: Option<AppHandle>,
@@ -129,6 +131,9 @@ impl Quest {
       .map(|(i, stage)| (stage.label.clone(), i))
       .collect::<HashMap<_, _>>();
 
+    let (origin_exists, _) = try_join!(origin.fetch(), upstream.fetch())?;
+    let origin_exists = AtomicBool::new(origin_exists);
+
     let q = Quest {
       dir,
       user,
@@ -136,13 +141,11 @@ impl Quest {
       upstream,
       origin,
       origin_git,
+      origin_exists,
       stage_index,
       app,
     };
 
-    try_join!(q.origin.fetch(), q.upstream.fetch())?;
-
-    // Need to infer_state_update after fetching repo data so issues/PRs are populated
     q.infer_state_update().await?;
 
     if q.dir.exists() {
@@ -297,15 +300,26 @@ impl Quest {
     })
   }
 
+  pub async fn state_descriptor(&self) -> Result<StateDescriptor> {
+    let state = self.infer_state().await?;
+    Ok(StateDescriptor {
+      dir: self.dir.clone(),
+      stages: self.stage_states(),
+      state,
+    })
+  }
+
   pub async fn infer_state_update(&self) -> Result<()> {
-    let (new_state, _) = try_join!(self.infer_state(), self.origin.fetch())?;
+    // If the repo is not yet created, then there's nothing to fetch.
+    // This also prevents panics wrt trying to access issues/PRs on origin
+    if !self.origin_exists.load(Ordering::SeqCst) {
+      return Ok(());
+    }
+
+    self.origin.fetch().await?;
+    let state = self.state_descriptor().await?;
     if let Some(app) = &self.app {
-      let descriptor = StateDescriptor {
-        dir: self.dir.clone(),
-        stages: self.stage_states(),
-        state: new_state,
-      };
-      StateEvent(descriptor).emit(app)?;
+      StateEvent(state).emit(app)?;
     }
 
     Ok(())
@@ -336,6 +350,10 @@ impl Quest {
     // Initialize the upstreams and fetch content
     self.origin_git.setup_upstream(&self.upstream)?;
 
+    self.origin_exists.store(true, Ordering::SeqCst);
+
+    self.infer_state_update().await?;
+
     Ok(())
   }