mirror of
https://github.com/storytold/artcraft.git
synced 2026-11-11 00:11:05 +00:00
main
7
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 FredApps
|
71adfd4701 |
artcraft app: mark fire-and-forget promises with void (Sonar S9383) (#2806)
## Summary
Resolves all 38 SonarCloud **typescript:S9383** findings ("Promises must
be awaited, end with a call to .catch, end with a call to .then with a
rejection handler or be explicitly marked as ignored with the `void`
operator") in `frontend/apps/artcraft/app`, across 19 files. They are:
- startup calls in `index.tsx`, such as `SyncStorytellerApiConfig()` and
`persistLogin()`
- polling in `useActiveJobs`, `useBackgroundLoadingMedia`,
`signals/activeJobs.ts` and `signals/authentication/fetchers.ts`
- listener cleanups (`unlisten*.then(f => f())`) in `TaskQueue` and
`useDesktopGenerationFeed`
- UI handlers that start uploads or renames, such as
`ImageTo3DExperience`, `UploadFilesMedia`, `SceneTitleInput`,
`ProfileDropdown` and `DesktopGenerationGallery`
- async IIFEs in `useDesktopUsername`, `TaskQueue` and
`BaseImageSelector`
This is part 2 of 3 for artcraft's 157 S9383 findings. Part 1 (Tauri
libraries) is #2805. Part 3 (`libs/components`) is #2807.
Several of these files are also changed by open PRs for unrelated
reasons, for example #2519, #2520, #2615 and #2064. Each change here
inserts one word at the start of a statement, so a rebase either way is
trivial.
## Approach
This ports the approach of storytold/artcraft-services#179, which fixes
the same rule in artcraft-services. **#179 is still open and unmerged.**
This PR follows its approach and does not depend on it.
- Each flagged expression is a fire-and-forget promise: a listener
`setup()`, an `unlisten.then(f => f())` cleanup, a loader called from an
effect, an upload kicked off from a handler, or an async IIFE. Prefixing
it with `void` marks the ignored promise explicitly, which is the fix
the rule documents.
- It deliberately does not add `.catch()` everywhere. These call sites
already handle failures inside the async function or surface them
through their own state, and empty handlers would hide errors.
- Async IIFEs become `void (async () => …)()`. `if (cond) call()`
becomes `if (cond) void call()`.
- There is no behaviour change. `void` only discards the promise value,
which was already being discarded.
Each insertion is placed at the exact `textRange` start offset of the
SonarCloud finding, with no other edits.
## Testing
- Each changed file parses with `esbuild <file>`, using the same check
as #179.
- I parsed each changed file with the TypeScript 5.9 compiler API
(`createSourceFile`). That found no parse errors, and every inserted
`void` is either a whole expression statement or an arrow-function body
applied to a call, never part of a larger expression.
- `frontend/.eslintrc.json` has no `no-void` rule.
- I did not run the full Nx type-check, lint or test targets, because
`frontend/node_modules` is not installed in my environment. Please rely
on CI.
- No tests are added. Discarding an already-discarded promise value is
not observable, so no test can fail before this change and pass after
it.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Chores**
* Updated background tasks across uploads, sign-in, polling, scene
editing, and other app workflows to handle asynchronous operations
consistently.
* Existing workflows and their success and error handling remain
unchanged; no user-facing behavior changes are expected.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
FredApps
|
c9da5cac18 |
tauri-events/tauri-api/tauri-utils: mark fire-and-forget promises with void (Sonar S9383) (#2805)
## Summary
Resolves all 53 SonarCloud **typescript:S9383** findings ("Promises must
be awaited, end with a call to .catch, end with a call to .then with a
rejection handler or be explicitly marked as ignored with the `void`
operator") in the Tauri frontend libraries:
| Library | Findings | What is marked |
|----------------------------|----------|------------------------------------------------------------------------|
| `frontend/libs/tauri-events` | 48 | `setup()` and the `unlisten.then(f
=> f())` cleanups in each `use*Event` hook |
| `frontend/libs/tauri-api` | 4 | Same pattern in
`CanvasBgRemovedEvent`, plus `setupListener()` in `useSoraLoginListener`
|
| `frontend/libs/tauri-utils` | 1 | The async IIFE in
`useTauriWindowControls` |
This is part 1 of 3 for artcraft's 157 S9383 findings. Part 2
(`apps/artcraft/app`) is #2806, and part 3 (`libs/components`) is #2807.
## Approach
This ports the approach of storytold/artcraft-services#179, which fixes
the same rule in artcraft-services. **#179 is still open and unmerged.**
This PR follows its approach and does not depend on it.
- Each flagged expression is a fire-and-forget promise: a listener
`setup()`, an `unlisten.then(f => f())` cleanup, a loader called from an
effect, an upload kicked off from a handler, or an async IIFE. Prefixing
it with `void` marks the ignored promise explicitly, which is the fix
the rule documents.
- It deliberately does not add `.catch()` everywhere. These call sites
already handle failures inside the async function or surface them
through their own state, and empty handlers would hide errors.
- Async IIFEs become `void (async () => …)()`. `if (cond) call()`
becomes `if (cond) void call()`.
- There is no behaviour change. `void` only discards the promise value,
which was already being discarded.
Each insertion is placed at the exact `textRange` start offset of the
SonarCloud finding, with no other edits.
## Testing
- Each changed file parses with `esbuild <file>`, using the same check
as #179.
- I parsed each changed file with the TypeScript 5.9 compiler API
(`createSourceFile`). That found no parse errors, and every inserted
`void` is either a whole expression statement or an arrow-function body
applied to a call, never part of a larger expression.
- `frontend/.eslintrc.json` has no `no-void` rule.
- I did not run the full Nx type-check, lint or test targets, because
`frontend/node_modules` is not installed in my environment. Please rely
on CI.
- No tests are added. Discarding an already-discarded promise value is
not observable, so no test can fail before this change and pass after
it.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Refactor**
* Clarified that asynchronous event listener setup, cleanup, and
window-control initialization run without blocking. Existing
subscription behavior and cleanup conditions remain unchanged; no
user-facing functionality changes.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
FredApps
|
83aad5eb54 |
Exclude TypeScript and JavaScript from Sonar coverage (#2813)
## Summary Adds `sonar.coverage.exclusions=**/*.ts,**/*.tsx,**/*.js,**/*.jsx` to `sonar-project.properties`. There were no existing exclusion entries. The shared SonarCloud workflow only publishes the Rust lcov report (`sonar.rust.lcov.reportPaths=lcov.info`). Artcraft's workflow wrapper does not pass a TypeScript or JavaScript coverage report, so those files can never appear covered. That is why [#2805](https://github.com/storytold/artcraft/pull/2805) and [#2806](https://github.com/storytold/artcraft/pull/2806) fail the quality gate on **new_coverage** (0.0% against the 80% target) after the organisation was recreated. They only add `void` in front of existing TypeScript calls: - #2805: 53 new lines to cover, all uncovered - #2806: 38 new lines to cover, all uncovered [#2807](https://github.com/storytold/artcraft/pull/2807) also touches TypeScript (`void` in `libs/components`) and is listed here for the same exclusion, but its current gate failure is duplicated new code (10.6%), which this PR does not address. Same approach as [anicraft#106](https://github.com/storytold/anicraft/pull/106), which excluded JS, PowerShell and Python for the same reason. filmcraft and lightcraft already exclude `**/tools/**,**/docs/**,**/examples/**`. This PR uses extensions because the uncovered TypeScript and JavaScript live under `frontend/`. No product code changes. No open PR already edits `sonar-project.properties`. ## Testing Visual review of `sonar-project.properties` against anicraft, filmcraft and lightcraft, and of the shared `sonarcloud.yml` coverage inputs. No Rust or frontend tests were run (properties file only). The quality-gate effect on #2805 and #2806 will be visible after this merges and those branches are updated from main. - [x] Clean room: no proprietary code, scripts or assets copied (AGENTS.md) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Chores** * Updated project configuration for coverage reporting. No user-facing changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
FredApps
|
50afb36f9c |
api_clients: use tokio::fs inside async functions (Sonar S7493) (#2803)
## Summary
Replaces blocking `std::fs` calls inside `async fn`s with `tokio::fs` in
four API client crates. SonarCloud reports these as **rust:S7493** ("use
an asynchronous file API instead of this blocking file operation"). This
PR resolves 13 of the 40 artcraft findings.
| Crate | File | Change |
|---------------------|-------------------------------------------------------------|------------------------------------------------------------|
| `artcraft_client` | `endpoints/media_files/*_from_file.rs` (4 files) |
`std::fs::read` → `tokio::fs::read(..).await` |
| `artcraft_client` | `recipes/download_media_file.rs` | `File::create`
+ `write_all` + `flush` → `tokio::fs::write` |
| `artcraft_router` | `generate_video/providers/kinovi/upload.rs` |
`std::fs::read` → `tokio::fs::read(..).await` |
| `midjourney_client` | `credentials/credential_manager.rs`
(`clear_credentials`) | `std::fs::remove_file` →
`tokio::fs::remove_file(..).await` |
| `openai_sora_client`|
`requests/deprecated/job_status/sora_job_status.rs` | `create_dir_all` /
`File::create` + `write_all` → `tokio::fs` |
The workspace `tokio` dependency enables only `macros`.
`artcraft_client` (which had `tokio` only as a dev-dependency) and
`artcraft_router` now declare `tokio = { workspace = true, features =
["fs"] }`. `midjourney_client` and `openai_sora_client` already use
`tokio` with `full`. `Cargo.lock` does not change. In
`save_generations_to_dir`, the blocking `dir.exists()` check before
`create_dir_all` is dropped, because `create_dir_all` already succeeds
when the directory exists.
The behaviour stays the same: same paths, same error mapping, and the
same `NotFound` handling in `clear_credentials`.
## Not included
- `crates/schema/database/sqlite_tasks/src/connection.rs:25` (S7493):
#2360 removes this `remove_file` entirely, so it's left to that PR.
- `crates/desktop/artcraft` (26 findings): handled in a separate PR.
#2365, #2373 and #2785 edit nearby lines in the `*_from_file.rs` upload
files (client builder, idempotency token). None of them touch the
`std::fs::read` lines. Expect at most a trivial rebase.
## Testing
Run locally on Linux x86_64 with stable Rust:
- `cargo check -p artcraft_client -p artcraft_router -p
midjourney_client -p openai_sora_client` passes.
- `cargo test` on the same four crates passes: 2555 + 39 + 16 + 6 unit
tests, with ignored tests left ignored. That includes
`midjourney_client`'s existing
`loads_a_saved_login_and_logout_removes_every_format`, which exercises
the changed `clear_credentials` path.
- `cargo clippy` on these crates reports nothing on the changed lines.
Clippy cannot finish on `main` itself: `enums` fails on
`clippy::deprecated_semver` and `openai_sora_client` fails on
`should_implement_trait` in `task_id.rs`, both pre-existing.
No new test is added. This is a non-blocking I/O refactor with identical
observable behaviour, so there is no test that fails before the change
and passes after it. The other changed paths do network I/O and have no
offline tests. Please rely on CI and the SonarCloud re-analysis to
confirm the S7493 findings close.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Refactor**
* File uploads and downloads, generated image saving, and
credential-file cleanup now use asynchronous file operations.
* When a local file can’t be read during a video upload, the existing
fallback to downloading from the source URL remains in place.
* Existing handling of file-read, write, and removal errors is
preserved, including ignoring missing credential files.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
FredApps
|
c3fa0453a4 |
desktop: use tokio::fs for Sora and Midjourney file I/O (Sonar S7493) (#2804)
## Summary
Replaces blocking `std::fs` calls inside `async fn`s in the desktop
crate with `tokio::fs`. SonarCloud reports these as **rust:S7493** ("use
an asynchronous file API instead of this blocking file operation"). This
PR resolves 17 of the 26 desktop findings.
| File (under `crates/desktop/artcraft/src/services/`) | Findings |
Change |
|----------------------------------------------------------------|----------|----------------------------------------------------------------------------|
| `sora/state/read_sora_credentials_from_disk_with_upgrade.rs` | 10 |
`read_to_string` and `OpenOptions`+`write_all`+`flush` →
`tokio::fs::{read_to_string, write}` |
| `sora/state/initialize_sora_jwt_bearer_token.rs` | 2 |
`std::fs::{read_to_string, write}` → `tokio::fs` |
| `sora/state/sora_credential_manager.rs` (`call_sentinel_refresh`) | 2
| `OpenOptions`+`write_all`+`flush` → `tokio::fs::write` |
| `midjourney/utils/download_midjourney_image.rs` | 2 |
`File::create`+`write_all` → `tokio::fs::write` |
| `midjourney/threads/upload_midjourney_batch.rs` | 1 |
`std::fs::remove_file` → `tokio::fs::remove_file(..).await` |
`tokio::fs::write` creates or truncates the file and writes everything.
That is the same as the
`OpenOptions::new().create(true).write(true).truncate(true)` +
`write_all` + `flush` sequence it replaces. The synchronous helpers in
`sora_credential_manager.rs` (`persist_*_to_disk`) are not `async`, are
not flagged, and are left unchanged. The `.exists()` guards are also
left as they are. The desktop crate already depends on `tokio` with the
`fs` feature, so no manifest or lockfile changes.
## Not included (overlap with open PRs)
The remaining 9 desktop S7493 findings are in files that open PRs are
rewriting. They are left alone to avoid conflicting changes:
- `core/utils/simple_http_download.rs`,
`sora/.../handle_successful_generations.rs`,
`worldlabs/threads/worldlabs_marble_task_polling.rs`: #2777 (fail
downloads on non-2xx responses).
- `core/threads/.../fal/handle_fal_complete.rs`: #2550, #2373.
- `core/utils/auto_download.rs`: #2785, #2686, #2393.
The api_clients findings are in #2803.
## Testing
- Each changed file parses with `rustfmt --check`. The formatting diffs
it reports already exist on `main`, and no files were reformatted.
- **I could not build the desktop crate locally.** `cargo check -p
artcraft` stops at `libdbus-sys` because the build machine lacks the
GTK, WebKit and D-Bus system libraries Tauri needs. Please rely on CI
for compilation before marking this ready.
- No new tests are added. This is a behaviour-preserving switch to
non-blocking I/O, so no test fails before the change and passes after
it, and these paths have no existing offline tests.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Chores**
* Updated background handling for saved images, checkpoints, and account
credentials. Existing cleanup, credential processing, and error
reporting behavior remain unchanged.
* These maintenance updates do not change the app’s available features
or how people interact with it.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
FredAppsandFredApps
|
1b99c184ef |
ci: one macOS and one Windows release build per branch at a time, with job time limits (#2809)
This changes release workflows (house rule 9). It only adds limits, but
it needs a maintainer's look before merging.
## Problem
`artcraft-macos-publish.yml` and `artcraft-windows-publish.yml` have no
`concurrency` and no `timeout-minutes`. `artcraft-linux-publish.yml`
already has both.
- **Overlapping builds:** two pushes to `release` or `artcraft-release`
in quick succession start two builds of the same platform side by side.
Both create or update the same `artcraft-v<version>` draft release
through `tauri-action`. Nothing stops this today. It happens whenever a
second push lands while a 10-36 minute build is still running. (The two
runs on 2026-11-07 were 35 minutes apart and did not overlap.)
- **Hung builds:** a build that hangs runs until GitHub's 6-hour
default. The Windows run on 2026-06-12 did exactly that and was
cancelled at 360 minutes. Successful builds take far less:
| workflow | successful builds, last 15 runs (2026-04 to 2026-11) |
|---|---|
| macOS publish | 10-22 min |
| Windows publish | 17-36 min |
## Change
Two files, limits only. Triggers, steps, secrets, signing, release
settings and action versions are unchanged.
- **Concurrency:** `concurrency: { group: macos-release-${{ github.ref
}}, cancel-in-progress: false }`, and likewise `windows-release-…`. This
is the same pattern as `artcraft-linux-publish.yml` (`linux-release-${{
github.ref }}`).
- A release build that is already running is **never cancelled**, since
it may already have uploaded to the draft release.
- One GitHub behaviour to be aware of: a concurrency group holds one
running and one pending run. If a third push lands while one build runs
and another waits, GitHub replaces the waiting run with the newest. So
the next build is always of the latest commit on that branch, and an
intermediate commit may not get its own build. For a release branch I
think that is what we want. If every commit must get a build, this PR
shouldn't be merged as it is; tell me and I'll change it.
- The groups are per platform and per branch, so macOS, Windows and
Linux still build in parallel with each other.
- **Timeouts:** `timeout-minutes: 60` for macOS and `90` for Windows.
That is about 2.5-3× the slowest successful build, and Windows matches
the Linux build job's 90. A hang now stops after an hour or an hour and
a half instead of six.
Not changed: `dtolnay/rust-toolchain@stable` and
`tauri-apps/tauri-action@v0` are still unpinned tags. Pinning them would
change which action code builds a release, so it should be a separate
decision.
## Verification
- `actionlint` is clean on the three publish workflows.
- These workflows only run on pushes to `release`, `artcraft-release`,
`artcraft-macos` and `artcraft-windows`, so this PR's checks don't
exercise them. The new settings take effect on the next release push.
- I didn't trigger a release build.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Chores**
* Release builds for macOS and Windows now finish in progress without
being cancelled by newer runs. When multiple builds are queued for the
same branch or ref, only the newest pending build is kept.
* Publishing jobs now have maximum run times of 60 minutes for macOS and
90 minutes for Windows.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: FredApps <FredApps@users.noreply.github.com>
|
||
|
FredApps
|
dc6006f625 |
Copy the shared house rules into AGENTS.md (#2749)
## Summary - Puts the shared house-rules blocks at the top of `AGENTS.md`, copied from craft-contrib `dde09ea31e2e457f5b4653a6e76f7cdf55944bdf`. - Adds `.github/workflows/house-rules.yml`, which calls the shared check. Artcraft is the Tauri product, so it gets the core block only. The Rust rules are for native Craft apps and shared Rust libraries. - `CLAUDE.md` was a symlink to `AGENTS.md` and is now `@AGENTS.md`. - No local paragraph was removed. The Tauri and TypeScript description stays. ## Sizes - Core block: 3,052 bytes. - Root `AGENTS.md`: 6,934 bytes. |