Skip to content

Reject ambiguous element orderings #164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ChrisPenner merged 14 commits into from
Dec 9, 2025
Merged

Reject ambiguous element orderings #164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
4e0bcf2
Bump unison to trunk
ChrisPenner Dec 8, 2025
b28f7bb
Revive logic to reject components with ambiguous element orderings
ChrisPenner Dec 8, 2025
00f2046
Move sync transcripts
ChrisPenner Dec 8, 2025
5628202
Rewrite 'run.zsh' for sync transcripts
ChrisPenner Dec 8, 2025
b9969be
Update sync transcript output
ChrisPenner Dec 8, 2025
0d99db0
Fix makefile for new project structure
ChrisPenner Dec 8, 2025
4c1aa36
Add transcript to assert rejection from pushing ambiguous component
ChrisPenner Dec 9, 2025
69d213e
Bump transcripts to use UCM 1.0.0
ChrisPenner Dec 9, 2025
0458557
Rerun all transcripts against new unison
ChrisPenner Dec 9, 2025
fd9481a
Makefile speedup when `fd` is installed
ChrisPenner Dec 9, 2025
e16df5d
Expect push to fail in transcript
ChrisPenner Dec 9, 2025
b74da46
Add logic for intentional failures
ChrisPenner Dec 9, 2025
91dc1da
Rerun transcripts again
ChrisPenner Dec 9, 2025
34c4abb
Rerun all transcripts
ChrisPenner Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ jobs:

# Install ucm
mkdir ucm
curl -L https://github.com/unisonweb/unison/releases/download/release%2F0.5.47/ucm-linux-x64.tar.gz | tar -xz -C ucm
curl -L https://github.com/unisonweb/unison/releases/download/release%2F1.0.0/ucm-linux-x64.tar.gz | tar -xz -C ucm
export PATH=$PWD/ucm:$PATH

# Start share and it's dependencies in the background
Expand Down
10 changes: 4 additions & 6 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ SHARE_PROJECT_ROOT := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))
export SHARE_PROJECT_ROOT
UNAME := $(shell uname)
STACK_FLAGS := "--fast"
dist_dir := $(shell stack path | awk '/^dist-dir/{print $$2}')
exe_name := share-api
exe := $(dist_dir)/build/$(exe_name)/$(exe_name)
exe := $(shell stack exec -- which $(exe_name))
target_dir := docker/tmp
installed_share := $(target_dir)/$(exe_name)
unison := $(shell command -v unison)
Expand All @@ -27,10 +26,9 @@ endif
$(target_dir):
mkdir $@

$(exe): $(shell find . unison -type f -name '*.hs') $(shell find . unison -type f -name '*.yaml')
@echo $(exe)
@echo $@
stack build $(STACK_FLAGS)
$(exe): $(shell fd '' . unison --type file -e hs 2>/dev/null || find . unison -type f -name '*.hs') $(shell fd '' . unison --type file -e yaml 2>/dev/null || find . unison -type f -name '*.yaml')
@echo Building $(exe_name)
stack build $(STACK_FLAGS) $(exe_name)

