# Upstream issues — Invercargill / `invercargill-json`

Friction points surfaced by the Percipio migration that must be fixed **upstream of
Statum**, i.e. in the `invercargill-1` / `invercargill-json` libraries (declared as
dependencies in `meson.build`).

All of the items below are correctness/crash bugs in Invercargill's JSON mapping
layer — specifically the `ValueElement` → `from_element` → `JsonElement` /
`from_properties` path. Statum merely *calls* these APIs (see call sites below), so
it can only work around them, not fix them. Item numbers in parentheses refer to
the original Percipio friction report.

---

## Severe — process-crashing serialization

### I-1. `int64` values segfault during JSON serialisation (orig #7)
> **RESOLVED (verified 2026-08-31, WP1 server-side effort).** The dispatch fix
> landed upstream in Invercargill-Json commit `fc38c4c` ("Fix bool and int
> serialisation issues") — `from_element` now reads the raw `GValue` behind a
> `ValueElement` and dispatches on the fundamental type. This effort did not
> need to modify Invercargill-Json; `Statum/tests` round-trips `int64`
> (full 2^53+ precision) through both the public snapshot path and
> `EncryptionProvider.author_properties`/`read_properties` (the Percipio login
> crash path).


- **What happens:** `GObjectMapping.to_properties()` wraps each scalar GObject
  property in `new ValueElement(value)` (`src/GObjectMapping.vala:85`). When that
  `PropertyDictionary` is later serialised via `JsonElement.from_properties()` —
  e.g. in `EncryptionProvider.author_properties`
  (`src/Cryptography/EncryptionProvider.vala:77`) or in the snapshot-to-client
  path — the `int64` branch of `from_element` crashes at
  `node.set_int(element.as<int64?>())`.
- **GDB:** backtrace confirmed the segfault in
  `encryption_provider_author_properties` → `JsonElement.from_properties`.
- **Impact:** catastrophic. The login action hard-crashed the **whole server
  process** (segfault) until every `int64` was purged from slot state. Single
  most dangerous footgun.
- **Fix (Invercargill):** repair the `int64` path in
  `from_element` / `ValueElement.as<int64?>()` so a legitimately-held `int64`
  `ValueElement` produces a valid `Json.Node` via `set_int` instead of crashing.

### I-2. `bool` values segfault on the encryption serialisation path (orig #9)
> **RESOLVED (verified 2026-08-31, WP1 server-side effort).** Same upstream fix
> as I-1 (Invercargill-Json `fc38c4c`); verified through the
> `set_private_typed` → `author_properties` → `read_properties` path by
> `Statum/tests`. A related Statum-side lifetime bug found while testing this
> path — `EncryptionProvider.read_properties` returned a `JsonObject` wrapping
> memory owned by a temporary `JsonElement` (use-after-free) — was fixed in
> Statum; it now returns values re-parsed into a `PropertyDictionary` it owns.


- **What happens:** the same `from_properties`/`from_element` family crashes on a
  `bool` `ValueElement`. The **public** path does not crash on bools (it
  serialises the snapshot DTO through a different, robust serialiser), but the
  **encryption/private** path — `set_private_typed`
  (`src/DirectiveBuilder.vala:56`) → `to_properties` →
  `EncryptionProvider.author_properties`
  (`src/Cryptography/EncryptionProvider.vala:77`) → `from_properties` — does.
- **Impact:** had to strip all `bool`s out of `AuthPrivate` and derive admin
  status from the signed public `role` instead.
- **Fix (Invercargill):** make `from_element`/`from_properties` handle `bool`
  `ValueElement`s without crashing, so the private/encryption path is as robust
  as the public path. (Same root cause as I-1: the `ValueElement` type dispatch
  in `from_element` is incomplete.)

## Correctness — wrong wire type

### I-3. `int` (32-bit) serialises as a JSON **string**, not a number (orig #8)
> **RESOLVED (verified 2026-08-31, WP1 server-side effort).** Same upstream fix
> as I-1 (Invercargill-Json `fc38c4c`); `Statum/tests` asserts `int` fields
> serialise as unquoted JSON numbers and round-trip on both paths.


- **What happens:** `to_properties` on a GObject with an `int` field produces a
  JSON string (`"best_score": "-1"`) instead of a number. The int32 branch in
  `from_element` is not reached for `ValueElement`s; it falls through to the
  `assignable_to<string>()` transform fallback.
- **Impact:** slot numbers render as quoted strings; server-side `get_int` on a
  round-tripped value fails because the encrypted JSON stored a string. JS
  coercion mostly hides it for display, but it breaks round-tripping.
- **Fix (Invercargill):** make `from_element` emit numbers for numeric
  `ValueElement`s consistently (reach the int32/int64 branches rather than the
  string fallback).

> Note: I-1, I-2 and I-3 share a single root cause — the `ValueElement` →
> `from_element` type dispatch is unreliable for non-string scalars. Fixing the
> dispatch holistically would close all three at once.

## Ergonomic / API

### I-4. `Properties` interface has no typed setter / `set_json` (orig #16, secondary)
> **Mitigated Statum-side (2026-08-31):** `GObjectMapping.to_properties` now
> returns `PropertyDictionary` directly (see `statum-issues.md` S-13), so
> Statum callers no longer need the cast. The upstream interface enhancement
> remains open.

- **What happens:** `GObjectMapping.to_properties()` returns the `Properties`
  interface (`src/GObjectMapping.vala:57`), but adding a list/nested value
  requires casting to the concrete `PropertyDictionary` to use its indexer
  (`(PropertyDictionary) to_properties(pub)`).
- **Impact:** fragile, undocumented cast.
- **Fix (Invercargill):** give the `Properties` interface a typed setter (e.g.
  `set_json(string key, Element element)`) so callers need not cast to
  `PropertyDictionary`.
  - The **primary** mitigation for orig #16 is Statum-side (see
    `statum-issues.md` S-13: change `to_properties` to return `PropertyDictionary`
    directly); this item is the upstream API enhancement that would remove the
    cast at the source.

