Skip to content

chore: release v0.5.124#498

Draft
gonewton-deploy[bot] wants to merge 1 commit into
mainfrom
auto-release-0.5.124
Draft

chore: release v0.5.124#498
gonewton-deploy[bot] wants to merge 1 commit into
mainfrom
auto-release-0.5.124

Conversation

@gonewton-deploy

Copy link
Copy Markdown
Contributor

Automated release for version 0.5.124

This PR was created by the auto-release workflow. Merging it brings
the bumped version back into main.

What triggered this release:
Commit: c8851c8
Message: fix: tranche 4 audit remediation — smells & hygiene (spec 074) (#497)

Implements Tranche 4 ("smells & hygiene") of the audit remediation spec
(074, local), following tranches 1 (#491), 2 (#493), and 3 (#495). Ten
work items plus a code-review pass with fixes.

Changes

  • Repo hygiene (H1-H6): untracked 220 accidental UUID-named
    workflow-run artifacts under crates/cli/.newton/, fixed .gitignore's
    .newton/* root-only anchoring (**/.newton/* + a
    !**/.newton/scripts/** exception), deleted three empty/abandoned
    directory skeletons and two throwaway files, deleted-or-implemented
    scripts/linter.sh, fixed architecture.md's operator table and
    docs/realtime.md's stale event-type documentation.
  • Dead clap layer + operator module hygiene (S6/S13): stripped
    unused clap derives/attributes from crates/cli/src/cli/args.rs (kept
    as plain DTOs — real parsing is cli-framework's ArgSpec layer),
    removed the now-unused clap dependency, moved assessment.rs to
    workflow/grading/ (it's a helper library, not an operator).
    gh_authorization.rs was not deleted — investigation showed it's
    load-bearing (GhOperator's real authorization flow, 65 passing tests)
    despite the audit's premise that it was orphaned.
  • Small fixes (P7/P13/P14): a new lint rule warns when
    model_stylesheet.context_fidelity is set to a non-default value that
    has no effect yet; purged the removed monitor CLI command from skill
    metadata; wired the previously-discarded _terminal_stop signal into a
    new WorkflowExecution.terminal_stop field (kept the lossless
    WorkflowExecutionStatus mapping from S11 intact — added a field rather
    than a new status variant).
  • Unified event-sink trait (S7): DbSink/ServerNotifier were
    already consolidated to one WorkflowSink trait with two impls by an
    earlier tranche (premise correction — the audit's "collapse to one
    trait" text no longer matched current code); this item's remaining scope
    was replacing let _ =-swallowed channel-send failures with
    tracing::debug!-logged failures.
  • spawn_blocking for workflow-file I/O (S10): all five
    workflow-files axum handlers (list/get/put/delete/validate) wrap their
    blocking std::fs calls and run_diagnostics' parse/validate/lint in
    tokio::task::spawn_blocking, following the codebase's existing
    outer-JoinError/inner-Result idiom.
  • Typed Finding lifecycle enums (S3): FindingStatus (8 canonical
    CONTEXT.md variants), Severity, and Origin replace String fields
    on FindingItem/CreateFindingBody/PatchFindingBody.
    finding_row_to_item (the store boundary) parses DB strings via
    FromStr, treating an unparseable value as store corruption rather than
    silently coercing it. Deleted the now-redundant FINDING_STATUSES
    allowlist (it only covered 5 of 8 canonical statuses) — invalid values
    are now rejected at the type level. reconcile.rs's is_open_status
    takes &FindingStatus with an exhaustive match, the actual mechanism
    behind "the compiler owns what is_open_status approximates."
  • Route colocation + pagination (S12):
    Product/Component/Repo/ModuleDependency's read routes
    (previously split across dashboard.rs/portfolio.rs from their CRUD
    in catalog.rs) are now colocated under one "catalog" OpenAPI tag.
    Added limit/offset pagination (default 100, hard cap 1000) to the
    previously-unbounded
    kpis/modules/plans/products/components/regressions list
    endpoints named by the audit.
  • Capture-truncation handling + hot-path panics (S15): every
    silently-swallowed stdout/stderr capture write (let _ = write_all(...)) now tracks its failure reason and appends a [capture truncated: <reason>] marker to the artifact file — covering both
    genuine I/O failures and the pre-existing byte-cap cutoff, neither of
    which used to leave any trace. The truncation reason also surfaces on
    the task's own output JSON
    (stdout_capture_warning/stderr_capture_warning) since no per-task
    "warnings" channel exists on ExecutionContext. Replaced the two
    .expect("stdout/stderr must be piped") panics on the subprocess hot
    path with typed AppErrors.
  • Rhai env() opt-in per workflow (S8, part 1):
    ExpressionEngine::default() no longer registers env() — expression
    evaluation is deterministic unless a workflow explicitly sets the new
    settings.allow_env_fn: true. Threaded through all real-execution call
    sites (child_runner.rs, newton run's loader.rs path, newton optimize's workflow driver) while validation/lint/explain paths stay
    deterministic by design.
  • LlmAdjudicator/AgentClient trait injection (S8, part 2):
    ReconcileOperator, GraderAgentOperator, and ChangeRequestOperator
    no longer call aikit_sdk::AgentRunner/Pipeline directly inside
    execute() — a new llm_client.rs module defines the two seams, each
    with a real implementation that moves the prior spawn_blocking body
    unchanged, and each operator gains an additive
    with_adjudicator/with_agent_client constructor (the plain ::new()
    and every existing test are untouched). This is the actual enabler for
    testing B2's failure path (adjudication failure must fail closed with
    zero Finding mutations) — previously only reachable via a real LLM call
    failing.
  • Shared FK-validation service layer (S5): investigation found the
    audit's premise didn't fully hold —
    create_component_db/create_repo_db/create_module_db/create_eval_run_db/create_grade_db
    already validate their FKs transactionally and return a clean 404, so
    the axum handlers and dispatch_data's real write path were never
    actually duplicating broken validation. The one real, still-live
    duplication was narrower: the CLI's --dry-run block hand-rolled its
    own separate copy of the same "does this FK exist" checks. Extracted
    those into crates/backend/src/service.rs (validate_*_fks, five
    resources) and wired --dry-run to call them, collapsing ~115 lines of
    hand-rolled match arms. Deliberately not wired into the real write paths
    (would reintroduce a TOCTOU gap for no behavioral gain).

Review (8-angle, high effort)

8 finder angles (line-by-line, removed-behavior, cross-file tracer,
reuse, simplification, efficiency, altitude, conventions) plus targeted
verification. Fixed on-branch:

  • env() opt-in didn't reach real execution (found by the
    cross-file tracer, independently confirmed by direct code reading):
    apply_default_pipeline — which runs the
    macro-expansion/include_if/template-interpolation transforms — was
    called before the settings-aware ExpressionEngine was built in
    child_runner.rs/optimize.rs, and each of those three transforms
    constructed its own always-deterministic engine. A workflow that opted
    in via settings.allow_env_fn: true would still fail to load if it used
    env() inside a template string, macro argument, or include_if
    condition (as opposed to a task's own $expr param, which worked
    correctly). Threaded allow_env_fn through apply_default_pipeline's
    signature into all real-execution call sites — this also caught a third
    genuine execution path, loader.rs's load_and_lint_workflow (used by
    newton run despite its lint-sounding name), that the original S8 work
    missed. Validation/lint/explain paths explicitly stay deterministic
    (documented at each call site). New regression test proves both
    directions: fails without the opt-in, works with it.
  • Finding enum malformed-value error shape: typing
    CreateFindingBody/PatchFindingBody's status/severity/origin as
    enums moved invalid-value rejection into serde deserialization inside
    axum's Json<T> extractor — before the handler runs, and before this
    API's usual ApiError JSON envelope gets applied. A bad value on POST /findings/PATCH /findings/{id} fell through to axum's default
    plain-text rejection instead. Added AppJson<T>, a drop-in
    FromRequest wrapper mapping JsonRejection to a 422 ApiError,
    adopted by the two Finding handlers that surfaced the bug (kept as a
    reusable primitive; adopting it repo-wide is a documented follow-up, not
    needed by any other endpoint today).
  • /repos and /module-dependencies missing pagination: these two
    endpoints were relocated into catalog.rs by the same S12
    route-colocation work but never got the limit/offset treatment their
    newly-adjacent siblings received. Applied the same pattern.
  • S15 capture-truncation logic duplicated between command.rs and
    sdk.rs
    : a coverage-gap-closing commit had already extracted
    write_capture_chunk inside sdk.rs to deduplicate three copies there,
    but left command.rs's two structurally-identical inline copies
    untouched — confirmed by two independent finder angles, and by git
    history showing the exact same commit introduced both files'
    duplication. Moved the helper to artifacts.rs (which both files
    already import from) so both share one implementation.
  • NEWTON-0011 doc-comment gap: the three new
    FindingStatus/Severity/Origin::as_str() methods had no /// doc
    comments.

Investigated and refuted or deliberately deferred:

  • Four CLI arg-parsing gaps (--port/--format
    silent-default-instead-of-error, --parallel-limit/--timeout/--last
    negative-value integer-wraparound bypassing validation) were flagged by
    two finder angles reading adjacent code for context — all four confirmed
    real, but confirmed via git diff main...HEAD --stat to live in files
    this branch never touches (framework_setup/mod.rs,
    commands/serve.rs, commands/workflow.rs), pre-existing on main
    since tranche 3. Out of scope for this PR; worth a follow-up ticket.
  • A claimed change_request_op.rs severity-fallback behavior change
    (garbage min_severity silently landing on a more permissive rank than
    before) was refuted — the pre- and post-enum fallback both land on rank
    4; the reviewer conflated it with an unrelated function's
    .unwrap_or(2).
  • RealLlmAdjudicator duplicating RealAgentClient's spawn_blocking
    body instead of delegating to it, and workflow_files.rs's 5x-repeated
    spawn_blocking+JoinError triad, are real but lower-severity — left
    as documented follow-ups rather than expanding this PR further.
  • The Finding/ChangeRequest FK-validation asymmetry (they still lack
    the pre-check the 5 covered resources have, so a bad FK still surfaces
    as a raw 500 rather than a clean 404) is real but pre-existing, not
    introduced by this branch — the S5 commit's premise only checked the 5
    resources it covers, not these two. Documented follow-up.
  • Pagination fetching the full unbounded result set and slicing in Rust
    rather than pushing LIMIT/OFFSET into SQL is a deliberate,
    already-documented tradeoff from the S12 work (pushing to SQL would
    require changing the BackendStore trait and its CLI/grading call sites
    — out of scope for a hygiene fix on tables that aren't expected to be
    large).

Verification

  • All suites green across newton-types/backend/core/cli; fmt + clippy
    -D warnings clean; full cargo test --workspace green after every
    commit, independently re-verified (not just trusting subagent
    self-reports) at each of the 10 work items plus the coverage-gap and
    review-fix commits.
  • Diff coverage 94.45% (1567/1659 instrumented added lines vs
    main) — an initial pass measured 88.54%; closed the gap with targeted
    tests (the AgentClient/LlmAdjudicator real-implementation error
    paths via the established "non-runnable engine" test trick, the S15
    truncation-marker branches via a real over-cap subprocess and a
    helper-level unit test, FindingStatus/Severity/Origin round-trip
    tests, and spawn_blocking JoinError-mapping tests via a panicking
    store test double).
  • 8-angle code review with targeted 1-vote verification on ambiguous
    findings; 5 confirmed issues fixed on-branch (above).
  • One commit (context_fidelity lint / monitor purge / terminal_stop,
    originally 90 characters) exceeded NEWTON-0009's 72-char subject limit —
    fixed via scripted git rebase -i reword before push, content
    byte-identical.

Known follow-ups (documented, not blocking)

  • Four pre-existing CLI arg-parsing validation gaps discovered
    incidentally during review (silent-default-on-invalid-value for
    --port/--format, integer-wraparound-bypasses-validation for
    --parallel-limit/--timeout/--last) — live in code this branch
    never touches, predate tranche 4 entirely; worth a dedicated follow-up.
  • Finding/ChangeRequest creation still has no FK pre-check (unlike
    the 5 resources S5 covers) — a bad componentId/repoId/kpiId on
    POST /findings surfaces as a raw 500 rather than a clean 404.
  • RealLlmAdjudicator::adjudicate duplicates
    RealAgentClient::run_pipeline's spawn_blocking body rather than
    delegating to it with adjudication-specific template/schema/parsing — a
    ~45-line reduction is available.
  • workflow_files.rs's spawn_blocking+JoinError three-way match is
    repeated at all 5 handler call sites; only the join-error arm is truly
    shared logic across them.
  • AppJson<T> (the new Json<T> rejection wrapper) is adopted only by
    the two Finding handlers that surfaced the bug; the rest of this API's
    many Json<T> extractors could adopt it for consistency, though none
    have this specific typed-enum-rejection issue today.
  • Pagination's limit/offset clamp logic
    (DEFAULT_LIST_LIMIT/MAX_LIST_LIMIT/clamp helper) is duplicated
    across catalog.rs/dashboard.rs/plans.rs rather than shared from
    one location.
  • service.rs's FK-existence checks fetch a full row via
    get_product/get_component/get_repo rather than the cheaper
    row_exists probe already used for the same purpose elsewhere in the
    store layer.

@gonewton-deploy gonewton-deploy Bot added the auto-release Automated release PR label Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-release Automated release PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants