10 Commits

Author SHA1 Message Date
bahdotsh
5ac563d213 Release 0.8.0
wrkflw@0.8.0
wrkflw-evaluator@0.8.0
wrkflw-executor@0.8.0
wrkflw-github@0.8.0
wrkflw-gitlab@0.8.0
wrkflw-logging@0.8.0
wrkflw-matrix@0.8.0
wrkflw-models@0.8.0
wrkflw-parser@0.8.0
wrkflw-runtime@0.8.0
wrkflw-secrets@0.8.0
wrkflw-trigger-filter@0.8.0
wrkflw-ui@0.8.0
wrkflw-utils@0.8.0
wrkflw-validators@0.8.0
wrkflw-watcher@0.8.0

Generated by cargo-workspaces
2026-04-22 00:22:41 +05:30
Gokul
ebe8083f33 feat(ui): ship screens 4, 7, 8 and the Tweaks overlay (#105)
* feat(ui): ship screens 4, 7, 8 and the Tweaks overlay

PR #104 landed three screens from the Claude Design handoff
(Dashboard, Live Run, Step Inspector) and deliberately punted the
rest because "a UI without backing data is worse than no UI." Fair.
Picked up the remaining screens that actually *have* backing data
today, and left the one that doesn't alone.

Three new top-level tabs:

  - DAG (tab 3) — full topological view of the selected workflow,
    `g` toggles between the spatial column layout and a stage-list
    layout. Reuses the existing `dag::topo_levels` so what you see
    here is exactly what the mini-DAG in the Execution tab shows,
    just bigger.
  - Trigger (tab 5) — form + live curl preview, dispatches through
    `wrkflw_github::trigger_workflow` or
    `wrkflw_gitlab::trigger_pipeline`. `p` flips platform, `+` adds
    a k=v input, `Tab` walks fields, `Enter` dispatches, `c` dumps
    the curl into the log buffer because integrating with every
    terminal's clipboard is not a fight I want to pick today.
  - Secrets (tab 6) — reads SecretConfig::default(), shows the
    providers the user actually has wired, detail pane with `m` to
    toggle the value mask. Runtime pills on the side share state
    with the existing runtime cycler. We do *not* render a list of
    individual secret keys; SecretManager doesn't expose a
    list_known_keys() and I refuse to fake one.

Step Inspector's Matrix sub-tab now calls
wrkflw_matrix::expand_matrix and renders up to 32 combos with an
overflow badge. Status glyph inherits from the parent job because
per-combo status isn't tracked end-to-end yet — a future executor
change to surface it will drop straight into this render.

Tweaks overlay (`,` key) ports the design's floating TweaksPanel.
Only one knob: accent color, five slots matching the design
palette. theme::set_accent_override plumbs the chosen color into
a thread-local the brand mark and focused borders consult every
frame. Dropped theme (dark/light) and density from the panel
because they'd be dead toggles today — the rule is the rule.

What's still not in: screen 6 (Run history + diff). That one
needs a storage layer we don't have — no run persistence, no
serialized run records, no diff engine. Building a UI on top of
nothing would be exactly the thing PR #104 warned against. Left
for a separate PR that starts from storage.

Verified: cargo build --workspace, cargo clippy --workspace
--all-targets -- -D warnings, cargo test --workspace (734 tests,
all green), cargo test -p wrkflw-ui (25/25). Clean.

* docs(examples): add ui-demo workflow set for the new screens

The new UI (screens 4, 7, 8 + Tweaks) shipped without a dedicated
set of workflows to poke at it. The existing tests/workflows/
fixtures are fine for the parser, but they don't have the shapes
you actually want to eyeball — there's no clean diamond, no wide
multi-stage fan-out, no workflow_dispatch with a mix of input
types.

So you end up firing up the TUI and squinting at whatever happens
to be checked in. Not great.

Add examples/ui-demo/ with one workflow per screen/feature: a
diamond and a wide fan-out DAG for screen 4, a workflow_dispatch
with choice / string / bool inputs for screen 8, a matrix with
include/exclude plus three scopes of env for the Step Inspector,
a secrets + services + container job for screen 7, a multi-event
trigger for the `d` diff filter, and a mixed-status workflow so
the status badges aren't stuck on all-green.

Every file carries a header comment explaining which screen it
targets and what to look at. All eight parse clean under
`wrkflw validate`.

* fix(ui): clean up loose ends around the new tabs and Tweaks overlay

The DAG / Trigger / Secrets tabs and the Tweaks overlay shipped
with the edges unfinished. A review turned up a pile of
correctness and consistency bugs that individually didn't look
like much but together amounted to "the tabs work but everything
around them lies."

The status bar's context_hints still only knew about the old
4-tab layout — tab 2 was now DAG but showed Logs hints, tab 3
was Logs but showed Help hints, tabs 4-6 went blank. The Help
overlay still said "1-4 / w,x,l,h" and listed four tabs. Every
keyboard-help surface was advertising the wrong layout.

The Tweaks overlay's doc comment claimed it was modal — "the
overlay wins" — but the match arm only handled Esc / \`,\` /
\`a\`/\`A\`. Everything else (q, d, 1-7, ?) fell through to the
global handler. The comment was aspirational. The code was not.

trigger_dispatch had no in-flight guard. Hit Enter twice
quickly, fire two workflow_dispatch requests. Against real
repos. This is the kind of "oops we double-ran the deploy" bug
that nobody enjoys.

render_target_pane called resolve_target on every frame, which
shelled out to \`git remote get-url origin\` + \`git symbolic-ref\`
+/- \`git rev-parse\` *on every frame*. The event loop polls at
50ms, so sitting on the Trigger tab produced ~40-60 git
subprocesses per second. Per second.

And the DAG tab cheerfully printed fabricated column labels —
["setup", "lint", "build", "test/docs", …] — positionally
assigned to topological levels, with no relationship to actual
workflow content. A release pipeline's third layer would read
"build" even when it was \`deploy\`. Exactly the "UI without
backing data" footgun the previous PR was supposed to avoid.

The fix is mechanical:

  - Rewrite status_bar hints and the Help overlay for 7 tabs.
  - Make the Tweaks overlay actually modal — unmatched keys
    \`continue\` instead of falling through.
  - Add a trigger_in_flight AtomicBool; set before spawn,
    cleared by the spawned task regardless of outcome.
  - Cache the resolved target on App; invalidate on platform
    toggle. No more subprocess storm.
  - Replace the hand-rolled JSON escaper with serde_json, which
    has been in the tree the whole time. While at it,
    shell-escape the GitHub ref and double-quote-escape the
    GitLab k/v pairs so the curl preview is copy-paste safe.
  - Drop the fabricated stage labels; print "stage N" because
    that's the honest thing.
  - Sort the include-only matrix column tail so HashMap order
    can't cause visual jitter between frames.
  - switch_tab re-masks secrets when leaving the Secrets tab.
    The doc had claimed this all along; now it's actually true.
  - Empty env vars no longer count as a set auth token.
  - trigger_input_cursor is Option<usize>, not a usize::MAX
    sentinel. Please don't use usize::MAX for "not set" when
    Option exists.

Sixteen new unit tests cover the new helpers and edge cases.
Full workspace build + clippy -D warnings + test suite clean.

* fix(ui): stop the Trigger tab from smearing the TUI on dispatch

The Trigger tab shipped in the previous commit calls into
wrkflw_github::trigger_workflow and wrkflw_gitlab::trigger_pipeline
from a tokio::spawn. Both of those are happily scattering println!
and eprintln! calls — "Repository: x/y", "Using branch: main",
"Workflow triggered successfully!", you name it — straight to the
stdout that ratatui is rendering into.

The TUI owns the terminal. The SDK crates write to it anyway. The
result is a frame full of garbled half-redrawn panels the moment the
user hits Enter. wrkflw_logging::set_quiet_mode(true) is already
being armed at TUI start, but it only gates the logging subsystem —
raw println! skips right past it.

Route every one of those prints through wrkflw_logging::{info,warning}
so the quiet-mode flag actually does the thing it says on the tin.
Adds wrkflw-logging as a dep to both crates, which is fine — it's
where those messages belonged in the first place.

While at it, fix the rest of what was broken around the Trigger tab:

The GitLab curl preview was cheerfully lying. It pointed users at
/trigger/pipeline with a PRIVATE-TOKEN header, which is a
combination that has never worked — /trigger/pipeline wants a
trigger token, and the real dispatcher calls /pipeline with a JSON
body. Copy-paste the preview and you'd get a 401, maybe a 404.

Rebuilt both previews off the same request shape the dispatcher
actually sends. GitHub and GitLab bodies are now built via
serde_json end-to-end (github_dispatches_body, gitlab_pipeline_body),
shell-escaped with a single helper, and the URL uses the resolved
repo from the target cache instead of <owner>/<repo> / <id>
placeholders.

The dispatch outcome used to reach the user only via the Logs tab —
the one on Trigger heard nothing back. Added a mpsc channel so the
spawned task reports success/failure back to the main event loop,
which mirrors it onto the status bar. Drain runs once per tick next
to the existing execution-result drain.

The DAG graph view was splitting the area into equal-ratio columns
with no floor, so on anything narrower than 18 * stages cells the
box-drawing characters wrapped and the whole graph looked like
someone spilled coffee on it. Clamped columns to a fixed 18-cell
width, render "+N more stages" in the overflow slot, and nudge
users toward g for list view. Dropped the duplicated "L1 stage 1"
header while I was in there.

AccentScope wraps the thread-local accent override in an RAII guard
so it lives for exactly one frame. A thread-local that nobody ever
clears is a booby trap for whoever adds the second theme knob.

The old trigger-dispatch test spawned a real tokio task that called
the real wrkflw_github::trigger_workflow. If GITHUB_TOKEN happened
to be set in the environment — hello, CI — it would fire a real
workflow_dispatch against whatever repo git remote resolved to. That
is not a test. Replaced with a synchronous guard test that pre-arms
the in-flight flag and asserts rejection. No spawn, no runtime, no
network. New tests cover the preview bodies, split_slug, and the
outcome-drain mapping.

* fix(ui): sand off the UX edges flagged in the screens-4-8 review

A review pass over the new DAG / Trigger / Secrets / Tweaks tabs
turned up a small but consistent failure mode: several bits of the
UI were quietly lying to the user, and a couple of input paths
were eating keystrokes that mattered.

The Secrets tab's header badge advertised "providers configured",
but the tab reads `SecretConfig::default()` unconditionally — no
real config file is loaded yet. A user who had carefully
customised their secrets config would see the two hard-coded
defaults and a badge telling them everything was wired up. That's
exactly the "UI without backing data" footgun PR #104 set out to
ban. Rename the badge to "default providers" and update the file
header so the caveat doesn't get lost the next time someone skims
the module.

The Trigger tab's edit-mode key handler returned `false` for Up /
Down / Left / Right, which meant directional keys fell through to
the global handler and called `trigger_tab_prev/next_workflow`.
So a user typing `env=prod` who hit ↓ to correct the row above
had the *workflow about to be dispatched* silently changed
underneath them. Consume the arrows in edit mode — no-op for now;
a future enhancement can route them to prev/next field if we want
that.

`resolve_trigger_target`'s error branch admitted the repo was
`<unresolved>` while cheerfully inventing `default_branch: "main"`.
Two fields, two voices, one warn badge the user might or might
not notice. Mark the branch `<unresolved>` too so the
un-resolution story is consistent. The dispatcher has its own
`get_repo_info` call in the hot path, so this is strictly a
display-honesty fix.

The Tweaks overlay was modal to the point of swallowing `q`. Quit
is universally modal-safe in this TUI; silently eating it is a
discoverability trap. Let it through.

While at it, `field_row_hl` was taking `hint: String` where the
sibling `field_row` took `&str`. Match the sibling. Tiny thing,
but the kind of inconsistency that compounds.

Two regression tests pinned to the arrow-swallow path and the
resolve-error shape, so a future refactor can't quietly re-open
either footgun.

* fix(ui): clear out the self-review pile for screens 4-8

A review of the new DAG / Trigger / Secrets tabs and the Tweaks
overlay turned up a depressingly long list of "works in the happy
path, lies to the user otherwise" bugs. Dealing with them in one
sweep rather than trickling out a dozen one-liner fixes.

The headliner: the Trigger tab renders a "Branch / ref" row as if
it's editable, but *no keystroke* ever wrote into `trigger_branch`.
The dispatcher and the curl preview both honoured the field — the
field just had no input path. A user wanting anything other than
the git-resolved default was out of luck.

Add a `b` shortcut to focus the branch row, extend
`trigger_handle_input_key` to route into `self.trigger_branch`
when focused, and rework `trigger_tab_next_field` so Tab cycles
Branch → Input(0).key → Input(0).value → … → wrap. Three tests
pin the edit path, the Esc-clears-override path, and the mutual
exclusion between branch edit and input edit.

`state_for_job` in the DAG tab was using `std::ptr::eq` against
`app.workflows` elements to figure out which workflow was the
current execution, with `usize::MAX` as the fallback. It worked
today because the render path gets its `&Workflow` from the same
Vec, but it was one refactor away from returning `Some(usize::MAX)`
and silently matching against `app.current_execution`. Thread the
index the caller already has and compare that. No more pointer
identity.

The Secrets tab's "reveal / mask" toggle was theatre. There are no
actual secret values at this layer — the "value" being masked was
the *provider's source descriptor*, i.e. a filesystem path or an
env-var prefix. Bullet-masking a filename protects nothing and
teaches the user that "masking" means something it doesn't. Rip
out `secrets_reveal`, the `m` handler, and the `switch_tab`
auto-revert. When per-secret values land the toggle can come back
attached to something it can legitimately hide.

The Trigger tab's curl preview was building a `format!` string
with Rust's backslash-newline line continuation (which *strips*
the backslash) and then splitting the result on `" \\"`. That
split never matched anything, so the preview rendered as one long
line relying on terminal wrap. Rebuild with explicit ` \\\n` joins
and split on `\n` at render time.

`format_yaml_scalar`'s fallback runs non-scalar matrix values
through `serde_yaml::to_string`, which for sequences and maps
returns multi-line YAML. That got shoved into a single ratatui
Span. Collapse embedded newlines to a visible ` · ` separator.

`drain_trigger_outcomes` overwrote `status_message` on every
iteration — if two outcomes arrived in the same tick (rare, but
the in-flight guard could relax later) the first was silently
lost. Log each drained outcome to `self.logs` as the durable
record, and have the status bar prefer errors over later
successes in the same drain.

Tab indices: stop pretending `2, 3, 4, 5, 6` are self-documenting.
Introduce `TAB_WORKFLOWS` / `TAB_EXECUTION` / … constants next to
`TAB_LABELS` and use them everywhere. `switch_tab` used to define
a local `SECRETS_TAB = 5` that the rest of the file promptly
ignored. Please don't do that.

While at it, widen the accent override plumb-through. The Tweaks
panel claimed to recolour the UI but only `block_focused` and the
title_bar brand were actually consulting `current_accent()` — the
other 14 `COLORS.accent` call sites stayed on the static palette.
A user flipping accent saw the focused borders change and nothing
else, which looks broken. Point all accent readers at the
thread-local override.

Minor: hoist \`provider_entries()\` to one call per frame (was 3×),
dedupe the \`truncate(name, 12)\` call in the DAG graph (was
computed twice per node).

cargo fmt + clippy -D warnings clean; 48/48 UI tests green.

* fix(ui): close the screens-4-8 review debt

A review pass over the screens-4-8 branch turned up three things
that the earlier "done" sweep in 09da60a should have caught. Dealt
with them in one commit rather than three.

The accent plumb-through was overstated. 09da60a claimed all accent
readers went through the thread-local override, but
`BadgeKind::Accent` still resolved to `COLORS.accent`,
`brand_style`/`label_style` were dead but still reaching for the
static palette, and `progress_bar`'s default was hardcoded cyan. A
user flipping to Amber saw focused borders change and the "d Toggle
diff filter" chip stay exactly the same — the same "feature lies
about what it does" failure mode the review was trying to plug.
Route the badge variant through `current_accent()`, delete the two
dead helpers, and switch the progress bar default over.

The curl preview was shell-escaping the JSON body but interpolating
owner/repo/workflow-name raw into the URL path. Those three come
from `git remote` + the local filesystem — nothing stops a workflow
filename like `x;rm -rf /.yml` from producing a copy-paste line that
does exactly what it says on the tin. Pipe every path segment
through `urlencoding::encode`. Not a high-probability hazard, but
the preview is *explicitly* there to be copy-pasted, so unquoted
user input is an unforced error.

Same 09da60a commit introduced TAB_WORKFLOWS..TAB_HELP as canonical
indices and swept the codebase — except for
`status_bar::context_hints`, which was still matching bare 0..6.
Port it. Please don't use a bare integer for a tab index when the
constant is right there.

While at it: `trigger_tab_copy_curl` was pushing a multi-line curl
string as one log entry (embedded newlines, rendered garbled);
successful dispatches now set a Success (green) status message
instead of Info (cyan) so they're visually distinct from the
"queued" messages that share the Trigger tab's accent; renamed a
shadowed `truncated` local in `dag_tab` that was doing double duty
as both the overflow-stage count and the char-truncated job name.
Four new tests pin the curl fallback placeholders, the URL-encoding
regression, input-row Enter fallthrough, and AccentScope dropping
the thread-local on scope exit.

cargo fmt + clippy -D warnings + full workspace test suite clean.

* fix(ui): close the remaining review debt on screens 4-8

A review pass over the screens-4-8 branch turned up three things
that survived the last round. Dealing with them in one commit.

It turns out the curl preview and the real dispatcher were
computing the workflow URL segment *differently*. The preview
stripped `.yml`/`.yaml` and url-encoded the full user input —
subdir and all. The dispatcher ran `Path::new(name).file_stem()`
(which drops the subdir) and then unconditionally re-appended
`.yml`, turning a `ci.yaml` workflow into a `ci.yaml.yml` URL
that has never worked.

So the preview was lying about what Enter would send, the
dispatcher had a latent `.yml.yml` bug for any input that already
carried the extension, and `.yaml` files silently became `.yml`
URLs on dispatch.

Consolidate both paths behind a single
`wrkflw_github::workflow_dispatch_path_segment` helper. Drops any
subdir prefix, preserves an existing `.yml`/`.yaml`, appends
`.yml` only when absent. The preview and the dispatcher now
produce byte-identical URL segments for the same input — and the
`.yaml.yml` footgun is gone as a side effect.

The second one: `a` (select-all) and `r` (queue+run) were gated
only on `!app.running`, not on the active tab. Pressing `a` on
the DAG or Trigger or Secrets tabs — outside edit mode — would
silently flip every workflow to `selected` behind the user's
back. Same story for `r` queuing an execution. Add the tab gate.
Please don't do that.

The third one: `trigger_in_flight` was cleared by a trailing
`store(false)` at the end of the spawned dispatch task. Fine on
normal return. A panic inside reqwest or either SDK would skip
right past it, strand the flag at `true`, and lock the Trigger
tab into a permanent "already in flight" state until the TUI
restarts. Wrap the flag in an `InFlightGuard` RAII so Drop is
what actually clears it. Normal return, early return, *and*
unwinding all land it back at `false`.

While at it: dropped the `DEBUG: Shift+R pressed` log spam that
was sprinkled through the reset handlers, deleted the now-dead
`strip_yaml_suffix` helper, and cleaned up a stray
`test_edge_cases.rs` debug script that had been sitting in the
repo root.

Eight new tests across the two crates cover the shared URL
helper, the preview/dispatcher identity, \`.yaml\` round-trip, and
the in-flight guard on both normal drop and \`catch_unwind\` panic
paths.

cargo fmt + clippy -D warnings + full workspace test suite clean.
2026-04-21 22:47:50 +05:30
Gokul
8a8d7e5eec fix: resolve correctness, security, and parsing bugs across codebase (#73)
* fix: resolve 10 bugs found during full codebase review

- Fix memory leak from Box::leak() in status bar render loop
- Fix AES-GCM nonce reuse vulnerability in encrypted secret storage
- Fix Default impl for EncryptedSecretStore that discarded encryption key
- Fix early return in list command that prevented GitLab pipeline listing
- Fix double validation call in validate_github_workflow
- Wire --show-action-messages CLI flag through ExecutionConfig
- Add serde(rename = "if") to GitLab Rule if_ field for correct deserialization
- Fix potential panic on multibyte paths in workflow tab path shortening
- Include involved job names in circular dependency error messages
- Improve cron syntax validation to check value ranges, steps, and expressions

* fix: address PR review feedback

- Remove dead `_nonce` parameter from `EncryptedSecretStore::from_data`
- Add clarifying comment for inverted show/hide action messages mapping
- Add comprehensive cron validation tests (valid expressions, out-of-range
  values, wrong part count, invalid steps, invalid ranges, edge cases)

* fix: resolve 27 bugs found during full codebase verification

A thorough manual verification of every feature uncovered a
*remarkable* collection of bugs hiding in plain sight. The
highlights:

The `strategy.matrix` YAML structure was never parsed. The Job
struct had `matrix` at the top level, but GitHub Actions nests it
under `strategy.matrix`. Serde silently ignored the `strategy`
key, so matrix expansion code existed but could never run. For
absolutely no reason. Introduce a proper `Strategy` struct and
wire it through the executor.

The Step struct was missing `if`, `id`, `working-directory`,
`shell`, and `timeout-minutes` fields. Step-level conditionals
were silently dropped — every step always ran regardless of its
`if` condition. While at it, `continue-on-error` was in the
struct but had no serde rename and was never checked during
execution. Fix all of that.

The validator cheerfully reported cyclic `needs` dependencies as
"Valid". Add DFS cycle detection so `A -> B -> C -> A` is caught
at validation time instead of blowing up at execution time.

Five of eight GitLab CI test fixtures failed to parse because the
model was too rigid: `extends` only accepted arrays (not strings),
`variables` rejected integers, `cache.key` rejected structured
formats, and `script` rejected single strings. Add custom
deserializers following the existing codebase pattern.

The GitHub trigger function leaked the auth token via curl process
arguments visible in `/proc/[pid]/cmdline`. Replace with reqwest,
matching the pattern already used elsewhere. Also add symlink and
path traversal protections in the executor.

Other fixes: hardcoded matrix variable stripping replaced with
proper substitution, `show_action_messages` wired through TUI,
dead `if true {}` removed, default branch detection uses remote
HEAD instead of current branch, cron validator accepts named
days/months, reusable workflow ref validation loosened from OR
to AND, matrix include entries merge into all matching combos.

* fix: harden step-level evaluation, volume checks, and add tests

The PR review turned up a few things that needed fixing before this
was actually ready.

The step-level `if` condition evaluator was silently reusing the
job-level `evaluate_job_condition` function, which knows nothing
about step-scoped expressions like `steps.<id>.outcome`, `success()`,
`failure()`, `always()`, or `cancelled()`. These would fall through
to the generic "unknown condition" path without so much as a warning.
Now they're detected early, a warning is logged, and they default to
true — which is at least *honest* about the limitation.

The volume path traversal check (`..`) was applied to the entire
volume spec string, meaning a perfectly legitimate container path
like `/safe/host:/container/..weird` would get rejected. The check
now only inspects the host path component after splitting on `:`,
which is the part that actually matters for traversal attacks.

While at it, renamed the awkwardly-named `step_name_for_skip` to
just `step_name` in `execute_matrix_job` for consistency with
`execute_job`, and added a BREAKING_CHANGES.md documenting the
EncryptedSecretStore serialization format change.

Added 19 new tests covering matrix include/exclude merge semantics,
step condition evaluation for unsupported expressions, volume path
traversal edge cases, and continue-on-error + step-level if parsing.

* fix: correct condition defaults, path traversal check, and null variable handling

The previous commit defaulted *all* unsupported step-level
condition functions (failure(), cancelled(), always(), success())
to true. It turns out that defaulting failure() and cancelled()
to true is semantically wrong — it means steps guarded by
`if: failure()` will *always* run, even when nothing failed.
That's not a feature, that's a bug.

Default each function to its most likely state: always() and
success() return true, failure() and cancelled() return false.
Not perfect (we still can't track actual step outcomes), but at
least we're not silently running cleanup steps on every build.

The path traversal check was using `contains("..")` which is a
substring match. A directory literally named `..hidden` would
get rejected. Use Path::components() to detect actual ParentDir
components instead of playing string matching games.

While at it, fix deserialize_variables in the GitLab models to
handle YAML null values as empty strings instead of producing
"~\n". Also trim the catch-all serialization output.

* fix: correct cycle detection, condition evaluation, and matrix continue-on-error

The DFS cycle detector in `dfs_detect_cycle` had a genuinely nasty bug:
when a cycle was found, it returned early *without popping itself from
rec_stack*. This left stale entries that corrupted the stack for
subsequent DFS traversals. Net result: cross-edges to already-visited
nodes would be falsely reported as cycles. A→B→A is a cycle, but
D→E→A is just a cross-edge. The old code couldn't tell the difference.

Fix this properly by introducing a separate `in_stack` HashSet for O(1)
membership checks, while keeping the Vec for path reconstruction. Both
are now correctly cleaned up — no early returns skip the cleanup.

While at it, `execute_matrix_job` was silently ignoring `continue-on-error`
on the Err branch. The non-matrix `execute_job` handled it correctly,
but the matrix path would just abort the entire job. Copy-paste bugs
are fun like that. Let's fix that.

The `evaluate_job_condition` status function handling was doing sequential
`contains()` checks with early returns, which meant compound expressions
like `failure() || success()` would match `failure()` first and return
false. Now we scan for all status functions in one pass and pick the
most permissive default when positive functions are present.

Also: `convert_yaml_to_step` was hardcoding `None` for `if_condition`,
`id`, `working_directory`, `shell`, and `timeout_minutes` despite the
YAML potentially having them. And `is_valid_cron_atom` was rejecting
valid POSIX cron syntax like `5/2`.

* refactor(executor): extract step guards into shared helper, fix steps.* default

The step-level if-condition check and continue-on-error handling was
copy-pasted between execute_job and execute_matrix_job with subtly
different control flow — one sets job_success=false and breaks, the
other returns Ok(JobResult{Failure}) immediately. Two copies of the
same logic that *already* disagree is not redundancy, it's a bug
waiting to happen. Let's fix that.

Extract run_step_with_guards() that encapsulates the if-condition
evaluation, execute_step call, and continue-on-error wrapping into
a single StepOutcome enum. Both job execution paths now call this
shared helper.

While at it, fix the condition evaluator defaulting bare steps.*
references to true — "steps.build.outcome == 'failure'" should
*not* optimistically run the step. Now only always() and success()
default to true; everything else (bare step refs, failure(),
cancelled()) conservatively defaults to false.

Also add serde alias "matrix" on Job.strategy so old workflows with
flat matrix: at job level still parse, and document the intentional
or_insert_with in matrix include merging per GitHub Actions spec.

* fix: clean up review findings in step guards, secret store, and test fixture

The PR review flagged three issues worth fixing before merge.

First, run_step_with_guards had a bogus StepStatus::Skipped check
in the abort_job logic. The condition tested for Failure *or*
Skipped, then only actually aborted on Failure — meaning the
Skipped branch did nothing except confuse anyone reading the code.
Simplify to just check Failure directly.

Second, EncryptedSecretStore::from_json would silently fail with a
generic serde error when fed the old serialization format (which
had a shared top-level nonce field). Now it detects the old format
by checking for the "nonce" key and returns a clear error pointing
at BREAKING_CHANGES.md. Added a test for this.

Third, tests/workflows/continue-on-error-test.yml was an orphan
fixture — nothing referenced it. The same content is already
tested inline by parse_continue_on_error_workflow in the parser.
Removed it.

* fix: correct cron day-of-week range, steps. false positive, and Step boilerplate

Three issues from PR review, all straightforward:

The cron validator was rejecting day-of-week value 7, which is a
perfectly valid Sunday alias in both POSIX cron and GitHub Actions.
The max was 6 when it should be 7. The named-value resolver guard
also needed updating from `max == 6` to `max >= 6` so named days
still resolve correctly with the wider range.

The `evaluate_job_condition` heuristic for detecting `steps.*`
references was using a bare `contains("steps.")`, which means an
env var like `env.MY_STEPS_COUNT` would falsely trigger it and
short-circuit to false. Now we check that the character before
"steps." is either start-of-string or non-alphanumeric. Not a
full expression parser, but it stops the obvious false positives.

While at it, add a `Step::with_run` constructor so the GitLab
converter doesn't need three identical 12-field struct literals
that silently break every time someone adds a field to Step.

* fix: harden steps. boundary check, document condition semantics, dedup cycles

The steps. word-boundary heuristic in evaluate_job_condition was
checking for alphanumeric characters before "steps." to avoid false
positives on env vars like "env.MY_STEPS_COUNT". It turns out that
underscore is *not* alphanumeric, so "env._STEPS_CHECK" would
incorrectly trigger the step-reference path and return false.

While at it, the always() && failure() compound expression returning
true got a proper comment explaining *why* that's intentional — we
lack step-status context locally, so we'd rather over-run than
silently skip steps. Not ideal, but honest.

The DFS cycle detector in detect_cyclic_needs could report the same
cycle multiple times depending on HashMap iteration order. Normalize
cycles by rotating the node list to start at the lexicographically
smallest node, then deduplicate via a HashSet. Same cycle from
different entry points now gets reported exactly once.

* fix: squash review nits — double parse, clippy warnings, lost flag

Three leftover issues from the codebase review PR:

The from_json() deserialization was parsing the JSON *twice* — once
into serde_json::Value to sniff for the old nonce field, then again
from the raw string into the actual struct. Parse once, use
from_value() on the already-parsed Value. Not rocket science.

The cycle detector had two clippy warnings: .iter().cloned().collect()
on a slice (just use .to_vec(), please) and .min_by_key() cloning a
double reference instead of comparing properly. Switch to .min_by()
with an explicit cmp.

The show_action_messages flag was being silently dropped in
execute_workflow_cli — hardcoded to false regardless of what the user
asked for. Propagate it through the function signature and the TUI
fallback path so it actually does something.
2026-04-01 23:06:48 +05:30
bahdotsh
b1cc74639c version fix 2025-09-05 08:22:15 +05:30
bahdotsh
51a655f07b version fixes 2025-08-28 12:56:05 +05:30
bahdotsh
960f7486a2 Release 0.7.0
wrkflw@0.7.0
wrkflw-evaluator@0.7.0
wrkflw-executor@0.7.0
wrkflw-github@0.7.0
wrkflw-gitlab@0.7.0
wrkflw-logging@0.7.0
wrkflw-matrix@0.7.0
wrkflw-models@0.7.0
wrkflw-parser@0.7.0
wrkflw-runtime@0.7.0
wrkflw-ui@0.7.0
wrkflw-utils@0.7.0
wrkflw-validators@0.7.0

Generated by cargo-workspaces
2025-08-13 18:07:11 +05:30
bahdotsh
58de01e69f docs(readme): add per-crate READMEs and enhance wrkflw crate README 2025-08-12 15:09:38 +05:30
bahdotsh
537bf2f9d1 chore: bump version to 0.6.0
- Updated workspace version from 0.5.0 to 0.6.0
- Updated all internal crate dependencies to 0.6.0
- Verified all tests pass and builds succeed
2025-08-09 17:46:09 +05:30
bahdotsh
f0b6633cb8 renamed 2025-08-09 17:03:03 +05:30
bahdotsh
470132c5bf Refactor: Migrate modules to workspace crates
- Extracted functionality from the `src/` directory into individual crates within the `crates/` directory. This improves modularity, organization, and separation of concerns.
- Migrated modules include: models, evaluator, ui, gitlab, utils, logging, github, matrix, executor, runtime, parser, and validators.
- Removed the original source files and directories from `src/` after successful migration.
- This change sets the stage for better code management and potentially independent development/versioning of workspace members.
2025-05-02 12:53:41 +05:30