$(installed_share): $(exe) $(target_dir)
cp $(exe) $(installed_share)
Expand Down
23 changes: 12 additions & 11 deletions share-api/src/Share/Web/UCM/Sync/Impl.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import Share.Web.Errors
import Share.Web.UCM.Sync.HashJWT qualified as HashJWT
import Share.Web.UCM.Sync.Types (EntityBunch (..), RepoInfoKind (..), entityKind)
import U.Codebase.Causal qualified as Causal
import U.Codebase.Sqlite.HashHandle qualified as HH
import U.Codebase.Sqlite.Orphans ()
import Unison.Codebase.Path qualified as Path
import Unison.Hash32 (Hash32)
Expand Down Expand Up @@ -259,11 +260,11 @@ insertEntitiesToCodebase codebase entities = do
mayErrs <- PG.transactionUnsafeIO $ batchValidateEntities maxParallelismPerUploadRequest isComponentHashMismatchAllowedIO isCausalHashMismatchAllowedIO unsavedEntities
case mayErrs of
Nothing -> pure ()
Just (err :| _errs) -> throwError err
-- case err of
-- Right e -> throwError e
-- Left () ->
-- throwError $ Sync.InvalidByteEncoding (Hash32.fromHash hash) Sync.TermComponentType "Incomplete element ordering in term components"
Just (err :| _errs) ->
case err of
Right e -> throwError e
Left (HH.IncompleteElementOrderingError (ComponentHash hash)) ->
throwError $ Sync.InvalidByteEncoding (Hash32.fromHash hash) Sync.TermComponentType "Incomplete element ordering in term components"
SyncQ.saveTempEntities codebase unsavedEntities
let hashesNowInTemp = Set.fromList (fst <$> Foldable.toList unsavedEntities) <> (Set.fromList . Foldable.toList $ hashesAlreadyInTemp)
pure hashesNowInTemp
Expand Down Expand Up @@ -397,7 +398,7 @@ batchValidateEntities ::
(ComponentHash -> ComponentHash -> IO Bool) ->
(CausalHash -> CausalHash -> IO Bool) ->
f (Hash32, Sync.Entity Text Hash32 Hash32) ->
IO (Maybe (NonEmpty Sync.EntityValidationError))
IO (Maybe (NonEmpty (Either HH.HashingFailure Sync.EntityValidationError)))
batchValidateEntities maxParallelism checkIfComponentHashMismatchIsAllowed checkIfCausalHashMismatchIsAllowed entities = do
errs <- UnliftIO.pooledForConcurrentlyN maxParallelism entities \(hash, entity) ->
validateEntity checkIfComponentHashMismatchIsAllowed checkIfCausalHashMismatchIsAllowed hash entity
Expand All @@ -409,16 +410,16 @@ validateEntity ::
(CausalHash -> CausalHash -> m Bool) ->
Hash32 ->
Share.Entity Text Hash32 Hash32 ->
m (Maybe Sync.EntityValidationError)
m (Maybe (Either HH.HashingFailure Sync.EntityValidationError))
validateEntity checkIfComponentHashMismatchIsAllowed checkIfCausalHashMismatchIsAllowed hash entity = do
case (Sync.validateEntity hash entity) of
Just (err@(Sync.EntityHashMismatch Sync.TermComponentType (Sync.HashMismatchForEntity {supplied = expectedHash, computed = actualHash}))) ->
Just (Right (err@(Sync.EntityHashMismatch Sync.TermComponentType (Sync.HashMismatchForEntity {supplied = expectedHash, computed = actualHash})))) ->
checkIfComponentHashMismatchIsAllowed (ComponentHash . Hash32.toHash $ expectedHash) (ComponentHash . Hash32.toHash $ actualHash) >>= \case
False -> pure (Just err)
False -> pure (Just $ Right err)
True -> pure Nothing
Just (err@(Sync.EntityHashMismatch Sync.CausalType (Sync.HashMismatchForEntity {supplied = expectedHash, computed = actualHash}))) ->
Just (Right (err@(Sync.EntityHashMismatch Sync.CausalType (Sync.HashMismatchForEntity {supplied = expectedHash, computed = actualHash})))) ->
checkIfCausalHashMismatchIsAllowed (CausalHash . Hash32.toHash $ expectedHash) (CausalHash . Hash32.toHash $ actualHash) >>= \case
False -> pure (Just err)
False -> pure (Just $ Right err)
True -> pure Nothing
Just err ->
-- This shouldn't happen unless the ucm client is buggy or malicious
Expand Down
3 changes: 2 additions & 1 deletion share-task-runner/src/Share/Tasks/AmbiguousComponentCheck.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import Share.Postgres.Cursors qualified as PG
import Share.Prelude
import Share.Utils.Logging (Loggable (..))
import Share.Utils.Logging qualified as Logging
import U.Codebase.Sqlite.HashHandle qualified as HH
import U.Codebase.Sqlite.TempEntity
import Unison.Hash32
import Unison.Sync.EntityValidation qualified as EV
Expand All @@ -17,7 +18,7 @@ import Unison.Util.Servant.CBOR qualified as CBOR

