fix(forge): review fixes for the forgejo-api port

This commit is contained in:
müde 2026-07-07 14:42:43 +02:00
commit 261f02439b
6 changed files with 57 additions and 22 deletions

1
Cargo.lock generated
View file

@ -1439,7 +1439,6 @@ dependencies = [
"serde", "serde",
"serde_json", "serde_json",
"tempfile", "tempfile",
"time",
"tokio", "tokio",
"tokio-stream", "tokio-stream",
"tracing", "tracing",

View file

@ -381,6 +381,10 @@ async fn format_notification(
let subj = notif.thread.subject.as_ref(); let subj = notif.thread.subject.as_ref();
let title = subj.and_then(|s| s.title.as_deref()).unwrap_or("?"); let title = subj.and_then(|s| s.title.as_deref()).unwrap_or("?");
let subject_type = subj.and_then(|s| s.r#type); let subject_type = subj.and_then(|s| s.r#type);
// forgejo-api maps a blank `html_url` (Go marshals empty strings) to
// `None`, so the API-url fallback also covers present-but-empty —
// deliberate: a fetchable API link beats the raw-HTTP predecessor's
// empty `url:` line.
let html_url = subj let html_url = subj
.and_then(|s| s.html_url.as_ref().or(s.url.as_ref())) .and_then(|s| s.html_url.as_ref().or(s.url.as_ref()))
.map_or("", url::Url::as_str); .map_or("", url::Url::as_str);

View file

@ -14,7 +14,6 @@ bcrypt.workspace = true
reqwest.workspace = true reqwest.workspace = true
forgejo-api.workspace = true forgejo-api.workspace = true
url.workspace = true url.workspace = true
time.workspace = true
clap.workspace = true clap.workspace = true
clap_complete.workspace = true clap_complete.workspace = true
clap-markdown = "0.1" clap-markdown = "0.1"

View file

@ -117,6 +117,18 @@ fn is_conflict(e: &ForgejoError) -> bool {
} }
} }
/// Whether an error is Forgejo saying 404 — the resource is absent,
/// as opposed to a transport / auth / server failure.
fn is_not_found(e: &ForgejoError) -> bool {
match e {
ForgejoError::ApiError(api) => {
matches!(api.error_kind(), ApiErrorKind::NotFound { .. })
}
ForgejoError::UnexpectedStatusCode(s) => *s == StatusCode::NOT_FOUND,
_ => false,
}
}
/// Fold a repo-creation result's "already exists" (409 / 422) into /// Fold a repo-creation result's "already exists" (409 / 422) into
/// success. `label` is `<owner>/<name>` — purely for log + error /// success. `label` is `<owner>/<name>` — purely for log + error
/// context. /// context.
@ -474,23 +486,33 @@ async fn ensure_mirror_repo(
admin_token: &str, admin_token: &str,
) -> Result<()> { ) -> Result<()> {
let client = api(admin_token)?; let client = api(admin_token)?;
if client.repo_get(owner, repo).await.is_ok() { match client.repo_get(owner, repo).await {
// Mirror already present. Patch interval so mirrors seeded before Ok(_) => {
// this field was introduced (or with a different value) converge. // Mirror already present. Patch interval so mirrors seeded before
let mut edit = sparse_edit_repo_option(); // this field was introduced (or with a different value) converge.
edit.mirror_interval = Some(MIRROR_INTERVAL.to_owned()); let mut edit = sparse_edit_repo_option();
match client.repo_edit(owner, repo, edit).await { edit.mirror_interval = Some(MIRROR_INTERVAL.to_owned());
Ok(_) => { match client.repo_edit(owner, repo, edit).await {
tracing::debug!(%owner, %repo, interval = MIRROR_INTERVAL, "forge: pull-mirror interval updated"); Ok(_) => {
} tracing::debug!(%owner, %repo, interval = MIRROR_INTERVAL, "forge: pull-mirror interval updated");
Err(e) => { }
tracing::warn!( Err(e) => {
%owner, %repo, error = %e, tracing::warn!(
"forge: failed to set mirror_interval on existing pull-mirror" %owner, %repo, error = %e,
); "forge: failed to set mirror_interval on existing pull-mirror"
);
}
} }
return Ok(());
}
// Absent — fall through to migrate.
Err(e) if is_not_found(&e) => {}
// Anything else (transport, auth, 5xx) leaves the repo's existence
// unknown: migrating anyway would fold a 409 into success and skip
// the interval patch this pass. Surface it instead.
Err(e) => {
return Err(e).with_context(|| format!("get pull-mirror {owner}/{repo}"));
} }
return Ok(());
} }
let opts = MigrateRepoOptions { let opts = MigrateRepoOptions {
auth_password: None, auth_password: None,

View file

@ -148,13 +148,23 @@ async fn seed_readme(core_token: &str) -> Result<()> {
/// Called at startup alongside [`ensure_local_clone`]. No-op when the /// Called at startup alongside [`ensure_local_clone`]. No-op when the
/// core token is absent (forge not yet provisioned). /// core token is absent (forge not yet provisioned).
pub async fn ensure_webhook(core_token: &str, dashboard_port: u16) -> Result<()> { pub async fn ensure_webhook(core_token: &str, dashboard_port: u16) -> Result<()> {
// The typed client carries no per-request timeout, so each call is
// wrapped in one: this runs as a detached startup task, and a forge
// that accepts connections but never answers would otherwise hang
// it forever (and the hourly pull fallback masks the missing hook).
const HTTP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
let target_url = format!("http://127.0.0.1:{dashboard_port}/webhook/knowledge"); let target_url = format!("http://127.0.0.1:{dashboard_port}/webhook/knowledge");
let client = crate::forge::api(core_token)?; let client = crate::forge::api(core_token)?;
// List existing hooks — skip creation if ours is already there. // List existing hooks — skip creation if ours is already there.
// Best-effort like the raw-HTTP predecessor: a listing failure // Best-effort like the raw-HTTP predecessor: a listing failure
// falls through to the create attempt. // falls through to the create attempt.
match client.repo_list_hooks(ORG, REPO).all().await { let listed = tokio::time::timeout(HTTP_TIMEOUT, client.repo_list_hooks(ORG, REPO).all())
.await
.map_err(anyhow::Error::from)
.and_then(|r| r.map_err(anyhow::Error::from));
match listed {
Ok(hooks) => { Ok(hooks) => {
let already_exists = hooks.iter().any(|h| { let already_exists = hooks.iter().any(|h| {
h.config h.config
@ -186,9 +196,10 @@ pub async fn ensure_webhook(core_token: &str, dashboard_port: u16) -> Result<()>
events: Some(vec!["push".to_owned()]), events: Some(vec!["push".to_owned()]),
r#type: CreateHookOptionType::Forgejo, r#type: CreateHookOptionType::Forgejo,
}; };
client tokio::time::timeout(HTTP_TIMEOUT, client.repo_create_hook(ORG, REPO, hook))
.repo_create_hook(ORG, REPO, hook)
.await .await
.map_err(anyhow::Error::from)
.and_then(|r| r.map_err(anyhow::Error::from))
.with_context(|| format!("create webhook for {ORG}/{REPO}"))?; .with_context(|| format!("create webhook for {ORG}/{REPO}"))?;
tracing::info!(%target_url, "knowledge: push webhook created"); tracing::info!(%target_url, "knowledge: push webhook created");
Ok(()) Ok(())

View file

@ -1,7 +1,7 @@
//! Resolve the on-disk path to hyperhive's static assets (branding + //! Resolve the on-disk path to hyperhive's static assets (branding +
//! claude prompts). Single source of truth for both the host daemon //! claude prompts). Single source of truth for both the host daemon
//! (`hive-c0re`) and the in-container harness (the `hive` binary) so //! (`hive-c0re`) and the in-container harness binaries so they agree
//! they agree on the lookup contract. //! on the lookup contract.
//! //!
//! At runtime, the path is read from `$HIVE_ASSETS_DIR`. In nix //! At runtime, the path is read from `$HIVE_ASSETS_DIR`. In nix
//! builds that env var is set by the `hive-c0re` / `harness-base` modules //! builds that env var is set by the `hive-c0re` / `harness-base` modules