SY-4122: Arc Statuses RFC

[sy-nico]

Pull Request

SY-4122

Greptile Summary

This PR introduces RFC 0037, specifying an Arc status module with two ExecBoth functions — status.set (name/key upsert with preserve-on-omit semantics) and status.delete (by key or name) — plus a companion trade study evaluating design alternatives. The RFC is detailed and addresses several edge cases (concurrency via UpsertByName transaction, WASM positional constraints, empty-string truthiness prerequisite), but has one implementation-level inconsistency that could silently mis-route errors:

• P1: The delete host-function pseudocode checks gorp.ErrNotFound while the set pseudocode and the specified Update method both use query.ErrNotFound; if these are distinct sentinel values, the delete by-key-miss branch will never match, routing not-found to reportError instead of the specified reportWarning.

Confidence Score: 4/5

Safe to merge after resolving the ErrNotFound inconsistency between set and delete pseudocode.

One P1 finding: the delete host-function pseudocode uses gorp.ErrNotFound while the set pseudocode and Update spec use query.ErrNotFound. If these differ, the delete by-key-miss warning branch is silently dead. No other blocking issues; the concurrent-create race is addressed via UpsertByName, the multi-match contradiction was resolved, and WASM positional limitations are now documented and acknowledged.

docs/tech/rfc/0037-260427-arc-status-updates.md — specifically Sections 5.2.2 and 5.5 regarding error sentinel consistency and the undocumented Writer struct additions.

Important Files Changed

Filename	Overview
docs/tech/rfc/0037-260427-arc-status-updates.md	New RFC (840 lines) specifying the Arc status module (set/delete). Contains a P1 inconsistency: the delete pseudocode uses gorp.ErrNotFound while the set pseudocode and Update spec use query.ErrNotFound; also references a w.db field and Writer.NewRetrieve() method that are not listed in the change table.
docs/tech/rfc/0037-trade-study.md	Companion trade study evaluating polymorphism vs pipe/threading alternatives for status.set. Self-contained and well-reasoned; minor issue is the opening header says "Not committed" despite being committed.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Arc call site] -->|WASM positional| B[status.set host fn]
    A -->|Flow named config| C[setStatus Flow node]
    B --> D{uuid.Parse identifier}
    C --> D
    D -->|parseable - key path| E[Writer.Update]
    D -->|not parseable - name path| F[Writer.UpsertByName tx]
    E -->|query.ErrNotFound| G[reportError / return 0]
    E -->|other error| G
    E -->|success| H[return key handle]
    F --> I{WhereNames results}
    I -->|0 matches| J[create new status]
    I -->|1 match| K[apply overlay + persist]
    I -->|>1 matches| L[errMultipleMatches → reportError / return 0]
    I -->|query error| L
    J --> M[return new key]
    K --> N[return existing key]
    O[status.delete host fn] --> P{uuid.Parse identifier}
    P -->|parseable| Q[Writer.Delete by key]
    P -->|not parseable| R[Retrieve WhereNames]
    Q -->|gorp.ErrNotFound| S[reportWarning]
    Q -->|other error| T[reportError]
    R -->|0 matches| S
    R -->|1 match| U[Delete single row]
    R -->|N matches| V[Delete all rows + reportInfo count]

Loading

_{Reviews (8): Last reviewed commit: "SY-4122: update" | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[pjdotson]

The biggest thing to note here is that I don't think we should have separate function signatures (arguments and returns) for WASM and Flow. This is going to confuse users because something with the same name (status.set or status.delete) will have two different ways of calling it and return two different things.

[pjdotson]

I don't understand this: your signature above does not have a name argument

[sy-nico]

I got myself confused when working on the ranges rfc in parallel. My aim/suggestion is to have a unified "identifer" field that accepts the key or name streing for the status. I'm still exploring this idea.

I think it would make sense to have a `status.create(name="") function, where the behavior would be:

• create if not_found
• If one is found, throw a notification and return.
• If multiple are found, throw a warning and return the first.
  • Or we could decide to throw an error and return None.

We just need a clean way for operators to initialize/define the statuses and make sure they are in the system before their sequences run and they call the standard status.set(id, variant, msg) with low friction.

[emilbon99]

1. We don't have a separation between set and create in our existing client libraries. This means we're intentionally deviating from the existing pattern we've laid out over the last two years. Do you feel your justification for having separate methods is warranted, and, if so, should we be changing our client libraries? I highly recommend doing research into how the best ORMs and client libraries in the world solve this problem. Are there novel ways of working across updates/creates that you haven't discovered yet? Examples are prisma, django, etc. These companies have spent years thinking about these problems. How much of what you wrote in your RFC is based off of what industry says we should do? If so, cite examples.

2. From what I've read it doesn't seem like you have a proper solution to the reactive vs. imperative context versions of status.set i.e. these are two separate function signatures. You say that the configuration parameters map directly to the function input parameters? How and why? This is another new standard you're introducing. These two things are fundamentally different in arc.

func set{
    cat string
}() {

}

vs.

func set(cat string) {

}

One gets called like this set{cat="dog"} and one takes its input parameter as string_chan -> set{} which then receives cat via string chan. If you're proposing we merge the two somehow, how do we do it? does this apply only to statuses? If not, how does it positively or negatively affect other functions. This doesn't seem well thought out.

[emilbon99]

See my long comment attached to the RFC

[emilbon99]

We already have built-in update capabilities in gorp. There is no need to invent your own retrieve/write mechanisms here. If you need an update mechanism, then you should update the status service to support it.

[emilbon99]

Do you want me to leave syntax correction comments like this in an RFC?

[sy-nico]

Updated the RFC to add a Writer.Update method on the status service that wraps gorp.NewUpdate, and reworked the host function to compose service methods instead of opening retrieve/write directly.

[sy-nico]

@emilbon99 , replying to this top-level comment

1. The set/create split is gone. create was removed and set is now a true upsert, matching the Python and TS clients.

2. Vocabulary slip on my part. Should have declared the params as Config, not Inputs. The model is the same as trigger -> time.wait{duration=3s} -> next: wire triggers, Config carries the values. WASM fills the same slots positionally. Fixed in the type declarations and the Vocabulary entry.

[greptile-apps]

gorp.ErrNotFound vs query.ErrNotFound inconsistency

The set host function (Section 5.2.1, line ~511) checks errors.Is(err, query.ErrNotFound) when the Update method returns a not-found error (as specified in Section 5.5). The delete host function here checks errors.Is(err, gorp.ErrNotFound) against the return from Writer.Delete. If these are different sentinel values, one of the two checks is silently a no-op: for delete, a by-key miss would fall through to reportError instead of the intended reportWarning path. Both call sites should use the same sentinel type, and Section 6.0's change table should document which package owns the canonical not-found error.