data AmbiguousComponentCheckError
= TaskAmbiguousComponentCheckError Hash32
| TaskEntityValidationError Hash32 (Sync.EntityValidationError)
| TaskEntityValidationError Hash32 (Either HH.HashingFailure Sync.EntityValidationError)
| TaskEntityDecodingError Hash32 CBOR.DeserialiseFailure
deriving (Show, Eq)

Expand Down
4 changes: 3 additions & 1 deletion transcripts/run-transcripts.zsh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ done;

source "$(realpath "$(dirname "$0")")/transcript_helpers.sh"

# Base directory containing all transcripts
echo "UCM Version: $(transcript_ucm --version)"

# Base directory containing share-api transcripts
transcripts_location="transcripts/share-apis"

# Find all directories within transcripts_location
Expand Down
1 change: 1 addition & 0 deletions transcripts/share-apis/branch-browse/branch-find.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@
{
"annotation": {
"contents": "#r7hq9rqc3hebqailf77f656vbgice0716c6f0j7bj06muhj5ac1pq103gdh5974f14fcbmvp44fai5in1ajp5j3ejckfp6bffd3fpoo",
"fqn": "Remote",
"tag": "TypeReference"
},
"segment": "Remote"
Expand Down
2 changes: 2 additions & 0 deletions transcripts/share-apis/code-browse/codebase-definition-by-hash-constructor.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
{
"annotation": {
"contents": "#6kbe32g06nqg93cqub6ohqc4ql4o49ntgnunifds0t75qre6lacnbsr3evn8bkivj68ecbvmhkbak4dbg4fqertcpgb396rmo34tnh0#d0",
"fqn": "names.Thing.This",
"tag": "TermReference"
},
"segment": "This"
Expand All @@ -57,6 +58,7 @@
{
"annotation": {
"contents": "#6kbe32g06nqg93cqub6ohqc4ql4o49ntgnunifds0t75qre6lacnbsr3evn8bkivj68ecbvmhkbak4dbg4fqertcpgb396rmo34tnh0#d1",
"fqn": "names.Thing.That",
"tag": "TermReference"
},
"segment": "That"
Expand Down
2 changes: 2 additions & 0 deletions transcripts/share-apis/code-browse/codebase-definition-by-hash-term.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down Expand Up @@ -36,6 +37,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down
2 changes: 2 additions & 0 deletions transcripts/share-apis/code-browse/codebase-definition-by-name-encoded.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down Expand Up @@ -128,6 +129,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down
2 changes: 2 additions & 0 deletions ...pts/share-apis/code-browse/codebase-definition-by-name-should-minimally-suffix-again.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down Expand Up @@ -36,6 +37,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down
2 changes: 2 additions & 0 deletions transcripts/share-apis/code-browse/codebase-definition-by-name-should-minimally-suffix.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down Expand Up @@ -36,6 +37,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down
6 changes: 6 additions & 0 deletions ...owse/codebase-definition-by-name-should-pretty-print-external-names-if-no-local-name.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down Expand Up @@ -36,6 +37,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down Expand Up @@ -108,6 +110,7 @@
{
"annotation": {
"contents": "##Nat.+",
"fqn": "builtin.Nat.+",
"tag": "TermReference"
},
"segment": "+"
Expand All @@ -119,6 +122,7 @@
{
"annotation": {
"contents": "#dcgdua2lj6upd1ah5v0qp09gjsej0d77d87fu6qn8e2qrssnlnmuinoio46hiu53magr7qn8vnqke8ndt0v76700o5u8gcvo7st28jg",
"fqn": "names.apples.two",
"tag": "TermReference"
},
"segment": "two"
Expand All @@ -130,6 +134,7 @@
{
"annotation": {
"contents": "##Nat.+",
"fqn": "builtin.Nat.+",
"tag": "TermReference"
},
"segment": "+"
Expand All @@ -141,6 +146,7 @@
{
"annotation": {
"contents": "#dcgdua2lj6upd1ah5v0qp09gjsej0d77d87fu6qn8e2qrssnlnmuinoio46hiu53magr7qn8vnqke8ndt0v76700o5u8gcvo7st28jg",
"fqn": "names.apples.two",
"tag": "TermReference"
},
"segment": "two"
Expand Down
2 changes: 2 additions & 0 deletions ...hare-apis/code-browse/codebase-definition-by-name-should-work-with-empty-perspective.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down Expand Up @@ -36,6 +37,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down
4 changes: 4 additions & 0 deletions transcripts/share-apis/code-browse/codebase-definition-by-name.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down Expand Up @@ -36,6 +37,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down Expand Up @@ -108,6 +110,7 @@
{
"annotation": {
"contents": "##Nat.+",
"fqn": "builtin.Nat.+",
"tag": "TermReference"
},
"segment": "+"
Expand All @@ -119,6 +122,7 @@
{
"annotation": {
"contents": "#dcgdua2lj6upd1ah5v0qp09gjsej0d77d87fu6qn8e2qrssnlnmuinoio46hiu53magr7qn8vnqke8ndt0v76700o5u8gcvo7st28jg",
"fqn": "names.apples.two",
"tag": "TermReference"
},
"segment": "two"
Expand Down
1 change: 1 addition & 0 deletions transcripts/share-apis/code-browse/codebase-find.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down
3 changes: 3 additions & 0 deletions transcripts/share-apis/code-browse/codebase-lib-term-definition-from-lib-perspective.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
{
"annotation": {
"contents": "#2lg4ah6ir6t129m33d7gssnigacral39qdamo20mn6r2vefliubpeqnjhejai9ekjckv0qnu9mlu3k9nbpfhl2schec4dohn7rjhjt8",
"fqn": "lib.data.Tuple",
"tag": "TypeReference"
},
"segment": "Tuple"
Expand Down Expand Up @@ -76,6 +77,7 @@
{
"annotation": {
"contents": "#2lg4ah6ir6t129m33d7gssnigacral39qdamo20mn6r2vefliubpeqnjhejai9ekjckv0qnu9mlu3k9nbpfhl2schec4dohn7rjhjt8",
"fqn": "lib.data.Tuple",
"tag": "TypeReference"
},
"segment": "Tuple"
Expand Down Expand Up @@ -154,6 +156,7 @@
{
"annotation": {
"contents": "#2lg4ah6ir6t129m33d7gssnigacral39qdamo20mn6r2vefliubpeqnjhejai9ekjckv0qnu9mlu3k9nbpfhl2schec4dohn7rjhjt8#d0",
"fqn": "lib.data.Tuple.Tuple",
"tag": "TermReference"
},
"segment": "Tuple"
Expand Down
1 change: 1 addition & 0 deletions transcripts/share-apis/code-browse/codebase-lib-type-definition-from-lib-perspective.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
{
"annotation": {
"contents": "#2lg4ah6ir6t129m33d7gssnigacral39qdamo20mn6r2vefliubpeqnjhejai9ekjckv0qnu9mlu3k9nbpfhl2schec4dohn7rjhjt8#d0",
"fqn": "lib.data.Tuple.Tuple",
"tag": "TermReference"
},
"segment": "Tuple"
Expand Down
2 changes: 1 addition & 1 deletion transcripts/share-apis/code-browse/codebase-namespace-by-name-root.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"body": {
"fqn": "",
"hash": "#69sktmjiq1tc5tkvfs42676goeqbrlfq5edldkivupc0ene9qldekcm82njkfq1sfnjlqjcnrm8s93fr2l1rviqqospgv7clm4s7j0o",
"hash": "#00i5tm0i0v41f379hmhk06sktu1ifrtaaftsbe3qcpsef6b8p1rrpir1ac88v3m6pp5q8m4on28i87tftnsrs41eb4h7tq1e6lbd48g",
"readme": null
},
"status": [
Expand Down
4 changes: 4 additions & 0 deletions transcripts/share-apis/code-browse/codebase-term-dependencies-by-hash.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand All @@ -32,6 +33,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand All @@ -53,6 +55,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand All @@ -76,6 +79,7 @@
{
"annotation": {
"contents": "##Nat",
"fqn": "builtin.Nat",
"tag": "TypeReference"
},
"segment": "Nat"
Expand Down
Loading
Loading