### I-5. `PropertyMapperBuilder`'s default constructor closure dangled after build
> **RESOLVED (2026-08-31, WP1 server-side effort).** Fixed in this repository's
> copy of Invercargill as a pure bug fix — no API change. The builder now
> installs no default constructor closure, and
> `PropertyMapper.materialise()` falls back to `Object.new(typeof(T))` when no
> constructor was set explicitly.

- **What happens:** `PropertyMapperBuilder<T>`'s constructor defaulted its
  `constructor` field to a closure capturing the builder itself. The builder
  is usually transient (built and dropped inside `PropertyMapper.build_for`),
  while the returned `PropertyMapper` lives on, so every later
  `materialise()` call went through a closure pointing at freed memory — a
  use-after-free that surfaced as crashes/garbage instances when mappers were
  resolved and used after the building scope had ended.
- **Impact:** latent lifetime bug on every mapper built via
  `PropertyMapper.build_for` that did not call `set_constructor` explicitly
  (the common case).
- **Fix (Invercargill):** leave the builder's `constructor` null by default
  and let `materialise()` construct through `Object.new(typeof(T))` — no
  closure outlives the builder.

---

## Reproduction call sites (in Statum, for the Invercargill maintainers)

| Bug | Statum call site | Invercargill symbol that crashes/misbehaves |
|-----|------------------|---------------------------------------------|
| I-1 | `GObjectMapping.vala:85` (`new ValueElement(value)`) → `EncryptionProvider.vala:77` (`JsonElement.from_properties`) | `ValueElement.as<int64?>()` → `Json.Node.set_int` |
| I-2 | `DirectiveBuilder.vala:56` → `EncryptionProvider.vala:77` | `from_properties` / `from_element` on a `bool` |
| I-3 | `GObjectMapping.vala:85` → any `from_properties`/`stringify` | `from_element` falls through to `assignable_to<string>()` for int32 |
| I-4 | `GObjectMapping.vala:57` (return type) | `Properties` interface lacks a setter |